8245487: Potential double-free of interfaces array

Free _local_interfaces when we free _transitive_interfaces since they can share memory.

Reviewed-by: fparain, lfoltan, iklam
This commit is contained in:
Coleen Phillimore 2020-06-05 06:54:05 -04:00
parent f833afa660
commit cb960ee7b5
5 changed files with 118 additions and 14 deletions
src/hotspot/share
test/hotspot/jtreg/runtime/classFileParserBug

@ -3949,12 +3949,12 @@ void ClassFileParser::apply_parsed_class_metadata(
this_klass->set_inner_classes(_inner_classes);
this_klass->set_nest_members(_nest_members);
this_klass->set_nest_host_index(_nest_host);
this_klass->set_local_interfaces(_local_interfaces);
this_klass->set_annotations(_combined_annotations);
this_klass->set_permitted_subclasses(_permitted_subclasses);
this_klass->set_record_components(_record_components);
// Delay the setting of _transitive_interfaces until after initialize_supers() in
// fill_instance_klass(). It is because the _transitive_interfaces may be shared with
// Delay the setting of _local_interfaces and _transitive_interfaces until after
// initialize_supers() in fill_instance_klass(). It is because the _local_interfaces could
// be shared with _transitive_interfaces and _transitive_interfaces may be shared with
// its _super. If an OOM occurs while loading the current klass, its _super field
// may not have been set. When GC tries to free the klass, the _transitive_interfaces
// may be deallocated mistakenly in InstanceKlass::deallocate_interfaces(). Subsequent
@ -5788,7 +5788,6 @@ void ClassFileParser::fill_instance_klass(InstanceKlass* ik,
assert(NULL == _methods, "invariant");
assert(NULL == _inner_classes, "invariant");
assert(NULL == _nest_members, "invariant");
assert(NULL == _local_interfaces, "invariant");
assert(NULL == _combined_annotations, "invariant");
assert(NULL == _record_components, "invariant");
assert(NULL == _permitted_subclasses, "invariant");
@ -5862,7 +5861,9 @@ void ClassFileParser::fill_instance_klass(InstanceKlass* ik,
// Fill in information needed to compute superclasses.
ik->initialize_supers(const_cast<InstanceKlass*>(_super_klass), _transitive_interfaces, CHECK);
ik->set_transitive_interfaces(_transitive_interfaces);
ik->set_local_interfaces(_local_interfaces);
_transitive_interfaces = NULL;
_local_interfaces = NULL;
// Initialize itable offset tables
klassItable::setup_itable_offset_table(ik);
@ -6190,7 +6191,6 @@ void ClassFileParser::clear_class_metadata() {
_inner_classes = NULL;
_nest_members = NULL;
_permitted_subclasses = NULL;
_local_interfaces = NULL;
_combined_annotations = NULL;
_class_annotations = _class_type_annotations = NULL;
_fields_annotations = _fields_type_annotations = NULL;
@ -6258,6 +6258,7 @@ ClassFileParser::~ClassFileParser() {
clear_class_metadata();
_transitive_interfaces = NULL;
_local_interfaces = NULL;
// deallocate the klass if already created. Don't directly deallocate, but add
// to the deallocate list so that the klass is removed from the CLD::_klasses list

@ -666,6 +666,13 @@ ClassLoaderMetaspace* ClassLoaderDataGraphMetaspaceIterator::get_next() {
return result;
}
void ClassLoaderDataGraph::verify() {
ClassLoaderDataGraphIterator iter;
while (ClassLoaderData* cld = iter.get_next()) {
cld->verify();
}
}
#ifndef PRODUCT
// callable from debugger
extern "C" int print_loader_data_graph() {
@ -674,13 +681,6 @@ extern "C" int print_loader_data_graph() {
return 0;
}
void ClassLoaderDataGraph::verify() {
ClassLoaderDataGraphIterator iter;
while (ClassLoaderData* cld = iter.get_next()) {
cld->verify();
}
}
void ClassLoaderDataGraph::print_on(outputStream * const out) {
ClassLoaderDataGraphIterator iter;
while (ClassLoaderData* cld = iter.get_next()) {

@ -1094,12 +1094,10 @@ void Universe::verify(VerifyOption option, const char* prefix) {
log_debug(gc, verify)("SystemDictionary");
SystemDictionary::verify();
}
#ifndef PRODUCT
if (should_verify_subset(Verify_ClassLoaderDataGraph)) {
log_debug(gc, verify)("ClassLoaderDataGraph");
ClassLoaderDataGraph::verify();
}
#endif
if (should_verify_subset(Verify_MetaspaceUtils)) {
log_debug(gc, verify)("MetaspaceUtils");
MetaspaceUtils::verify_free_chunks();

@ -0,0 +1,39 @@
/*
* Copyright (c) 2020, 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.
*/
package java/lang;
super public class BadClassPackage
implements java/util/function/Function
version 47:0
{
public Method "<init>":"()V"
stack 1 locals 1
{
aload_0;
invokespecial Method Object."<init>":"()V";
return;
}
} // end Class BadClassPackage

@ -0,0 +1,66 @@
/*
* Copyright (c) 2020, 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
* @bug 8245487
* @summary Check that if the VM rejects classes from packages starting with "java/", it will exit
* cleanly after InstanceKlass::verify_on(), and not leave freed memory in _local_interfaces.
* @library /test/lib
* @compile BadClassPackage.jasm
* @run driver TestBadPackageWithInterface
*/
import java.io.InputStream;
import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.process.ProcessTools;
public class TestBadPackageWithInterface {
public static void main(String args[]) throws Throwable {
ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
"-cp", System.getProperty("test.classes"),
"-XX:+UnlockDiagnosticVMOptions",
"-XX:+VerifyBeforeExit", MyLoader.class.getName());
OutputAnalyzer oa = new OutputAnalyzer(pb.start());
oa.shouldHaveExitValue(0);
}
static class MyLoader extends ClassLoader {
public static void main(String args[]) throws Throwable {
try {
ClassLoader loader = TestBadPackageWithInterface.class.getClassLoader();
InputStream in = loader.getResourceAsStream("java/lang/BadClassPackage.class");
byte[] bytes = in.readAllBytes();
MyLoader myLoader = new MyLoader();
myLoader.defineClass(bytes, 0, bytes.length);
}
catch (SecurityException expected) {
System.out.println("Expected ==================================================");
expected.printStackTrace(System.out);
System.out.println("==================================================");
}
}
}
}