[PATCH] xfixes: Forbid manipulating clip for source-only pictures (#28968)

[Hans de Goede]

Hi,

On 04/10/2014 05:34 PM, Adam Jackson wrote:
> Just throw BadPicture instead of crashing.  It's not currently a
> meaningful thing to do anyway, RenderSetPictureRectangles would error if
> you tried (which this patch changes to BadPicture as well for
> consistency).  The problem with trying to do it is if the clip is
> specified as a pixmap then we try to convert it to a region, and
> ->BitmapToRegion requires a ScreenPtr, and source-only pictures don't
> have one.
> 
> I can imagine a use for client clip on source-only pictures, so if we
> really wanted to allow this, probably the way forward is to always store
> the clip as a region internally, and when setting the clip _from_ a
> pixmap, look up BitmapToRegion relative to the pixmap not the picture.
> But since clearly nobody can be relying on it working...
> 
> Signed-off-by: Adam Jackson <ajax at redhat.com>

Looks good.

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards,

Hans

> ---
>  render/render.c | 2 +-
>  xfixes/region.c | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/render/render.c b/render/render.c
> index 3b7151a..9ac4a98 100644
> --- a/render/render.c
> +++ b/render/render.c
> @@ -638,7 +638,7 @@ ProcRenderSetPictureClipRectangles(ClientPtr client)
>      REQUEST_AT_LEAST_SIZE(xRenderSetPictureClipRectanglesReq);
>      VERIFY_PICTURE(pPicture, stuff->picture, client, DixSetAttrAccess);
>      if (!pPicture->pDrawable)
> -        return BadDrawable;
> +        return RenderErrBase + BadPicture;
>  
>      nr = (client->req_len << 2) - sizeof(xRenderSetPictureClipRectanglesReq);
>      if (nr & 4)
> diff --git a/xfixes/region.c b/xfixes/region.c
> index cc8f1a5..f9de525 100644
> --- a/xfixes/region.c
> +++ b/xfixes/region.c
> @@ -269,6 +269,9 @@ ProcXFixesCreateRegionFromPicture(ClientPtr client)
>  
>      VERIFY_PICTURE(pPicture, stuff->picture, client, DixGetAttrAccess);
>  
> +    if (!pPicture->pDrawable)
> +        return RenderErrBase + BadPicture;
> +
>      switch (pPicture->clientClipType) {
>      case CT_PIXMAP:
>          pRegion = BitmapToRegion(pPicture->pDrawable->pScreen,
> @@ -750,6 +753,9 @@ ProcXFixesSetPictureClipRegion(ClientPtr client)
>      VERIFY_PICTURE(pPicture, stuff->picture, client, DixSetAttrAccess);
>      VERIFY_REGION_OR_NONE(pRegion, stuff->region, client, DixReadAccess);
>  
> +    if (!pPicture->pDrawable)
> +        return RenderErrBase + BadPicture;
> +
>      return SetPictureClipRegion(pPicture, stuff->xOrigin, stuff->yOrigin,
>                                  pRegion);
>  }
>