[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