<div dir="ltr"><div>Yeah, this is how I would write it. </div>Can you provide a patch, Alan? </div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Sep 4, 2013 at 2:30 AM, Alan Coopersmith <<a href="mailto:alan.coopersmith@oracle.com" target="_blank">alan.coopersmith@oracle.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 09/ 3/13 11:05 PM, Peter Hutterer wrote: >> From 8057fc810fb2815ea4c069ac9c1bc77709afb778 Mon Sep 17 00:00:00 2001 >> From: Thomas Klausner <wiz@NetBSD.org> >> Date: Tue, 3 Sep 2013 09:44:18 +0200 >> Subject: [PATCH:xserver] Fix bug in cursor handling. >> >> CreateCursor (Xlib call XCreatePixmapCursor) with a non-bitmap >> source pixmap and a None mask is supposed to error out with BadMatch, >> but didn't. >> >> From der Mouse <mouse@Rodents-Montreal.ORG> >> >> Signed-off-by: Thomas Klausner <wiz@NetBSD.org> >> --- >> dix/dispatch.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/dix/dispatch.c b/dix/dispatch.c >> index 51d0de2..ae3b97b 100644 >> --- a/dix/dispatch.c >> +++ b/dix/dispatch.c >> @@ -2874,9 +2874,12 @@ ProcCreateCursor(ClientPtr client) >> } >> else if (src->drawable.width != msk->drawable.width >> || src->drawable.height != msk->drawable.height >> - || src->drawable.depth != 1 || msk->drawable.depth != 1) >> + || msk->drawable.depth != 1) >> return BadMatch; >> >> + if (src->drawable.depth != 1) >> + return (BadMatch); >> + > > I'm staring hard at this and struggling to see the change. The previous code > already returned BadMatch for a src depth of != 1. </div></div>I couldn't see it from the patch either, but a little more context than the patch includes helps: 2867 rc = dixLookupResourceByType((pointer *) &msk, stuff->mask, RT_PIXMAP, 2868 client, DixReadAccess); 2869 if (rc != Success) { 2870 if (stuff->mask != None) { 2871 client->errorValue = stuff->mask; 2872 return rc; 2873 } 2874 } 2875 else if (src->drawable.width != msk->drawable.width 2876 || src->drawable.height != msk->drawable.height 2877 || src->drawable.depth != 1 || msk->drawable.depth != 1) 2878 return BadMatch; So if stuff->mask is None, we do a resource lookup for a pixmap with that id, which should fail, since it's None (aka 0). Thus we gp into the check at 2869, but don't match the if at 2870, so don't return. Since 2875 is the else clause for the if we already went into, we skip over all those checks. The patch moves the src->drawable.depth check out of the else clause so it's always checked, not just in the (rc == Success) case for the mask pixmap lookup. It could even be moved up above the lookup of the mask pixmap, since it only checks against the cursor image pixmap. I'm not sure if there's ever a reason to try to lookup a mask id of None, otherwise this might be easier to follow as: /* Find and validate cursor image pixmap */ rc = dixLookupResourceByType((pointer *) &src, stuff->source, RT_PIXMAP, client, DixReadAccess); if (rc != Success) { client->errorValue = stuff->source; return rc; <div class="im"> } if (src->drawable.depth != 1) </div> return (BadMatch); /* Find and validate cursor mask pixmap, if one is provided */ if (stuff->mask != None) { rc = dixLookupResourceByType((pointer *) &msk, stuff->mask, RT_PIXMAP, client, DixReadAccess); if (rc != Success) { client->errorValue = stuff->mask; return rc; <div class="im"> } if (src->drawable.width != msk->drawable.width || src->drawable.height != msk->drawable.height </div><div class="im"> || msk->drawable.depth != 1) return BadMatch; } </div> else msk = NULL; -- -Alan Coopersmith- <a href="mailto:alan.coopersmith@oracle.com">alan.coopersmith@oracle.com</a> Oracle Solaris Engineering - <a href="http://blogs.oracle.com/alanc" target="_blank">http://blogs.oracle.com/alanc</a> <div class="HOEnZb"><div class="h5">_______________________________________________ <a href="mailto:xorg-devel@lists.x.org">xorg-devel@lists.x.org</a>: X.Org development Archives: <a href="http://lists.x.org/archives/xorg-devel" target="_blank">http://lists.x.org/archives/xorg-devel</a> Info: <a href="http://lists.x.org/mailman/listinfo/xorg-devel" target="_blank">http://lists.x.org/mailman/listinfo/xorg-devel</a> </div></div></blockquote></div> -- Jasper </div>