From 7a44c18fb4b1a65ebb1fece0b0d04f2570ed4d82 Mon Sep 17 00:00:00 2001
From: Carlos O'Donell <carlos@redhat.com>
Date: Wed, 22 May 2013 14:50:26 -0400
Subject: 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.
---
 intl/dcigettext.c | 22 ++++++++++++++++++----
 intl/loadmsgcat.c |  7 ++++++-
 2 files changed, 24 insertions(+), 5 deletions(-)

(limited to 'intl')

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:
-- 
cgit v1.1