The fix for 7017193 causes segfaults

[Andrew Haley]

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
   }