8195744: Avoid calling ClassLoader.checkPackageAccess if security manager is not installed

Reviewed-by: dholmes, iklam
This commit is contained in:
Coleen Phillimore 2021-02-08 21:31:25 +00:00
parent ab65d53edf
commit ace8f94616
8 changed files with 131 additions and 82 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2021, 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
@ -132,12 +132,17 @@ bool Dictionary::resize_if_needed() {
return (desired_size != 0);
}
bool DictionaryEntry::is_valid_protection_domain(Handle protection_domain) {
return protection_domain() == NULL || !java_lang_System::allow_security_manager()
? true
: contains_protection_domain(protection_domain());
}
bool DictionaryEntry::contains_protection_domain(oop protection_domain) const {
// Lock the pd_set list. This lock cannot safepoint since the caller holds
// a Dictionary entry, which can be moved if the Dictionary is resized.
MutexLocker ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
#ifdef ASSERT
if (protection_domain == instance_klass()->protection_domain()) {
MutexLocker ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
// Ensure this doesn't show up in the pd_set (invariant)
bool in_pd_set = false;
for (ProtectionDomainEntry* current = pd_set();
@ -160,10 +165,15 @@ bool DictionaryEntry::contains_protection_domain(oop protection_domain) const {
return true;
}
// Lock the pd_set list. This lock cannot safepoint since the caller holds
// a Dictionary entry, which can be moved if the Dictionary is resized.
MutexLocker ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
for (ProtectionDomainEntry* current = pd_set();
current != NULL;
current = current->next()) {
if (current->object_no_keepalive() == protection_domain) return true;
if (current->object_no_keepalive() == protection_domain) {
return true;
}
}
return false;
}
@ -283,6 +293,7 @@ DictionaryEntry* Dictionary::get_entry(int index, unsigned int hash,
}
InstanceKlass* Dictionary::find(unsigned int hash, Symbol* name,
Handle protection_domain) {
NoSafepointVerifier nsv;
@ -307,11 +318,11 @@ InstanceKlass* Dictionary::find_class(unsigned int hash,
return (entry != NULL) ? entry->instance_klass() : NULL;
}
void Dictionary::add_protection_domain(int index, unsigned int hash,
InstanceKlass* klass,
Handle protection_domain,
TRAPS) {
assert(java_lang_System::allow_security_manager(), "only needed if security manager allowed");
Symbol* klass_name = klass->name();
DictionaryEntry* entry = get_entry(index, hash, klass_name);

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2021, 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
@ -153,14 +153,7 @@ class DictionaryEntry : public HashtableEntry<InstanceKlass*, mtClass> {
void set_pd_set(ProtectionDomainEntry* new_head) { _pd_set = new_head; }
// Tells whether the initiating class' protection domain can access the klass in this entry
bool is_valid_protection_domain(Handle protection_domain) {
if (!ProtectionDomainVerification) return true;
return protection_domain() == NULL
? true
: contains_protection_domain(protection_domain());
}
inline bool is_valid_protection_domain(Handle protection_domain);
void verify_protection_domain_set();
bool equals(const Symbol* class_name) const {

View File

@ -4391,18 +4391,41 @@ int java_lang_System::_static_in_offset;
int java_lang_System::_static_out_offset;
int java_lang_System::_static_err_offset;
int java_lang_System::_static_security_offset;
int java_lang_System::_static_allow_security_offset;
int java_lang_System::_static_never_offset;
#define SYSTEM_FIELDS_DO(macro) \
macro(_static_in_offset, k, "in", input_stream_signature, true); \
macro(_static_out_offset, k, "out", print_stream_signature, true); \
macro(_static_err_offset, k, "err", print_stream_signature, true); \
macro(_static_security_offset, k, "security", security_manager_signature, true)
macro(_static_security_offset, k, "security", security_manager_signature, true); \
macro(_static_allow_security_offset, k, "allowSecurityManager", int_signature, true); \
macro(_static_never_offset, k, "NEVER", int_signature, true)
void java_lang_System::compute_offsets() {
InstanceKlass* k = vmClasses::System_klass();
SYSTEM_FIELDS_DO(FIELD_COMPUTE_OFFSET);
}
// This field tells us that a security manager can never be installed so we
// can completely skip populating the ProtectionDomainCacheTable.
bool java_lang_System::allow_security_manager() {
static int initialized = false;
static bool allowed = true; // default
if (!initialized) {
oop base = vmClasses::System_klass()->static_field_base_raw();
int never = base->int_field(_static_never_offset);
allowed = (base->int_field(_static_allow_security_offset) != never);
}
return allowed;
}
// This field tells us that a security manager is installed.
bool java_lang_System::has_security_manager() {
oop base = vmClasses::System_klass()->static_field_base_raw();
return base->obj_field(_static_security_offset) != NULL;
}
#if INCLUDE_CDS
void java_lang_System::serialize_offsets(SerializeClosure* f) {
SYSTEM_FIELDS_DO(FIELD_SERIALIZE_OFFSET);

View File

@ -1372,11 +1372,15 @@ class java_lang_System : AllStatic {
static int _static_out_offset;
static int _static_err_offset;
static int _static_security_offset;
static int _static_allow_security_offset;
static int _static_never_offset;
public:
static int in_offset() { CHECK_INIT(_static_in_offset); }
static int out_offset() { CHECK_INIT(_static_out_offset); }
static int err_offset() { CHECK_INIT(_static_err_offset); }
static bool allow_security_manager();
static bool has_security_manager();
static void compute_offsets();
static void serialize_offsets(SerializeClosure* f) NOT_CDS_RETURN;

View File

@ -25,6 +25,7 @@
#include "precompiled.hpp"
#include "classfile/classLoaderDataGraph.hpp"
#include "classfile/dictionary.hpp"
#include "classfile/javaClasses.hpp"
#include "classfile/protectionDomainCache.hpp"
#include "logging/log.hpp"
#include "logging/logStream.hpp"
@ -66,6 +67,9 @@ class CleanProtectionDomainEntries : public CLDClosure {
};
void ProtectionDomainCacheTable::unlink() {
// The dictionary entries _pd_set field should be null also, so nothing to do.
assert(java_lang_System::allow_security_manager(), "should not be called otherwise");
{
// First clean cached pd lists in loaded CLDs
// It's unlikely, but some loaded classes in a dictionary might

View File

@ -460,54 +460,54 @@ void SystemDictionary::validate_protection_domain(InstanceKlass* klass,
Handle protection_domain,
TRAPS) {
// Now we have to call back to java to check if the initating class has access
JavaValue result(T_VOID);
LogTarget(Debug, protectiondomain) lt;
if (lt.is_enabled()) {
ResourceMark rm(THREAD);
// Print out trace information
LogStream ls(lt);
ls.print_cr("Checking package access");
if (class_loader() != NULL) {
assert(class_loader() != NULL, "Should not call this");
assert(protection_domain() != NULL, "Should not call this");
// We only have to call checkPackageAccess if there's a security manager installed.
if (java_lang_System::has_security_manager()) {
// This handle and the class_loader handle passed in keeps this class from
// being unloaded through several GC points.
// The class_loader handle passed in is the initiating loader.
Handle mirror(THREAD, klass->java_mirror());
InstanceKlass* system_loader = vmClasses::ClassLoader_klass();
JavaValue result(T_VOID);
JavaCalls::call_special(&result,
class_loader,
system_loader,
vmSymbols::checkPackageAccess_name(),
vmSymbols::class_protectiondomain_signature(),
mirror,
protection_domain,
THREAD);
LogTarget(Debug, protectiondomain) lt;
if (lt.is_enabled()) {
ResourceMark rm(THREAD);
// Print out trace information
LogStream ls(lt);
ls.print_cr("Checking package access");
ls.print("class loader: ");
class_loader()->print_value_on(&ls);
} else {
ls.print_cr("class loader: NULL");
}
if (protection_domain() != NULL) {
ls.print(" protection domain: ");
protection_domain()->print_value_on(&ls);
} else {
ls.print_cr(" protection domain: NULL");
ls.print(" loading: "); klass->print_value_on(&ls);
if (HAS_PENDING_EXCEPTION) {
ls.print_cr(" DENIED !!!!!!!!!!!!!!!!!!!!!");
} else {
ls.print_cr(" granted");
}
}
ls.print(" loading: "); klass->print_value_on(&ls);
ls.cr();
if (HAS_PENDING_EXCEPTION) return;
}
// This handle and the class_loader handle passed in keeps this class from
// being unloaded through several GC points.
// The class_loader handle passed in is the initiating loader.
Handle mirror(THREAD, klass->java_mirror());
InstanceKlass* system_loader = vmClasses::ClassLoader_klass();
JavaCalls::call_special(&result,
class_loader,
system_loader,
vmSymbols::checkPackageAccess_name(),
vmSymbols::class_protectiondomain_signature(),
mirror,
protection_domain,
THREAD);
if (HAS_PENDING_EXCEPTION) {
log_debug(protectiondomain)("DENIED !!!!!!!!!!!!!!!!!!!!!");
} else {
log_debug(protectiondomain)("granted");
}
if (HAS_PENDING_EXCEPTION) return;
// If no exception has been thrown, we have validated the protection domain
// Insert the protection domain of the initiating class into the set.
// We still have to add the protection_domain to the dictionary in case a new
// security manager is installed later. Calls to load the same class with class loader
// and protection domain are expected to succeed.
{
ClassLoaderData* loader_data = class_loader_data(class_loader);
Dictionary* dictionary = loader_data->dictionary();
@ -889,18 +889,15 @@ InstanceKlass* SystemDictionary::resolve_instance_class_or_null(Symbol* name,
// Make sure we have the right class in the dictionary
DEBUG_ONLY(verify_dictionary_entry(name, loaded_class));
// return if the protection domain in NULL
if (protection_domain() == NULL) return loaded_class;
// Check the protection domain has the right access
if (dictionary->is_valid_protection_domain(name_hash, name,
// Check if the protection domain is present it has the right access
if (protection_domain() != NULL &&
java_lang_System::allow_security_manager() &&
!dictionary->is_valid_protection_domain(name_hash, name,
protection_domain)) {
return loaded_class;
// Verify protection domain. If it fails an exception is thrown
validate_protection_domain(loaded_class, class_loader, protection_domain, CHECK_NULL);
}
// Verify protection domain. If it fails an exception is thrown
validate_protection_domain(loaded_class, class_loader, protection_domain, CHECK_NULL);
return loaded_class;
}
@ -1773,12 +1770,16 @@ bool SystemDictionary::do_unloading(GCTimer* gc_timer) {
if (unloading_occurred) {
SymbolTable::trigger_cleanup();
// Oops referenced by the protection domain cache table may get unreachable independently
// of the class loader (eg. cached protection domain oops). So we need to
// explicitly unlink them here.
// All protection domain oops are linked to the caller class, so if nothing
// unloads, this is not needed.
_pd_cache_table->trigger_cleanup();
if (java_lang_System::allow_security_manager()) {
// Oops referenced by the protection domain cache table may get unreachable independently
// of the class loader (eg. cached protection domain oops). So we need to
// explicitly unlink them here.
// All protection domain oops are linked to the caller class, so if nothing
// unloads, this is not needed.
_pd_cache_table->trigger_cleanup();
} else {
assert(_pd_cache_table->number_of_entries() == 0, "should be empty");
}
}
return unloading_occurred;

View File

@ -677,9 +677,6 @@ const intx ObjectAlignmentInBytes = 8;
develop(bool, UsePrivilegedStack, true, \
"Enable the security JVM functions") \
\
develop(bool, ProtectionDomainVerification, true, \
"Verify protection domain before resolution in system dictionary")\
\
product(bool, ClassUnloading, true, \
"Do unloading of classes") \
\

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2021, 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
@ -40,22 +40,38 @@ public class ProtectionDomainVerificationTest {
// -Xlog:protectiondomain=trace
ProcessBuilder pb = ProcessTools.createJavaProcessBuilder("-Xlog:protectiondomain=trace",
"-Xmx128m",
Hello.class.getName());
OutputAnalyzer out = new OutputAnalyzer(pb.start());
out.shouldContain("[protectiondomain] Checking package access");
out.shouldContain("[protectiondomain] pd set count = #");
Hello.class.getName(), "security_manager");
new OutputAnalyzer(pb.start())
.shouldHaveExitValue(0)
.shouldContain("[protectiondomain] Checking package access")
.shouldContain("[protectiondomain] pd set count = #");
// -Xlog:protectiondomain=debug
pb = ProcessTools.createJavaProcessBuilder("-Xlog:protectiondomain=debug",
"-Xmx128m",
Hello.class.getName());
out = new OutputAnalyzer(pb.start());
out.shouldContain("[protectiondomain] Checking package access");
out.shouldNotContain("pd set count = #");
Hello.class.getName(), "security_manager");
new OutputAnalyzer(pb.start())
.shouldHaveExitValue(0)
.shouldContain("[protectiondomain] Checking package access")
.shouldNotContain("pd set count = #");
// -Xlog:protectiondomain=debug
pb = ProcessTools.createJavaProcessBuilder("-Xlog:protectiondomain=trace",
"-Xmx128m",
"-Djava.security.manager=disallow",
Hello.class.getName());
new OutputAnalyzer(pb.start())
.shouldHaveExitValue(0)
.shouldNotContain("[protectiondomain] Checking package access")
.shouldNotContain("pd set count = #");
}
public static class Hello {
public static void main(String[] args) {
if (args.length == 1) {
// Need a security manager to trigger logging.
System.setSecurityManager(new SecurityManager());
}
System.out.print("Hello!");
}
}