OpenE2K
/
glibc
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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 006dd86111 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.
This commit is contained in:
Carlos O'Donell 2013-05-22 14:50:26 -04:00
parent b50a71810b
commit 7a44c18fb4
4 changed files with 36 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
ChangeLog
View File

@ -1,3 +1,13 @@
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.
2013-05-22 Joseph Myers <joseph@codesourcery.com>
* math/gen-libm-test.pl (parse_args): Do not include expected

4
NEWS
View File

@ -17,8 +17,8 @@ Version 2.18
15085, 15086, 15160, 15214, 15221, 15232, 15234, 15283, 15285, 15287,
15304, 15305, 15307, 15309, 15327, 15330, 15335, 15336, 15337, 15339,
15342, 15346, 15359, 15361, 15366, 15380, 15394, 15395, 15405, 15406,
15409, 15416, 15418, 15419, 15423, 15424, 15426, 15429, 15442, 15448,
15480, 15485, 15488, 15490, 15493, 15497, 15506.
15409, 15416, 15418, 15419, 15423, 15424, 15426, 15429, 15441, 15442,
15448, 15480, 15485, 15488, 15490, 15493, 15497, 15506.
* CVE-2013-0242 Buffer overrun in regexp matcher has been fixed (Bugzilla
#15078).

22
intl/dcigettext.c
View File

@ -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))

7
intl/loadmsgcat.c
View File

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