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 |
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: |