This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix _nl_find_msg malloc failure case, and callers.


On 05/09/2013 03:24 AM, Andreas Jaeger wrote:
> On 05/07/2013 06:46 PM, Carlos O'Donell wrote:
>> Community,
>>
>> This patch fixes two issues, and perhaps should be two distinct patches,
>> but I present it here as one for the sake of completeness.
>>
>> This commit:
>> commit 006dd86111c44572dbd3b26e9c63dd0f834d7762
>> Author: Jeff Law <law at redhat.com>
>> Date:   Thu Jun 21 17:15:38 2012 -0600
>>
>>              [BZ #14277]
>>              * intl/dcigettext.c (_nl_find_msg): Avoid use after potential
>>              free.  Simplify list management for _LIBC case.
>>
>> Fails to check malloc's return in intl/dcigettext.c (_nl_find_msg):
>> ~~~~
>>                 freemem_size = INITIAL_BLOCK_SIZE;
>>                 newmem = (transmem_block_t *) malloc (freemem_size);
>>   # ifdef _LIBC
>>               /* Add the block to the list of blocks we have to free
>>              at some point.  */
>>               newmem->next = transmem_list;
>>               transmem_list = newmem;
>> ~~~
>> If malloc fails then newmem is NULL then newmem->next results in a fault.
>> The fix is easy enough, check for newmem != NULL, and fall through to
>> the error condition below which returns (char *) -1 e.g. resource error.
>>
>> The problem is that returning (char *) -1  will break all sorts of other
>> code, so while what we did is correct, the real failure case fix is slightly
>> broader.
>>
>> There are 4 other places where _nl_find_msg is called:
>>
>> (a) Recursively in _nl_find_msg.
>>
>>    Here we return -1 immediately if a recursive call to _nl_find_msg
>>    returns -1. Resource errors are fatal at this point and if we
>>    continue it just makes the matter worse by trying to allocate other
>>    structure like conv_tab.
>>
>> (b) From _nl_load_domain.
>>
>>    Here the error is also fatal, the domain is incomplete without
>>    pluralization information and the failure in _nl_find_msg means
>>    we won't be able to setup pluralization. We finalize the lock
>>    we just initialized and exit via the normal error path.
>>
>> (c) Twice from DCIGETTEXT.
>>
>>    In both cases the error is not fatal, it just means we have no
>>    translation and return the original message. We patch the second
>>    call to _nl_find_msg to ensure that future maintenance on the
>>    function doesn't move the check for -1 further away.
>>
>> No regressions on x86-64 or x86.
>>
>> However, no regressions isn't really a useful metric for this code.
>>
>> The change was tested as documented here:
>> http://sourceware.org/glibc/wiki/Testing/WhiteBox
>>
>> OK to checkin?
> 
> This looks fine, thanks,

After some more testing in Fedora rawhide...

commit 7a44c18fb4b1a65ebb1fece0b0d04f2570ed4d82
Author: Carlos O'Donell <carlos@redhat.com>
Date:   Wed May 22 14:50:26 2013 -0400

    Fix _nl_find_msg malloc failure case, and callers.

Checked in.

Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]