JDK-7073913 : The fix for 7017193 causes segfaults
Type:Bug
Component:hotspot
Sub-Component:runtime
Affected Version:hs21
Priority:P4
Status:Closed
Resolution:Fixed
OS:linux
CPU:x86
Submitted:2011-08-02
Updated:2011-11-25
Resolved:2011-09-30
The Version table provides details related to the release that this issue/RFE will be addressed.
Unresolved : Release in which this issue/RFE will be addressed. Resolved: Release in which this issue/RFE has been resolved. Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.
EVALUATION
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. Other one is a buffer overflow.
see comments for details.
04-08-2011
SUGGESTED FIX
--- 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
}
04-08-2011
PUBLIC COMMENTS
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.)