Request for approval: Bug 100057 - Potential stack corruption in GetJavaProperties
David Holmes - Sun Microsystems
David.Holmes at Sun.COM
Fri May 22 11:58:35 UTC 2009
Andrew Haley said the following on 05/22/09 21:45:
> David Holmes - Sun Microsystems wrote:
>> If you use malloc then you have to check for a NULL return and deal with
>> the error possibility.
>> Alternatively use strncpy to make sure it's safe and continue to assume
>> that it will be big enough.
> It's just following the style used throughout that file, which is to
> allocate memory (usually via strdup) but not check the retval.
Hmmm. In most (all?) cases though that just results in a NULL being
stored in a property. Here if we get NULL we try to write into that
space - and that's not good. Can't say for sure all the strdup uses are
The risk of fixing old code is that suddenly people notice everything
else that's wrong with it.
> OK. I'll check for the NULL, then. If I have to change the patch that's
> been in IcedTea for ages then I'll use strdup instead of malloc.
I do wonder why strdup wasn't used in the first place.
It also appears that the strdup of lc is never freed.
> But what
> is one supposed to do if the allocation fails? Simply emit an error
> message to stderr and call abort() ?
Throw an OutOfMemory exception - see the use of JNU_ThrowByName later in
>> Andrew Haley said the following on 05/22/09 21:10:
>>> GetJavaProperties has a stack-allocated fixed size buffer for holding
>>> a copy of
>>> a string returned by setlocale(3). However, there is no guarantee
>>> that the
>>> string will fit into this buffer.
>>> This one is probably due to Solaris code being reused for Linux. The
>>> patch has been in IcedTea for a long while.
>>> OK to push, OpenJDK 7 and 6?
>>> 2008-08-28 04:15:51.000000000 -0400
>>> +++ openjdk/jdk/src/solaris/native/java/lang/java_props_md.c
>>> 2008-09-15 10:37:26.000000000 -0400
>>> @@ -211,7 +211,9 @@
>>> * <language name>_<country name>.<encoding
>>> name>@<variant name>
>>> * <country name>, <encoding name>, and <variant name>
>>> are optional.
>>> - char temp;
>>> + char * temp;
>>> + temp = (char*) malloc(strlen(lc)+1);
>>> char *language = NULL, *country = NULL, *variant = NULL,
>>> *encoding = NULL;
>>> char *std_language = NULL, *std_country = NULL,
>>> *std_variant = NULL,
>>> @@ -323,6 +325,9 @@
>>> /* return same result nl_langinfo would return for en_UK,
>>> * in order to use optimizations. */
>>> std_encoding = (*p != '\0') ? p : "ISO8859-1";
>>> + /* Free temp */
>>> + free(temp);
>>> #ifdef __linux__
More information about the core-libs-dev