Crash in X server [with Patch]

[Keith Packard]

Michel Dänzer <michel at daenzer.net> writes:

> On 24.10.2014 07:09, Andreas Hartmetz wrote:
>>
>> commit bbcc084afc1396d3df42516baafea5839c95a488
>> Author: Andreas Hartmetz <ahartmetz at gmail.com>
>> Date:   Sat Oct 4 18:13:04 2014 +0200
>>
>>      glamor: Don't free memory we are going to use.
>>
>>      glamor_color_convert_to_bits() returns its second argument on
>>      success, NULL on error, and need_free_bits already makes sure that
>>      "bits" aliasing converted_bits is freed in the success case.
>>      Looks like the memory leak that was supposed to be fixed in
>>      6e50bfa706cd3ab884c933bf1f17c221a6208aa4 only occurred in the error
>>      case.
>>
>> diff --git a/glamor/glamor_pixmap.c b/glamor/glamor_pixmap.c
>> index 355fe4b..7c9bf26 100644
>> --- a/glamor/glamor_pixmap.c
>> +++ b/glamor/glamor_pixmap.c
>> @@ -774,8 +774,8 @@ _glamor_upload_bits_to_pixmap_texture(PixmapPtr
>> pixmap, GLenum format,
>>               return FALSE;
>>           bits = glamor_color_convert_to_bits(bits, converted_bits, w, h,
>>                                               stride, no_alpha, revert,
>> swap_rb);
>> -        free(converted_bits);
>>           if (bits == NULL) {
>> +            free(converted_bits);
>>               ErrorF("Failed to convert pixmap no_alpha %d,"
>>                      "revert mode %d, swap mode %d\n", no_alpha, revert,
>> swap_rb);
>>               return FALSE;
>>
>> I've already, a week or two ago, sent this to Michel a who reviewed it
>> (no idea how to forward that, guess I'll need another review from
>> whoever volunteers) and OK'd it.
>
> You could have just submitted the patch with my
>
> Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>
>
> to this list and to Keith Packard (Cc'd, though that probably won't be 
> visible in my post as distributed by the list), as I suggested. :)

Looks like there's a path which will leak these bits still -- the fast
path doesn't reach the call to free the bits. Can either of you review
this patch and I'll merge both in?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-glamor-Free-converted-bits-in-_glamor_upload_bits_to.patch
Type: text/x-diff
Size: 4824 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20141024/45420814/attachment.patch>
-------------- next part --------------

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 810 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20141024/45420814/attachment.sig>