8227021: VM fails if any sun.boot.library.path paths are longer than JVM_MAXPATHLEN

The size of each path in sun.boot.library.path property should not exceed JVM_MAXPATHLEN

Reviewed-by: dholmes, coleenp, sspitsyn

Author: Adam Farley

src/hotspot/share/runtime/os.cpp

@@ -279,6 +279,19 @@ static bool conc_path_file_and_check(char *buffer, char *printbuffer, size_t pri
   return false;
 }
 
+// Frees all memory allocated on the heap for the
+// supplied array of arrays of chars (a), where n
+// is the number of elements in the array.
+static void free_array_of_char_arrays(char** a, size_t n) {
+      while (n > 0) {
+          n--;
+          if (a[n] != NULL) {
+            FREE_C_HEAP_ARRAY(char, a[n]);
+          }
+      }
+      FREE_C_HEAP_ARRAY(char*, a);
+}
+
 bool os::dll_locate_lib(char *buffer, size_t buflen,
                         const char* pname, const char* fname) {
   bool retval = false;
@@ -299,10 +312,10 @@ bool os::dll_locate_lib(char *buffer, size_t buflen,
       }
     } else if (strchr(pname, *os::path_separator()) != NULL) {
       // A list of paths. Search for the path that contains the library.
-      int n;
-      char** pelements = split_path(pname, &n);
+      size_t n;
+      char** pelements = split_path(pname, &n, fullfnamelen);
       if (pelements != NULL) {
-        for (int i = 0; i < n; i++) {
+        for (size_t i = 0; i < n; i++) {
           char* path = pelements[i];
           // Really shouldn't be NULL, but check can't hurt.
           size_t plen = (path == NULL) ? 0 : strlen(path);
@@ -314,12 +327,7 @@ bool os::dll_locate_lib(char *buffer, size_t buflen,
           if (retval) break;
         }
         // Release the storage allocated by split_path.
-        for (int i = 0; i < n; i++) {
-          if (pelements[i] != NULL) {
-            FREE_C_HEAP_ARRAY(char, pelements[i]);
-          }
-        }
-        FREE_C_HEAP_ARRAY(char*, pelements);
+        free_array_of_char_arrays(pelements, n);
       }
     } else {
       // A definite path.
@@ -1335,17 +1343,22 @@ bool os::set_boot_path(char fileSep, char pathSep) {
   return false;
 }
 
-/*
- * Splits a path, based on its separator, the number of
- * elements is returned back in n.
- * It is the callers responsibility to:
- *   a> check the value of n, and n may be 0.
- *   b> ignore any empty path elements
- *   c> free up the data.
- */
-char** os::split_path(const char* path, int* n) {
-  *n = 0;
-  if (path == NULL || strlen(path) == 0) {
+// Splits a path, based on its separator, the number of
+// elements is returned back in "elements".
+// file_name_length is used as a modifier for each path's
+// length when compared to JVM_MAXPATHLEN. So if you know
+// each returned path will have something appended when
+// in use, you can pass the length of that in
+// file_name_length, to ensure we detect if any path
+// exceeds the maximum path length once prepended onto
+// the sub-path/file name.
+// It is the callers responsibility to:
+//   a> check the value of "elements", which may be 0.
+//   b> ignore any empty path elements
+//   c> free up the data.
+char** os::split_path(const char* path, size_t* elements, size_t file_name_length) {
+  *elements = (size_t)0;
+  if (path == NULL || strlen(path) == 0 || file_name_length == (size_t)NULL) {
     return NULL;
   }
   const char psepchar = *os::path_separator();
@@ -1354,7 +1367,7 @@ char** os::split_path(const char* path, int* n) {
     return NULL;
   }
   strcpy(inpath, path);
-  int count = 1;
+  size_t count = 1;
   char* p = strchr(inpath, psepchar);
   // Get a count of elements to allocate memory
   while (p != NULL) {
@@ -1363,20 +1376,23 @@ char** os::split_path(const char* path, int* n) {
     p = strchr(p, psepchar);
   }
   char** opath = (char**) NEW_C_HEAP_ARRAY(char*, count, mtInternal);
-  if (opath == NULL) {
-    return NULL;
-  }
 
   // do the actual splitting
   p = inpath;
-  for (int i = 0 ; i < count ; i++) {
+  for (size_t i = 0 ; i < count ; i++) {
     size_t len = strcspn(p, os::path_separator());
-    if (len > JVM_MAXPATHLEN) {
-      return NULL;
+    if (len + file_name_length > JVM_MAXPATHLEN) {
+      // release allocated storage before exiting the vm
+      free_array_of_char_arrays(opath, i++);
+      vm_exit_during_initialization("The VM tried to use a path that exceeds the maximum path length for "
+                                    "this system. Review path-containing parameters and properties, such as "
+                                    "sun.boot.library.path, to identify potential sources for this path.");
     }
     // allocate the string and add terminator storage
-    char* s  = (char*)NEW_C_HEAP_ARRAY(char, len + 1, mtInternal);
+    char* s  = (char*)NEW_C_HEAP_ARRAY_RETURN_NULL(char, len + 1, mtInternal);
     if (s == NULL) {
+      // release allocated storage before returning null
+      free_array_of_char_arrays(opath, i++);
       return NULL;
     }
     strncpy(s, p, len);
@@ -1385,7 +1401,7 @@ char** os::split_path(const char* path, int* n) {
     p += len + 1;
   }
   FREE_C_HEAP_ARRAY(char, inpath);
-  *n = count;
+  *elements = count;
   return opath;
 }
 

src/hotspot/share/runtime/os.hpp

@@ -821,7 +821,7 @@ class os: AllStatic {
   // Amount beyond the callee frame size that we bang the stack.
   static int extra_bang_size_in_bytes();
 
-  static char** split_path(const char* path, int* n);
+  static char** split_path(const char* path, size_t* elements, size_t file_name_length);
 
   // support for mapping non-volatile memory using MAP_SYNC
   static bool supports_map_sync();

test/hotspot/jtreg/runtime/LoadLibrary/TestSunBootLibraryPath.java

@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test TestSunBootLibraryPath.java
+ * @bug 8227021
+ * @summary Confirm using too-long paths in sun.boot.library.path causes failure and useful error message.
+ * @author afarley
+ * @library /test/lib
+ * @build jdk.test.lib.process.ProcessTools
+ * @run driver TestSunBootLibraryPath
+ */
+
+import jdk.test.lib.process.ProcessTools;
+
+public class TestSunBootLibraryPath {
+    static String expectedErrorMessage = "The VM tried to use a path that exceeds the maximum path length for this system.";
+
+    public static void main(String[] args) throws Exception {
+        // Allows us to re-use this class as a do-nothing test class simply by passing a "Do-Nothing" argument.
+        if (args.length == 0) {
+            // Grab any path.
+            String tooLongPath = System.getProperty("sun.boot.library.path");
+            // Add enough characters to make it "too long".
+            tooLongPath += "a".repeat(5000);
+            // Start a java process with this property set, and check that:
+            // 1) The process failed and
+            // 2) The error message was correct.
+            ProcessTools.executeTestJvm("-Dsun.boot.library.path=" + tooLongPath,
+                                        "TestSunBootLibraryPath",
+                                        "'Do-Nothing'")
+                                        .shouldNotHaveExitValue(0)
+                                        .stdoutShouldContain(expectedErrorMessage);
+        } else if (!args[0].equals("Do-Nothing")) {
+            // Fail, to prevent accidental args from causing accidental test passing.
+            throw new IllegalArgumentException("Test was launched with an invalid argument.");
+        }
+    }
+}