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 | |
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')
-rw-r--r-- | intl/dcigettext.c | 22 | ||||
-rw-r--r-- | intl/loadmsgcat.c | 7 |
2 files changed, 24 insertions, 5 deletions
diff --git a/intl/dcigettext.c b/intl/dcigettext.c index 110307bdb2..f4aa215744 100644 --- a/intl/dcigettext.c +++ b/intl/dcigettext.c @@ -638,6 +638,11 @@ DCIGETTEXT (domainname, msgid1, msgid2, plural, n, category) retval = _nl_find_msg (domain->successor[cnt], binding, msgid1, 1, &retlen); + /* Resource problems are not fatal, instead we return no + translation. */ + if (__builtin_expect (retval == (char *) -1, 0)) + goto no_translation; + if (retval != NULL) { domain = domain->successor[cnt]; @@ -941,6 +946,11 @@ _nl_find_msg (domain_file, domainbinding, msgid, convert, lengthp) nullentry = _nl_find_msg (domain_file, domainbinding, "", 0, &nullentrylen); + /* Resource problems are fatal. If we continue onwards we will + only attempt to calloc a new conv_tab and fail later. */ + if (__builtin_expect (nullentry == (char *) -1, 0)) + return (char *) -1; + if (nullentry != NULL) { const char *charsetstr; @@ -1170,10 +1180,14 @@ _nl_find_msg (domain_file, domainbinding, msgid, convert, lengthp) 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 (newmem != NULL) + { + /* Add the block to the list of blocks we have to free + at some point. */ + newmem->next = transmem_list; + transmem_list = newmem; + } + /* Fall through and return -1. */ # endif } if (__builtin_expect (newmem == NULL, 0)) 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: |