Request for approval: Bug 100057 (original) (raw)

Request for approval: Bug 100057 - Potential stack corruption in GetJavaProperties

Andrew Haley aph at redhat.com
Fri May 22 17:40:03 UTC 2009

Previous message: Request for approval: Bug 100057 - Potential stack corruption in GetJavaProperties
Next message: Request for approval: Bug 100057 - Potential stack corruption in GetJavaProperties
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Alan Bateman wrote:

Andrew Haley wrote:

: I've reworked the patch.

I tried to refactor the code to make it cleaner and easier to follow, but I ended up touching so many places that it bacame a major change, and there's no way I could possibly test it all, especially the special cases for Solaris. So, I backed off and produced this, the minimum patch. encodingvariant and temp are malloc()d to the size that they need, and temp is realloc()d, as you suggest. This looks much better except the size when allocating encodingvariant. If I read the patch correctly then encodingvariant is malloc'ed before 'p' is initialized (or 'p' might actually be null in the Solaris case when not a euro locale). Do I have this right?

Yes, you're right. :-(

I reworked this silly patch so many times that I couldn't see it any more. As you say, encoding_variant needs to be at least the size of temp.

strlen(temp)+1 would work but would allocate a bit more than you need. A minor nit is that a malloc failure will return without freeing temp but I wouldn't worry about this because this is called once during> startup and we won't get too far if heap is completely exhausted.

Thanks for catching this.

Andrew.

diff -r 238591c80bc5 src/solaris/native/java/lang/java_props_md.c --- a/src/solaris/native/java/lang/java_props_md.c Thu May 21 18:41:50 2009 +0100 +++ b/src/solaris/native/java/lang/java_props_md.c Fri May 22 18:38:21 2009 +0100 @@ -211,14 +211,19 @@ * _.@ * , , and are optional. */

       char temp[64];
       char *language = NULL, *country = NULL, *variant = NULL,
            *encoding = NULL;
       char *std_language = NULL, *std_country = NULL, *std_variant = NULL,
            *std_encoding = NULL;

```
       char *p, encoding_variant[64];
```

       char *p, *encoding_variant;
       int i, found;

       char *temp = malloc(strlen(lc)+1);

```
       if (temp == NULL) {
```

           JNU_ThrowOutOfMemoryError(env, NULL);

```
           return NULL;
```
```
       }
```
#ifndef linux /* * Workaround for Solaris bug 4201684: Xlib doesn't like @euro

@@ -252,6 +257,13 @@ * to a default country if that's possible. It's also used to map * the Solaris locale aliases to their proper Java locale IDs. */ +

       encoding_variant = malloc(strlen(temp)+1);

```
       if (encoding_variant == NULL) {
```

           JNU_ThrowOutOfMemoryError(env, NULL);

```
           return NULL;
```
```
       }
```

        if ((p = strchr(temp, '.')) != NULL) {
            strcpy(encoding_variant, p); /* Copy the leading '.' */
            *p = '\0';

@@ -263,7 +275,12 @@ }

         if (mapLookup(locale_aliases, temp, &p)) {

```
           strcpy(temp, p);
```

           temp = realloc(temp, strlen(p)+1);

```
           if (temp == NULL) {
```

               JNU_ThrowOutOfMemoryError(env, NULL);

```
               return NULL;
```
```
           }
```

  strcpy(temp, p);
        }

        language = temp;

@@ -357,6 +374,9 @@ #endif sprops.encoding = std_encoding; sprops.sun_jnu_encoding = sprops.encoding; +

```
       free(temp);
```
```
       free(encoding_variant);
   }
```
}

Previous message: Request for approval: Bug 100057 - Potential stack corruption in GetJavaProperties
Next message: Request for approval: Bug 100057 - Potential stack corruption in GetJavaProperties
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

More information about the core-libs-dev mailing list