changeset 12447:de2b914558cd

8162998: Check at module definition that a package named java is not being defined to a class loader other than the boot or platform loader Summary: Added new prohibited package check to JVM_DefineModule and JVM_AddModulePackage Reviewed-by: hseigel, mchung
author lfoltan
date Thu, 15 Sep 2016 11:25:43 -0400
parents 8b9a67a9fdb6
children 89d191b6c6ee
files src/share/vm/classfile/moduleEntry.hpp src/share/vm/classfile/modules.cpp src/share/vm/oops/instanceKlass.cpp test/runtime/modules/JVMAddModulePackage.java test/runtime/modules/JVMDefineModule.java
diffstat 5 files changed, 107 insertions(+), 20 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/classfile/moduleEntry.hpp	Wed Sep 14 14:30:39 2016 -0700
+++ b/src/share/vm/classfile/moduleEntry.hpp	Thu Sep 15 11:25:43 2016 -0400
@@ -36,6 +36,8 @@
 #include "utilities/ostream.hpp"
 
 #define UNNAMED_MODULE "Unnamed Module"
+#define JAVAPKG "java/"
+#define JAVAPKG_LEN 5
 
 class ModuleClosure;
 
--- a/src/share/vm/classfile/modules.cpp	Wed Sep 14 14:30:39 2016 -0700
+++ b/src/share/vm/classfile/modules.cpp	Thu Sep 15 11:25:43 2016 -0400
@@ -34,6 +34,7 @@
 #include "classfile/packageEntry.hpp"
 #include "classfile/stringTable.hpp"
 #include "classfile/symbolTable.hpp"
+#include "classfile/systemDictionary.hpp"
 #include "classfile/vmSymbols.hpp"
 #include "logging/log.hpp"
 #include "memory/resourceArea.hpp"
@@ -44,6 +45,7 @@
 #include "runtime/handles.inline.hpp"
 #include "runtime/javaCalls.hpp"
 #include "runtime/reflection.hpp"
+#include "utilities/stringUtils.hpp"
 #include "utilities/utf8.hpp"
 
 static bool verify_module_name(char *module_name) {
@@ -287,6 +289,14 @@
 
   const char* module_version = get_module_version(version);
 
+  oop loader = java_lang_reflect_Module::loader(module_handle());
+  // Make sure loader is not the jdk.internal.reflect.DelegatingClassLoader.
+  if (loader != java_lang_ClassLoader::non_reflection_class_loader(loader)) {
+    THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
+              "Class loader is an invalid delegating class loader");
+  }
+  Handle h_loader = Handle(THREAD, loader);
+
   objArrayOop packages_oop = objArrayOop(JNIHandles::resolve(packages));
   objArrayHandle packages_h(THREAD, packages_oop);
   int num_packages = (packages_h == NULL ? 0 : packages_h->length());
@@ -307,6 +317,21 @@
                 err_msg("Invalid package name: %s for module: %s",
                         package_name, module_name));
     }
+
+    // Only modules defined to either the boot or platform class loader, can define a "java/" package.
+    if (!h_loader.is_null() &&
+        !SystemDictionary::is_platform_class_loader(h_loader) &&
+        strncmp(package_name, JAVAPKG, JAVAPKG_LEN) == 0) {
+      const char* class_loader_name = SystemDictionary::loader_name(h_loader());
+      StringUtils::replace_no_expand(package_name, "/", ".");
+      const char* msg_text1 = "Class loader (instance of): ";
+      const char* msg_text2 = " tried to define prohibited package name: ";
+      size_t len = strlen(msg_text1) + strlen(class_loader_name) + strlen(msg_text2) + strlen(package_name) + 1;
+      char* message = NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, char, len);
+      jio_snprintf(message, len, "%s%s%s%s", msg_text1, class_loader_name, msg_text2, package_name);
+      THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), message);
+    }
+
     Symbol* pkg_symbol = SymbolTable::new_symbol(package_name, CHECK);
     // append_if_missing() returns FALSE if entry already exists.
     if (!pkg_list->append_if_missing(pkg_symbol)) {
@@ -316,20 +341,6 @@
     }
   }
 
