aboutsummaryrefslogtreecommitdiff
path: root/intl
diff options
context:
space:
mode:
authorCarlos O'Donell <carlos@redhat.com>2013-05-22 14:50:26 -0400
committerCarlos O'Donell <carlos@redhat.com>2013-05-22 14:50:26 -0400
commit7a44c18fb4b1a65ebb1fece0b0d04f2570ed4d82 (patch)
treea6d12bc38f6df81be682c581c525653c883ed2f7 /intl
parentb50a71810bbd35b8c83ba6eff4e6cc6faf93a7ea (diff)
downloadglibc-7a44c18fb4b1a65ebb1fece0b0d04f2570ed4d82.zip
glibc-7a44c18fb4b1a65ebb1fece0b0d04f2570ed4d82.tar.gz
glibc-7a44c18fb4b1a65ebb1fece0b0d04f2570ed4d82.tar.bz2
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.c22
-rw-r--r--intl/loadmsgcat.c7
2 files changed, 24 insertions, 5 deletions
diff --git a/intl/dcigettext.c b/intl/dcigettext.c
index 110307b..f4aa215 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 e4b7b38..ac90ed1 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: