6788196: (porting) Bounds checks in io_util.c rely on undefined behaviour

Reviewed-by: alanb
Contributed-by: gbenson@redhat.com

src/share/native/java/io/io_util.c

@@ -58,12 +58,24 @@ readSingle(JNIEnv *env, jobject this, jfieldID fid) {
  */
 #define BUF_SIZE 8192
 
+/*
+ * Returns true if the array slice defined by the given offset and length
+ * is out of bounds.
+ */
+static int
+outOfBounds(JNIEnv *env, jint off, jint len, jbyteArray array) {
+    return ((off < 0) ||
+            (len < 0) ||
+            // We are very careful to avoid signed integer overflow,
+            // the result of which is undefined in C.
+            ((*env)->GetArrayLength(env, array) - off < len));
+}
 
 int
 readBytes(JNIEnv *env, jobject this, jbyteArray bytes,
           jint off, jint len, jfieldID fid)
 {
-    int nread, datalen;
+    int nread;
     char stackBuf[BUF_SIZE];
     char *buf = 0;
     FD fd;
@@ -72,10 +84,8 @@ readBytes(JNIEnv *env, jobject this, jbyteArray bytes,
         JNU_ThrowNullPointerException(env, 0);
         return -1;
     }
-    datalen = (*env)->GetArrayLength(env, bytes);
 
-    if ((off < 0) || (off > datalen) ||
-        (len < 0) || ((off + len) > datalen) || ((off + len) < 0)) {
+    if (outOfBounds(env, off, len, bytes)) {
         JNU_ThrowByName(env, "java/lang/IndexOutOfBoundsException", 0);
         return -1;
     }
@@ -136,7 +146,7 @@ void
 writeBytes(JNIEnv *env, jobject this, jbyteArray bytes,
           jint off, jint len, jfieldID fid)
 {
-    int n, datalen;
+    int n;
     char stackBuf[BUF_SIZE];
     char *buf = 0;
     FD fd;
@@ -145,10 +155,8 @@ writeBytes(JNIEnv *env, jobject this, jbyteArray bytes,
         JNU_ThrowNullPointerException(env, 0);
         return;
     }
-    datalen = (*env)->GetArrayLength(env, bytes);
 
-    if ((off < 0) || (off > datalen) ||
-        (len < 0) || ((off + len) > datalen) || ((off + len) < 0)) {
+    if (outOfBounds(env, off, len, bytes)) {
         JNU_ThrowByName(env, "java/lang/IndexOutOfBoundsException", 0);
         return;
     }

test/java/io/readBytes/ReadBytesBounds.java

@@ -22,107 +22,76 @@
  */
 
 /*
-  @test
-  @bug 4017728 4079849
-  @summary Check for correct Array Bounds check in read of FileInputStream and
-  RandomAccessFile
-  */
+ * @test
+ * @bug 4017728 4079849 6788196
+ * @summary Check for correct Array Bounds check in read of FileInputStream and
+ * RandomAccessFile
+ */
 
 import java.io.*;
 
-
 /*
- * The test calls the read(byte buf[] , int off , int len) of FileInputStream with
- * different values of off and len to see if the ArrayOutOfBoundsException is
- * thrown according to the JLS1.0 specification.  The read(...) method calls
- * readBytes(...) in native code(io_util.c).  The read(...) method in RandomAccessFile
- * also calls the same native method.  So one should see similar results.
+ * The test calls the read(byte buf[] , int off , int len) of
+ * FileInputStream with different values of off and len to see if the
+ * IndexOutOfBoundsException is thrown.  The read(...) method calls
+ * readBytes(...) in native code(io_util.c).  The read(...) method in
+ * RandomAccessFile also calls the same native method.  So one should
+ * see similar results.
  */
 
-
 public class ReadBytesBounds {
 
-    public static void main(String argv[]) throws Exception{
-
-        int num_test_cases = 12;
-        int off[] =     {-1 , -1 ,  0 , 0  , 33 , 33 , 0  , 32 , 32 , 4  , 1  , 0};
-        int len[] =     {-1 ,  0 , -1 , 33 , 0  , 4  , 32 , 0  , 4  , 16 , 31 , 0};
-        boolean results[] = { false ,  false ,  false , false  , false  , false  ,
-                              true  , true  , false  , true  , true  , true};
-
+    static final FileInputStream fis;
+    static final RandomAccessFile raf;
+    static final byte[] b = new byte[32];
+
+    static {
+        try {
+            String dir = System.getProperty("test.src", ".");
+            File testFile = new File(dir, "input.txt");
+            fis = new FileInputStream(testFile);
+            raf = new RandomAccessFile(testFile , "r");
+        } catch (Throwable t) {
+            throw new Error(t);
+        }
+    }
 
-        FileInputStream fis = null;
-        RandomAccessFile raf = null;
+    public static void main(String argv[]) throws Throwable {
         byte b[] = new byte[32];
+        testRead(-1, -1, false);
+        testRead(-1,  0, false);
+        testRead( 0, -1, false);
+        testRead( 0, 33, false);
+        testRead(33,  0, false);
+        testRead(33,  4, false);
+        testRead( 0, 32, true);
+        testRead(32,  0, true);
+        testRead(32,  4, false);
+        testRead( 4, 16, true);
+        testRead( 1, 31, true);
+        testRead( 0,  0, true);
+        testRead(31,  Integer.MAX_VALUE, false);
+        testRead( 0,  Integer.MAX_VALUE, false);
+        testRead(-1,  Integer.MAX_VALUE, false);
+        testRead(-4,  Integer.MIN_VALUE, false);
+        testRead( 0,  Integer.MIN_VALUE, false);
+    }
 
-        int num_good = 0;
-        int num_bad = 0;
-
-        String dir = System.getProperty("test.src", ".");
-        File testFile = new File(dir, "input.txt");
-        fis = new FileInputStream(testFile);
-        for(int i = 0; i < num_test_cases; i++) {
-
-            try {
-                int bytes_read = fis.read(b , off[i] , len[i]);
-            } catch(IndexOutOfBoundsException aiobe) {
-                if (results[i]) {
-                    throw new RuntimeException("Unexpected result");
-                }
-                else {
-                    num_good++;
-                }
-                continue;
-            }
-
-            if (results[i]) {
-                num_good++;
-            }
-            else {
-                throw new RuntimeException("Unexpected result");
-            }
-
+    static void testRead(int off, int len, boolean expected) throws Throwable {
+        System.err.printf("off=%d len=%d expected=%b%n", off, len, expected);
+        boolean result;
+        try {
+            fis.read(b, off, len);
+            raf.read(b, off, len);
+            result = true;
+        } catch (IndexOutOfBoundsException e) {
+            result = false;
         }
-        System.out.println("Results for FileInputStream.read");
-        System.out.println("\nTotal number of test cases = " + num_test_cases +
-                           "\nNumber succeded = " + num_good +
-                           "\nNumber failed   = " + num_bad);
-
-
-
-        num_good = 0;
-        num_bad = 0;
-
-        raf = new RandomAccessFile(testFile , "r");
-        for(int i = 0; i < num_test_cases; i++) {
-
-            try {
-                int bytes_read = raf.read(b , off[i] , len[i]);
-            } catch(IndexOutOfBoundsException aiobe) {
-                if (results[i]) {
-                    throw new RuntimeException("Unexpected result");
-                }
-                else {
-                    num_good++;
-                }
-                continue;
-            }
-
-            if (results[i]) {
-                num_good++;
-            }
-            else {
-                throw new RuntimeException("Unexpected result");
-            }
 
+        if (result != expected) {
+            throw new RuntimeException
+                (String.format("Unexpected result off=%d len=%d expected=%b",
+                               off, len, expected));
         }
-
-        System.out.println("Results for RandomAccessFile.read");
-        System.out.println("\nTotal number of test cases = " + num_test_cases +
-                           "\nNumber succeded = " + num_good +
-                           "\nNumber failed   = " + num_bad);
-
-
     }
-
 }