-  oop loader = java_lang_reflect_Module::loader(module_handle());
-  // Make sure loader is not the sun.reflect.DelegatingClassLoader.
-  if (loader != java_lang_ClassLoader::non_reflection_class_loader(loader)) {
-    THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
-              "Class loader is an invalid delegating class loader");
-  }
-  Handle h_loader = Handle(THREAD, loader);
-
-  // Check that loader is a subclass of java.lang.ClassLoader.
-  if (loader != NULL && !java_lang_ClassLoader::is_subclass(h_loader->klass())) {
-    THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
-              "Class loader is not a subclass of java.lang.ClassLoader");
-  }
-
   ModuleEntryTable* module_table = get_module_entry_table(h_loader, CHECK);
   assert(module_table != NULL, "module entry table shouldn't be null");
 
@@ -857,11 +868,27 @@
               err_msg("Invalid package name: %s", package_name));
   }
 
+  ClassLoaderData *loader_data = module_entry->loader_data();
+
+  // Only modules defined to either the boot or platform class loader, can define a "java/" package.
+  if (!loader_data->is_the_null_class_loader_data() &&
+      !loader_data->is_platform_class_loader_data() &&
+      strncmp(package_name, JAVAPKG, JAVAPKG_LEN) == 0) {
+    const char* class_loader_name = SystemDictionary::loader_name(loader_data);
+    StringUtils::replace_no_expand(package_name, "/", ".");
+    const char* msg_text1 = "Class loader (instance of): ";
+    const char* msg_text2 = " tried to define prohibited package name: ";
+    size_t len = strlen(msg_text1) + strlen(class_loader_name) + strlen(msg_text2) + strlen(package_name) + 1;
+    char* message = NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, char, len);
+    jio_snprintf(message, len, "%s%s%s%s", msg_text1, class_loader_name, msg_text2, package_name);
+    THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), message);
+  }
+
   log_debug(modules)("add_module_package(): Adding package %s to module %s",
                      package_name, module_entry->name()->as_C_string());
 
   TempNewSymbol pkg_symbol = SymbolTable::new_symbol(package_name, CHECK);
-  PackageEntryTable* package_table = module_entry->loader_data()->packages();
+  PackageEntryTable* package_table = loader_data->packages();
   assert(package_table != NULL, "Missing package_table");
 
   bool pkg_exists = false;
--- a/src/share/vm/oops/instanceKlass.cpp	Wed Sep 14 14:30:39 2016 -0700
+++ b/src/share/vm/oops/instanceKlass.cpp	Thu Sep 15 11:25:43 2016 -0400
@@ -26,6 +26,7 @@
 #include "classfile/classFileParser.hpp"
 #include "classfile/classFileStream.hpp"
 #include "classfile/javaClasses.hpp"
+#include "classfile/moduleEntry.hpp"
 #include "classfile/systemDictionary.hpp"
 #include "classfile/systemDictionaryShared.hpp"
 #include "classfile/verifier.hpp"
