The fix for 7017193 causes segfaults

[Dmitry Samersoff]

Andrew,

New bug id is 7073913.

I take care of fix integration.

-Dmitry

On 2011-08-01 20:06, Andrew Haley wrote:
> The fix for 7017193 is broken, and we get segfaults in the test case
> Test6929067.sh
>
> http://hg.openjdk.java.net/jdk7/jdk7/hotspot/rev/677234770800
>
> There are two bugs in os::get_line_chars.  One is that we read a char
> from a file into an int variable and we then use the value of that int
> variable.  However, we do not do anything to zero the upper part of
> the int variable, so the test is comparing uninitialized memory.
> The variable should not be an int, but a char.
>
> // line is longer than size of buf, skip to EOL
>    int ch;
>    while (read(fd,&ch, 1) == 1&&  ch != '\n') {
>      // Do nothing
>    }
>
> The second bug is a buffer overflow.  bsize is the size of the buffer,
> which is the number of characters to be read plus a terminating null
> character.  If bsize is 128, no more than 127 characters may be read.
>
> This loop reads the characters until i is (bsize-1), i.e. 127:
>
>   // read until EOF, EOL or buf is full
>    while ((sz = (int) read(fd,&buf[i], 1)) == 1&&  i<  (bsize-1)&&  buf[i] != '\n') {
>       ++i;
>    }
>
> Then later we do:
>
>    buf[i+1] = 0;
>
> This is buf[128]: we have written past the end of the array.
>
> (Despite the fact that this is a buffer overflow, it's not an
> exploitable security hole because there is no way for any attacker to
> control /proc/self/maps.)
>
> This is my proposed fix.  Comments, please.
>
> Andrew.
>
>
> --- os.cpp~     2011-08-01 16:00:06.000000000 +0200
> +++ os.cpp      2011-08-01 17:49:52.000000000 +0200
> @@ -1301,7 +1301,7 @@
>     size_t sz, i = 0;
>
>     // read until EOF, EOL or buf is full
> -  while ((sz = (int) read(fd,&buf[i], 1)) == 1&&  i<  (bsize-1)&&  buf[i] != '\n') {
> +  while ((sz = (int) read(fd,&buf[i], 1)) == 1&&  i<  (bsize-2)&&  buf[i] != '\n') {
>        ++i;
>     }
>
> @@ -1322,7 +1322,7 @@
>     }
>
>     // line is longer than size of buf, skip to EOL
> -  int ch;
> +  char ch;
>     while (read(fd,&ch, 1) == 1&&  ch != '\n') {
>       // Do nothing
>     }


-- 
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...