Bug 37749

Summary: XOrg TileScreenSaver double frees srcbits+mskbits when AllocARGBCursor()'s calloc() fails
Product: xorg Reporter: Andre Klapper <a9016009>
Component: Server/GeneralAssignee: Xorg Project Team <xorg-team>
Status: RESOLVED FIXED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: medium Keywords: patch
Version: git   
Hardware: Other   
OS: All   
See Also: https://bugs.meego.com/show_bug.cgi?id=14651
Whiteboard:
i915 platform: i915 features:

Description Andre Klapper 2011-05-30 07:42:31 UTC 
Upstreaming from https://bugs.meego.com/show_bug.cgi?id=14651

Still applies to current git master codebase in
http://cgit.freedesktop.org/xorg/xserver/tree/dix/window.c and
http://cgit.freedesktop.org/xorg/xserver/tree/dix/cursor.c (with only a few lines of offset).

PATCH by Josh Soref:
https://bugs.meego.com/attachment.cgi?id=4985&action=diff
remove double frees


http://mxr.meego.com/repo.meego.com/source/xorg-x11-server/dix/window.c?mark=3271-3273,3279,3283,3285,3296-3299#3225

3225 TileScreenSaver(ScreenPtr pScreen, int kind)
3226 {
3233     unsigned char *srcbits, *mskbits;
3234     CursorPtr cursor;
3271     srcbits = malloc( BitmapBytePad(32)*16);
3272     mskbits = malloc( BitmapBytePad(32)*16);
3273     if (!srcbits || !mskbits)
3279     else
3280     {
3283         result = AllocARGBCursor(srcbits, mskbits, NULL, &cm, 0, 0, 0, 0,
0, 0,
3284                                  &cursor, serverClient, (XID)0);

http://mxr.meego.com/repo.meego.com/source/xorg-x11-server/dix/cursor.c?mark=241-247#231

231 AllocARGBCursor(unsigned char *psrcbits, unsigned char *pmaskbits,
235                 CursorPtr *ppCurs, ClientPtr client, XID cid)
236 {
238     CursorPtr   pCurs;

cursor argument will evaluate as false after this point:
241     *ppCurs = NULL;

let this alloc fail:
242     pCurs = (CursorPtr)calloc(CURSOR_REC_SIZE + CURSOR_BITS_SIZE, 1);
243     if (!pCurs)
244     {
first frees:
245         free(psrcbits);
246         free(pmaskbits);
247         return BadAlloc;

3285         if (cursor)
3296         else
3297         {

second frees:
3298             free(srcbits);
3299             free(mskbits);
Comment 1 Alan Coopersmith 2011-05-30 11:32:34 UTC 
Since all X server patches must be reviewed on xorg-devel, it's much easier & 
faster to upstream patches by sending them in git format to xorg-devel for 
review yourself, instead of putting them in bugzilla and waiting for
someone else to do so.   See:
http://www.x.org/wiki/Development/Documentation/SubmittingPatches
http://www.x.org/wiki/XServer#DevelopmentProcess
Comment 2 Andre Klapper 2011-05-31 14:24:54 UTC 
(In reply to comment #1)
> Since all X server patches must be reviewed on xorg-devel, it's much easier & 
> faster to upstream patches by sending them in git format to xorg-devel

The patch was not written by me, I can just offer it for inclusion in the way we received it in bugs.meego.com. The rest is up to the Xorg developers.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.