@@ -2384,18 +2385,17 @@
 
 // Only boot and platform class loaders can define classes in "java/" packages.
 void InstanceKlass::check_prohibited_package(Symbol* class_name,
-                                                Handle class_loader,
-                                                TRAPS) {
-  const char* javapkg = "java/";
+                                             Handle class_loader,
+                                             TRAPS) {
   ResourceMark rm(THREAD);
   if (!class_loader.is_null() &&
       !SystemDictionary::is_platform_class_loader(class_loader) &&
       class_name != NULL &&
-      strncmp(class_name->as_C_string(), javapkg, strlen(javapkg)) == 0) {
+      strncmp(class_name->as_C_string(), JAVAPKG, JAVAPKG_LEN) == 0) {
     TempNewSymbol pkg_name = InstanceKlass::package_from_name(class_name, CHECK);
     assert(pkg_name != NULL, "Error in parsing package name starting with 'java/'");
     char* name = pkg_name->as_C_string();
-    const char* class_loader_name = InstanceKlass::cast(class_loader()->klass())->name()->as_C_string();
+    const char* class_loader_name = SystemDictionary::loader_name(class_loader());
     StringUtils::replace_no_expand(name, "/", ".");
     const char* msg_text1 = "Class loader (instance of): ";
     const char* msg_text2 = " tried to load prohibited package name: ";
--- a/test/runtime/modules/JVMAddModulePackage.java	Wed Sep 14 14:30:39 2016 -0700
+++ b/test/runtime/modules/JVMAddModulePackage.java	Thu Sep 15 11:25:43 2016 -0400
@@ -33,6 +33,7 @@
  */
 
 import static jdk.test.lib.Asserts.*;
+import java.sql.Time;
 
 public class JVMAddModulePackage {
 
@@ -123,6 +124,29 @@
             // Expected
         }
 
+        // Add package named "java" to an module defined to a class loader other than the boot or platform loader.
+        try {
+            // module1 is defined to a MyClassLoader class loader.
+            ModuleHelper.AddModulePackage(module1, "java/foo");
+            throw new RuntimeException("Failed to get the expected IAE");
+        } catch(IllegalArgumentException e) {
+            if (!e.getMessage().contains("prohibited package name")) {
+              throw new RuntimeException("Failed to get expected IAE message for prohibited package name: " + e.getMessage());
+            }
+        }
+
+        // Package "javabar" should be ok
+        ModuleHelper.AddModulePackage(module1, "javabar");
+
+        // Package named "java" defined to the boot class loader, should be ok
+        Object module_javabase = module1.getClass().getModule();
+        ModuleHelper.AddModulePackage(module_javabase, "java/foo");
+
+        // Package named "java" defined to the platform class loader, should be ok
+        // The module java.sql is defined to the platform class loader.
+        java.sql.Time jst = new java.sql.Time(45000); // milliseconds
+        Object module_javasql = jst.getClass().getModule();
+        ModuleHelper.AddModulePackage(module_javasql, "java/foo");
     }
 
     static class MyClassLoader extends ClassLoader { }
--- a/test/runtime/modules/JVMDefineModule.java	Wed Sep 14 14:30:39 2016 -0700
+++ b/test/runtime/modules/JVMDefineModule.java	Thu Sep 15 11:25:43 2016 -0400
@@ -33,6 +33,7 @@
  */
 
 import static jdk.test.lib.Asserts.*;
+import java.sql.Time;
 
 public class JVMDefineModule {
 
@@ -238,6 +239,39 @@
             }
         }
 
+        // Package named "java" defined to a class loader other than the boot or platform class loader, expect an IAE
+        m = ModuleHelper.ModuleObject("modulejavapkg1", cl, new String[] { "java/foo" });
+        try {
+            // module m is defined to an instance of MyClassLoader class loader
+            ModuleHelper.DefineModule(m, "9.0", "modulejavapkg1", new String[] { "java/foo" });
+            throw new RuntimeException("Failed to get expected IAE for package java/foo");
+        } catch(IllegalArgumentException e) {
+            if (!e.getMessage().contains("prohibited package name")) {
+              throw new RuntimeException("Failed to get expected IAE message for prohibited package name: " + e.getMessage());
+            }
+        }
+
+        // Package named "javabar" defined to a class loader other than the boot or platform class loader, should be ok
+        m = ModuleHelper.ModuleObject("modulejavapkg2", cl, new String[] { "javabar" });
+        assertNotNull(m, "Module should not be null");
+        ModuleHelper.DefineModule(m, "9.0", "modulejavapkg2", new String[] { "javabar" });
+
+        // Package named "java" defined to the boot class loader, should be ok
+        //   m's type is a java.lang.Object, module is java.base
+        //   java.base module is defined to the boot loader
+        ClassLoader boot_loader = m.getClass().getClassLoader();
+        m = ModuleHelper.ModuleObject("modulejavapkg3", boot_loader, new String[] { "java/foo" });
+        assertNotNull(m, "Module should not be null");
+        ModuleHelper.DefineModule(m, "9.0", "modulejavapkg3", new String[] { "java/foo" });
+
+        // Package named "java" defined to the platform class loader, should be ok
+        //   java.sql module defined to the platform class loader.
+        java.sql.Time jst = new java.sql.Time(45 * 1000);
+        ClassLoader platform_loader = jst.getClass().getClassLoader();
+        m = ModuleHelper.ModuleObject("modulejavapkg4", platform_loader, new String[] { "java/foo" });
+        assertNotNull(m, "Module should not be null");
+        ModuleHelper.DefineModule(m, "9.0", "modulejavapkg4", new String[] { "java/foo" });
+
         // module version that is null, should be okay
         m = ModuleHelper.ModuleObject("module8", cl, new String[] { "a_package_8" });
         assertNotNull(m, "Module should not be null");