[PATCH] render: Avoid infinite loops in alpha map handling (#23581)

[Keith Packard]

On Tue, 11 May 2010 14:12:52 -0400, Adam Jackson <ajax at nwnk.net> wrote:

> I just want it to not trivially crash my server.  I could care less
> about correctness for this.

How about this?

From 50be5e8ff00e734506d2bb0c25054d7648f0cf1e Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp at keithp.com>
Date: Fri, 21 May 2010 09:01:43 -0700
Subject: [PATCH] Don't let alpha maps recurse in fb. Bug 23581.

Recursive alpha maps (where one picture's alpha map is set to a
picture with an external alpha map) would be all fine and dandy,
except for the case where the client constructs a loop. Detecting this
case when setting the alpha map values would be difficult as any time
an alpha map is set, the server would have to check for the looping
case.

Instead, a far simpler fix is to simply disallow recursive alpha maps
in the rendering code, the Render spec is ambiguous in this area and
allows us to to ignore the recursive case.

Signed-off-by: Keith Packard <keithp at keithp.com>
---
 fb/fbpict.c |   24 ++++++++++++++++++------
 1 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/fb/fbpict.c b/fb/fbpict.c
index 896d33e..18a5204 100644
--- a/fb/fbpict.c
+++ b/fb/fbpict.c
@@ -332,8 +332,11 @@ create_bits_picture (PicturePtr pict,
     return image;
 }
 
+static pixman_image_t *
+image_from_pict_internal (PicturePtr pict, Bool has_clip, int *xoff, int *yoff, Bool is_alpha_map);
+
 static void
-set_image_properties (pixman_image_t *image, PicturePtr pict, Bool has_clip, int *xoff, int *yoff)
+set_image_properties (pixman_image_t *image, PicturePtr pict, Bool has_clip, int *xoff, int *yoff, Bool is_alpha_map)
 {
     pixman_repeat_t repeat;
     pixman_filter_t filter;
@@ -382,10 +385,13 @@ set_image_properties (pixman_image_t *image, PicturePtr pict, Bool has_clip, int
     
     pixman_image_set_repeat (image, repeat);
     
-    if (pict->alphaMap)
+    /* Fetch alpha map unless 'pict' is being used
+     * as the alpha map for this operation
+     */
+    if (pict->alphaMap && !is_alpha_map)
     {
 	int alpha_xoff, alpha_yoff;
-	pixman_image_t *alpha_map = image_from_pict (pict->alphaMap, FALSE, &alpha_xoff, &alpha_yoff);
+	pixman_image_t *alpha_map = image_from_pict_internal (pict->alphaMap, FALSE, &alpha_xoff, &alpha_yoff, TRUE);
 	
 	pixman_image_set_alpha_map (
 	    image, alpha_map, pict->alphaOrigin.x, pict->alphaOrigin.y);
@@ -417,8 +423,8 @@ set_image_properties (pixman_image_t *image, PicturePtr pict, Bool has_clip, int
     pixman_image_set_source_clipping (image, TRUE);
 }
 
-pixman_image_t *
-image_from_pict (PicturePtr pict, Bool has_clip, int *xoff, int *yoff)
+static pixman_image_t *
+image_from_pict_internal (PicturePtr pict, Bool has_clip, int *xoff, int *yoff, Bool is_alpha_map)
 {
     pixman_image_t *image = NULL;
 
@@ -452,11 +458,17 @@ image_from_pict (PicturePtr pict, Bool has_clip, int *xoff, int *yoff)
     }
     
     if (image)
-	set_image_properties (image, pict, has_clip, xoff, yoff);
+	set_image_properties (image, pict, has_clip, xoff, yoff, is_alpha_map);
     
     return image;
 }
 
+pixman_image_t *
+image_from_pict (PicturePtr pict, Bool has_clip, int *xoff, int *yoff)
+{
+    return image_from_pict_internal (pict, has_clip, xoff, yoff, FALSE);
+}
+
 void
 free_pixman_pict (PicturePtr pict, pixman_image_t *image)
 {
-- 
1.7.1


> 
> - ajax
Non-text part: application/pgp-signature

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20100521/728e6a33/attachment.pgp>