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

[Soeren Sandmann]

Keith Packard <keithp at keithp.com> writes:

> 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.
> 
> Then I think the only needed fix is to check for the trivial loop:
> 
> index 48693b8..78e8c9b 100644
> --- a/render/picture.c
> +++ b/render/picture.c
> @@ -1131,6 +1131,8 @@ ChangePicture (PicturePtr	pPicture,
>  		    pAlpha = NEXT_PTR(PicturePtr);
>  		if (!error)
>  		{
> +		    if (pAlpha && pAlpha->pDrawable == pPicture->pDrawable)
> +			pAlpha = NULL;
>  		    if (pAlpha && pAlpha->pDrawable->type == DRAWABLE_PIXMAP)
>  			pAlpha->refcnt++;
>  		    if (pPicture->alphaMap)
> 
> Is there somewhere in the server that is recursing through the alphaMap?
> fb doesn't, and I don't know of any acceleration code that deals
> with alphaMap.

The alphaMap structure gets mirrored in pixman images, and it does
recurse through its alpha_map pointer:

        static void
        bits_image_store_scanline_32 (bits_image_t *  image,
                                      int             x,
                                      int             y,
                                      int             width,
                                      const uint32_t *buffer)
        {
            image->store_scanline_raw_32 (image, x, y, width, buffer);
        
            if (image->common.alpha_map)
            {
                x -= image->common.alpha_origin_x;
                y -= image->common.alpha_origin_y;
        
                bits_image_store_scanline_32 (image->common.alpha_map, x, y, width, buffer);
            }
        }

There may be other cases. See also

        https://bugs.freedesktop.org/show_bug.cgi?id=23581

in which I suggest to simply BadMatch out if you try to set a picture
as its own alpha map or set an alpha map that already have an alpha
map.

The protocol should possibly also say that you get unpredictable
rendering if a picture and its alpha map point to the same underlying
drawable. (I don't think pixman will crash in that case, and
acceleration code ignores alpha maps generally).


Soren