[PATCH] render: Fix double-free on ARGB cursor error path

[Adam Jackson]

On Wed, 2014-08-20 at 11:48 -0500, Keith Packard wrote:
> Adam Jackson <ajax at redhat.com> writes:
> 
> > The gotos deleted by this patch are the only way to get to the bail:
> > label here.  In neither case do we need to free the cursor bits from the
> > caller; AllocARGBCursor will already do that on the failure path,
> > likewise AddResource will call the resource delete function on error.
> 
> I don't see AllocARGBCursor freeing anything in the failure paths, so I
> think we need to free source/mask. We also need to free argbits

I'm not making shit up, I promise.  AllocARGBCursor glues the source and
mask parameters into the cursor bits near the top:

int
AllocARGBCursor(unsigned char *psrcbits, unsigned char *pmaskbits,
                CARD32 *argb, CursorMetricPtr cm,
                unsigned foreRed, unsigned foreGreen, unsigned foreBlue,
                unsigned backRed, unsigned backGreen, unsigned backBlue,
                CursorPtr *ppCurs, ClientPtr client, XID cid)
{
// ...
    dixInitPrivates(bits, bits + 1, PRIVATE_CURSOR_BITS)
        bits->source = psrcbits;
    bits->mask = pmaskbits;
#ifdef ARGB_CURSOR
    bits->argb = argb;
#endif

(Note hideous indentation due to missing semicolon due to
dixInitPrivates() having one in the macro definition!)  Then the unwind
looks like:

 error:
    FreeCursorBits(bits);
    dixFiniPrivates(pCurs, PRIVATE_CURSOR);
    free(pCurs);

    return rc;

And the top of FreeCursorBits looks like:

    if (--bits->refcnt > 0)
        return;
    free(bits->source);
    free(bits->mask);
#ifdef ARGB_CURSOR
    free(bits->argb);
#endif

The way I triggered this was my RealizeCursor routine returned FALSE for
argb cursors since I hadn't implemented that path yet (see Xephyr hw
cursor patch).  The valgrind chirp in the commit message was from that,
not from AddResource failing.

- ajax