8162340: Better class stream parsing
Summary: Check platform and boot loader for java/* packages
Reviewed-by: lfoltan, coleenp, dholmes
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp Wed Jul 27 08:31:48 2016 -0400
@@ -5406,7 +5406,6 @@
Symbol* name,
ClassLoaderData* loader_data,
Handle protection_domain,
- TempNewSymbol* parsed_name,
const Klass* host_klass,
GrowableArray<Handle>* cp_patches,
Publicity pub_level,
@@ -5416,7 +5415,6 @@
_loader_data(loader_data),
_host_klass(host_klass),
_cp_patches(cp_patches),
- _parsed_name(parsed_name),
_super_klass(),
_cp(NULL),
_fields(NULL),
@@ -5657,15 +5655,6 @@
Symbol* const class_name_in_cp = cp->klass_name_at(_this_class_index);
assert(class_name_in_cp != NULL, "class_name can't be null");
- if (_parsed_name != NULL) {
- // It's important to set parsed_name *before* resolving the super class.
- // (it's used for cleanup by the caller if parsing fails)
- *_parsed_name = class_name_in_cp;
- // parsed_name is returned and can be used if there's an error, so add to
- // its reference count. Caller will decrement the refcount.
- (*_parsed_name)->increment_refcount();
- }
-
// Update _class_name which could be null previously
// to reflect the name in the constant pool
_class_name = class_name_in_cp;
@@ -5692,6 +5681,10 @@
return;
}
+ // Verification prevents us from creating names with dots in them, this
+ // asserts that that's the case.
+ assert(is_internal_format(_class_name), "external class name format used internally");
+
if (!is_internal()) {
if (log_is_enabled(Debug, class, preorder)){
ResourceMark rm(THREAD);
@@ -5900,3 +5893,20 @@
return _stream->clone();
}
+// ----------------------------------------------------------------------------
+// debugging
+
+#ifdef ASSERT
+
+// return true if class_name contains no '.' (internal format is '/')
+bool ClassFileParser::is_internal_format(Symbol* class_name) {
+ if (class_name != NULL) {
+ ResourceMark rm;
+ char* name = class_name->as_C_string();
+ return strchr(name, '.') == NULL;
+ } else {
+ return true;
+ }
+}
+
+#endif
--- a/hotspot/src/share/vm/classfile/classFileParser.hpp Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/classFileParser.hpp Wed Jul 27 08:31:48 2016 -0400
@@ -81,7 +81,6 @@
mutable ClassLoaderData* _loader_data;
const Klass* _host_klass;
GrowableArray<Handle>* _cp_patches; // overrides for CP entries
- TempNewSymbol* _parsed_name;
// Metadata created before the instance klass is created. Must be deallocated
// if not transferred to the InstanceKlass upon successful class loading
@@ -475,7 +474,6 @@
Symbol* name,
ClassLoaderData* loader_data,
Handle protection_domain,
- TempNewSymbol* parsed_name,
const Klass* host_klass,
GrowableArray<Handle>* cp_patches,
Publicity pub_level,
@@ -514,6 +512,11 @@
bool is_internal() const { return INTERNAL == _pub_level; }
static bool verify_unqualified_name(const char* name, unsigned int length, int type);
+
+#ifdef ASSERT
+ static bool is_internal_format(Symbol* class_name);
+#endif
+
};
#endif // SHARE_VM_CLASSFILE_CLASSFILEPARSER_HPP
--- a/hotspot/src/share/vm/classfile/classLoader.cpp Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/classLoader.cpp Wed Jul 27 08:31:48 2016 -0400
@@ -1503,7 +1503,6 @@
protection_domain,
NULL, // host_klass
NULL, // cp_patches
- NULL, // parsed_name
THREAD);
if (HAS_PENDING_EXCEPTION) {
if (DumpSharedSpaces) {
--- a/hotspot/src/share/vm/classfile/klassFactory.cpp Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/klassFactory.cpp Wed Jul 27 08:31:48 2016 -0400
@@ -96,7 +96,6 @@
Handle protection_domain,
const Klass* host_klass,
GrowableArray<Handle>* cp_patches,
- TempNewSymbol* parsed_name,
TRAPS) {
assert(stream != NULL, "invariant");
@@ -123,7 +122,6 @@
name,
loader_data,
protection_domain,
- parsed_name,
host_klass,
cp_patches,
ClassFileParser::BROADCAST, // publicity level
--- a/hotspot/src/share/vm/classfile/klassFactory.hpp Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/klassFactory.hpp Wed Jul 27 08:31:48 2016 -0400
@@ -1,5 +1,5 @@
/*
-* Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+* Copyright (c) 2015, 2016, 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
@@ -74,7 +74,6 @@
Handle protection_domain,
const Klass* host_klass,
GrowableArray<Handle>* cp_patches,
- TempNewSymbol* parsed_name,
TRAPS);
};
--- a/hotspot/src/share/vm/classfile/systemDictionary.cpp Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/systemDictionary.cpp Wed Jul 27 08:31:48 2016 -0400
@@ -70,7 +70,6 @@
#include "services/threadService.hpp"
#include "trace/traceMacros.hpp"
#include "utilities/macros.hpp"
-#include "utilities/stringUtils.hpp"
#include "utilities/ticks.hpp"
#if INCLUDE_CDS
#include "classfile/sharedClassUtil.hpp"
@@ -140,24 +139,6 @@
}
// ----------------------------------------------------------------------------
-// debugging
-
-#ifdef ASSERT
-
-// return true if class_name contains no '.' (internal format is '/')
-bool SystemDictionary::is_internal_format(Symbol* class_name) {
- if (class_name != NULL) {
- ResourceMark rm;
- char* name = class_name->as_C_string();
- return strchr(name, '.') == NULL;
- } else {
- return true;
- }
-}
-
-#endif
-
-// ----------------------------------------------------------------------------
// Parallel class loading check
bool SystemDictionary::is_parallelCapable(Handle class_loader) {
@@ -335,6 +316,10 @@
// Must be called, even if superclass is null, since this is
// where the placeholder entry is created which claims this
// thread is loading this class/classloader.
+// Be careful when modifying this code: once you have run
+// placeholders()->find_and_add(PlaceholderTable::LOAD_SUPER),
+// you need to find_and_remove it before returning.
+// So be careful to not exit with a CHECK_ macro betweeen these calls.
Klass* SystemDictionary::resolve_super_or_fail(Symbol* child_name,
Symbol* class_name,
Handle class_loader,
@@ -399,6 +384,7 @@
}
}
if (!throw_circularity_error) {
+ // Be careful not to exit resolve_super
PlaceholderEntry* newprobe = placeholders()->find_and_add(p_index, p_hash, child_name, loader_data, PlaceholderTable::LOAD_SUPER, class_name, THREAD);
}
}
@@ -669,7 +655,10 @@
#endif // INCLUDE_TRACE
}
-
+// Be careful when modifying this code: once you have run
+// placeholders()->find_and_add(PlaceholderTable::LOAD_INSTANCE),
+// you need to find_and_remove it before returning.
+// So be careful to not exit with a CHECK_ macro betweeen these calls.
Klass* SystemDictionary::resolve_instance_class_or_null(Symbol* name,
Handle class_loader,
Handle protection_domain,
@@ -1031,8 +1020,9 @@
}
// Note: this method is much like resolve_from_stream, but
-// updates no supplemental data structures.
-// TODO consolidate the two methods with a helper routine?
+// does not publish the classes via the SystemDictionary.
+// Handles unsafe_DefineAnonymousClass and redefineclasses
+// RedefinedClasses do not add to the class hierarchy
Klass* SystemDictionary::parse_stream(Symbol* class_name,
Handle class_loader,
Handle protection_domain,
@@ -1069,8 +1059,7 @@
protection_domain,
host_klass,
cp_patches,
- NULL, // parsed_name
- THREAD);
+ CHECK_NULL);
if (host_klass != NULL && k.not_null()) {
// If it's anonymous, initialize it now, since nobody else will.
@@ -1141,8 +1130,6 @@
// already be present in the SystemDictionary, otherwise we would not
// throw potential ClassFormatErrors.
//
- // Note: "parsed_name" is updated.
- TempNewSymbol parsed_name = NULL;
instanceKlassHandle k;
@@ -1154,9 +1141,7 @@
CHECK_NULL);
#endif
- if (k.not_null()) {
- parsed_name = k->name();
- } else {
+ if (k.is_null()) {
if (st->buffer() == NULL) {
return NULL;
}
@@ -1166,64 +1151,28 @@
protection_domain,
NULL, // host_klass
NULL, // cp_patches
- &parsed_name,
- THREAD);
+ CHECK_NULL);
}
- const char* pkg = "java/";
- if (!HAS_PENDING_EXCEPTION &&
- !class_loader.is_null() &&
- !SystemDictionary::is_platform_class_loader(class_loader) &&
- parsed_name != NULL &&
- !strncmp((const char*)parsed_name->bytes(), pkg, strlen(pkg))) {
- // It is illegal to define classes in the "java." package from
- // JVM_DefineClass or jni_DefineClass unless you're the bootclassloader
- ResourceMark rm(THREAD);
- TempNewSymbol pkg_name = InstanceKlass::package_from_name(parsed_name, CHECK_NULL);
- assert(pkg_name != NULL, "Error in parsing package name starting with 'java/'");
- char* name = pkg_name->as_C_string();
- StringUtils::replace_no_expand(name, "/", ".");
- const char* msg_text = "Prohibited package name: ";
- size_t len = strlen(msg_text) + strlen(name) + 1;
- char* message = NEW_RESOURCE_ARRAY(char, len);
- jio_snprintf(message, len, "%s%s", msg_text, name);
- Exceptions::_throw_msg(THREAD_AND_LOCATION,
- vmSymbols::java_lang_SecurityException(), message);
- }
+ assert(k.not_null(), "no klass created");
+ Symbol* h_name = k->name();
+ assert(class_name == NULL || class_name == h_name, "name mismatch");
- if (!HAS_PENDING_EXCEPTION) {
- assert(parsed_name != NULL, "Sanity");
- assert(class_name == NULL || class_name == parsed_name, "name mismatch");
- // Verification prevents us from creating names with dots in them, this
- // asserts that that's the case.
- assert(is_internal_format(parsed_name),
- "external class name format used internally");
-
- // Add class just loaded
- // If a class loader supports parallel classloading handle parallel define requests
- // find_or_define_instance_class may return a different InstanceKlass
- if (is_parallelCapable(class_loader)) {
- k = find_or_define_instance_class(class_name, class_loader, k, THREAD);
- } else {
- define_instance_class(k, THREAD);
- }
+ // Add class just loaded
+ // If a class loader supports parallel classloading handle parallel define requests
+ // find_or_define_instance_class may return a different InstanceKlass
+ if (is_parallelCapable(class_loader)) {
+ k = find_or_define_instance_class(h_name, class_loader, k, CHECK_NULL);
+ } else {
+ define_instance_class(k, CHECK_NULL);
}
// Make sure we have an entry in the SystemDictionary on success
debug_only( {
- if (!HAS_PENDING_EXCEPTION) {
- assert(parsed_name != NULL, "parsed_name is still null?");
- Symbol* h_name = k->name();
- ClassLoaderData *defining_loader_data = k->class_loader_data();
-
- MutexLocker mu(SystemDictionary_lock, THREAD);
+ MutexLocker mu(SystemDictionary_lock, THREAD);
- Klass* check = find_class(parsed_name, loader_data);
- assert(check == k(), "should be present in the dictionary");
-
- Klass* check2 = find_class(h_name, defining_loader_data);
- assert(check == check2, "name inconsistancy in SystemDictionary");
- }
+ Klass* check = find_class(h_name, k->class_loader_data());
+ assert(check == k(), "should be present in the dictionary");
} );
return k();
@@ -1425,6 +1374,8 @@
Handle lockObject = compute_loader_lock_object(class_loader, THREAD);
check_loader_lock_contention(lockObject, THREAD);
ObjectLocker ol(lockObject, THREAD, true);
+ // prohibited package check assumes all classes loaded from archive call
+ // restore_unshareable_info which calls ik->set_package()
ik->restore_unshareable_info(loader_data, protection_domain, CHECK_(nh));
}
@@ -1710,6 +1661,10 @@
// findClass(), i.e. FindLoadedClass/DefineClassIfAbsent or they
// potentially waste time reading and parsing the bytestream.
// Note: VM callers should ensure consistency of k/class_name,class_loader
+// Be careful when modifying this code: once you have run
+// placeholders()->find_and_add(PlaceholderTable::DEFINE_CLASS),
+// you need to find_and_remove it before returning.
+// So be careful to not exit with a CHECK_ macro betweeen these calls.
instanceKlassHandle SystemDictionary::find_or_define_instance_class(Symbol* class_name, Handle class_loader, instanceKlassHandle k, TRAPS) {
instanceKlassHandle nh = instanceKlassHandle(); // null Handle
--- a/hotspot/src/share/vm/classfile/systemDictionary.hpp Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/classfile/systemDictionary.hpp Wed Jul 27 08:31:48 2016 -0400
@@ -281,6 +281,7 @@
// Parse new stream. This won't update the system dictionary or
// class hierarchy, simply parse the stream. Used by JVMTI RedefineClasses.
+ // Also used by Unsafe_DefineAnonymousClass
static Klass* parse_stream(Symbol* class_name,
Handle class_loader,
Handle protection_domain,
@@ -413,10 +414,6 @@
// Verification
static void verify();
-#ifdef ASSERT
- static bool is_internal_format(Symbol* class_name);
-#endif
-
// Initialization
static void initialize(TRAPS);
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp Wed Jul 27 08:31:48 2016 -0400
@@ -67,6 +67,7 @@
#include "services/threadService.hpp"
#include "utilities/dtrace.hpp"
#include "utilities/macros.hpp"
+#include "utilities/stringUtils.hpp"
#include "logging/log.hpp"
#ifdef COMPILER1
#include "c1/c1_Compiler.hpp"
@@ -2225,9 +2226,14 @@
}
void InstanceKlass::set_package(ClassLoaderData* loader_data, TRAPS) {
+
+ // ensure java/ packages only loaded by boot or platform builtin loaders
+ check_prohibited_package(name(), loader_data->class_loader(), CHECK);
+
TempNewSymbol pkg_name = package_from_name(name(), CHECK);
if (pkg_name != NULL && loader_data != NULL) {
+
// Find in class loader's package entry table.
_package_entry = loader_data->packages()->lookup_only(pkg_name);
@@ -2376,6 +2382,31 @@
}
*/
+// 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/";
+ 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) {
+ 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();
+ StringUtils::replace_no_expand(name, "/", ".");
+ const char* msg_text1 = "Class loader (instance of): ";
+ const char* msg_text2 = " tried to load prohibited package name: ";
+ size_t len = strlen(msg_text1) + strlen(class_loader_name) + strlen(msg_text2) + strlen(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, name);
+ THROW_MSG(vmSymbols::java_lang_SecurityException(), message);
+ }
+ return;
+}
+
// tell if two classes have the same enclosing class (at package level)
bool InstanceKlass::is_same_package_member_impl(const InstanceKlass* class1,
const Klass* class2,
--- a/hotspot/src/share/vm/oops/instanceKlass.hpp Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/oops/instanceKlass.hpp Wed Jul 27 08:31:48 2016 -0400
@@ -470,6 +470,12 @@
static bool find_inner_classes_attr(instanceKlassHandle k,
int* ooff, int* noff, TRAPS);
+ private:
+ // Check prohibited package ("java/" only loadable by boot or platform loaders)
+ static void check_prohibited_package(Symbol* class_name,
+ Handle class_loader,
+ TRAPS);
+ public:
// tell if two classes have the same enclosing class (at package level)
bool is_same_package_member(const Klass* class2, TRAPS) const {
return is_same_package_member_impl(this, class2, THREAD);
--- a/hotspot/src/share/vm/oops/symbol.hpp Tue Jul 26 10:29:27 2016 -0400
+++ b/hotspot/src/share/vm/oops/symbol.hpp Wed Jul 27 08:31:48 2016 -0400
@@ -91,12 +91,6 @@
// The allocation (or lookup) of K increments the reference count for K
// and the destructor decrements the reference count.
//
-// Another example of TempNewSymbol usage is parsed_name used in
-// ClassFileParser::parseClassFile() where parsed_name is used in the cleanup
-// after a failed attempt to load a class. Here parsed_name is a
-// TempNewSymbol (passed in as a parameter) so the reference count on its symbol
-// will be decremented when it goes out of scope.
-
// This cannot be inherited from ResourceObj because it cannot have a vtable.
// Since sometimes this is allocated from Metadata, pick a base allocation
// type without virtual functions.