diff options
author | Carlos O'Donell <carlos@redhat.com> | 2013-05-22 14:50:26 -0400 |
---|---|---|
committer | Carlos O'Donell <carlos@redhat.com> | 2013-05-22 14:50:26 -0400 |
commit | 7a44c18fb4b1a65ebb1fece0b0d04f2570ed4d82 (patch) | |
tree | a6d12bc38f6df81be682c581c525653c883ed2f7 /intl/loadmsgcat.c | |
parent | b50a71810bbd35b8c83ba6eff4e6cc6faf93a7ea (diff) | |
download | glibc-7a44c18fb4b1a65ebb1fece0b0d04f2570ed4d82.tar.gz glibc-7a44c18fb4b1a65ebb1fece0b0d04f2570ed4d82.tar.xz glibc-7a44c18fb4b1a65ebb1fece0b0d04f2570ed4d82.zip |
Fix _nl_find_msg malloc failure case, and callers.
This patch fixes two issues, and perhaps should be two distinct commits, but I present it here as one for the sake of completeness. Commit 006dd86111c44572dbd3b26e9c63dd0f834d7762 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); ... 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, one is OK, the other three are fixed to handle -1 error return value. 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 using SystemTap for fault injection to simulate malloc failure. --- 2013-05-03 Carlos O'Donell <carlos at redhat.com> [BZ #15441] * intl/dcigettext.c (DCIGETTEXT): Skip translating if _nl_find_msg returns -1. (_nl_find_msg): Return -1 if recursive call returned -1. If newmem is null return -1. * intl/loadmsgcat.c (_nl_load_domain): If _nl_find_msg returns -1 abort loading the domain.
Diffstat (limited to 'intl/loadmsgcat.c')
-rw-r--r-- | intl/loadmsgcat.c | 7 |
1 files changed, 6 insertions, 1 deletions
diff --git a/intl/loadmsgcat.c b/intl/loadmsgcat.c index e4b7b385a0..ac90ed1015 100644 --- a/intl/loadmsgcat.c +++ b/intl/loadmsgcat.c @@ -1237,7 +1237,7 @@ _nl_load_domain (domain_file, domainbinding) default: /* This is an invalid revision. */ invalid: - /* This is an invalid .mo file. */ + /* This is an invalid .mo file or we ran out of resources. */ free (domain->malloced); #ifdef HAVE_MMAP if (use_mmap) @@ -1257,6 +1257,11 @@ _nl_load_domain (domain_file, domainbinding) /* Get the header entry and look for a plural specification. */ nullentry = _nl_find_msg (domain_file, domainbinding, "", 0, &nullentrylen); + if (__builtin_expect (nullentry == (char *) -1, 0)) + { + __libc_rwlock_fini (domain->conversions_lock); + goto invalid; + } EXTRACT_PLURAL_EXPRESSION (nullentry, &domain->plural, &domain->nplurals); out: |