[PATCH 6/8] Avoid memory leak on realloc failure in localRegisterFreeBoxCallback

Alan Coopersmith alan.coopersmith at oracle.com
Fri Feb 1 23:04:40 PST 2013


On 01/28/13 05:56 PM, Peter Hutterer wrote:
> On Mon, Jan 28, 2013 at 05:08:40PM -0800, Alan Coopersmith wrote:
>> Also avoids leaving invalid pointers in structures if realloc had to
>> move them elsewhere to make them larger.
>>
>> Found by parfait 1.1 code analyzer:
>>    Memory leak of pointer 'newCallbacks' allocated with realloc(((char*)offman->FreeBoxesUpdateCallback), (8 * (offman->NumCallbacks + 1)))
>>         at line 328 of hw/xfree86/common/xf86fbman.c in function 'localRegisterFreeBoxCallback'.
>>           'newCallbacks' allocated at line 320 with realloc(((char*)offman->FreeBoxesUpdateCallback), (8 * (offman->NumCallbacks + 1))).
>>           newCallbacks leaks when newCallbacks != NULL at line 327.
>>    Memory leak of pointer 'newPrivates' allocated with realloc(((char*)offman->devPrivates), (8 * (offman->NumCallbacks + 1)))
>>         at line 328 of hw/xfree86/common/xf86fbman.c in function 'localRegisterFreeBoxCallback'.
>>           'newPrivates' allocated at line 324 with realloc(((char*)offman->devPrivates), (8 * (offman->NumCallbacks + 1))).
>>           newPrivates leaks when newCallbacks == NULL at line 327.
>>
>> Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
>> ---
>>  hw/xfree86/common/xf86fbman.c |   12 +++++++-----
>>  1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/xfree86/common/xf86fbman.c b/hw/xfree86/common/xf86fbman.c
>> index c2e7bab..4da6af2 100644
>> --- a/hw/xfree86/common/xf86fbman.c
>> +++ b/hw/xfree86/common/xf86fbman.c
>> @@ -320,15 +320,17 @@ localRegisterFreeBoxCallback(ScreenPtr pScreen,
>>      newCallbacks = realloc(offman->FreeBoxesUpdateCallback,
>>                             sizeof(FreeBoxCallbackProcPtr) *
>>                             (offman->NumCallbacks + 1));
>> +    if (!newCallbacks)
>> +        return FALSE;
>> +    else
>> +        offman->FreeBoxesUpdateCallback = newCallbacks;
>>  
>>      newPrivates = realloc(offman->devPrivates,
>>                            sizeof(DevUnion) * (offman->NumCallbacks + 1));
>> -
>> -    if (!newCallbacks || !newPrivates)
>> +    if (!newPrivates)
>>          return FALSE;
> 
> not sure what the callers expect here, but you may be able to free
> newCallbacks here too. unless that's cleaned up elsewhere on failure. 

Since we already stuck newCallbacks into offman->FreeBoxesUpdateCallback,
I think it's taken care of by xf86FBCloseScreen.

-- 
	-Alan Coopersmith-              alan.coopersmith at oracle.com
	 Oracle Solaris Engineering - http://blogs.oracle.com/alanc


More information about the xorg-devel mailing list