From e433fa2719917cff6cb373e9a60981a7418e2f4f Mon Sep 17 00:00:00 2001 From: Ioi Lam Date: Wed, 16 Apr 2025 17:52:53 +0000 Subject: [PATCH] 8352001: AOT cache should not contain classes injected into built-in class loaders Reviewed-by: ccheung, matsaave --- src/hotspot/share/cds/aotClassLocation.cpp | 31 ++++ src/hotspot/share/cds/aotClassLocation.hpp | 5 + src/hotspot/share/classfile/classLoader.cpp | 14 ++ src/hotspot/share/classfile/classLoader.hpp | 1 + .../share/classfile/classLoaderExt.cpp | 10 +- .../share/classfile/classLoaderExt.hpp | 2 +- src/hotspot/share/utilities/zipLibrary.cpp | 9 + src/hotspot/share/utilities/zipLibrary.hpp | 3 +- src/java.base/share/native/libzip/zip_util.c | 4 +- src/java.base/share/native/libzip/zip_util.h | 4 +- .../aotClassLinking/FakeCodeLocation.java | 170 ++++++++++++++++++ 11 files changed, 246 insertions(+), 7 deletions(-) create mode 100644 test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/FakeCodeLocation.java diff --git a/src/hotspot/share/cds/aotClassLocation.cpp b/src/hotspot/share/cds/aotClassLocation.cpp index 656d7551b03..8471d04b572 100644 --- a/src/hotspot/share/cds/aotClassLocation.cpp +++ b/src/hotspot/share/cds/aotClassLocation.cpp @@ -47,6 +47,7 @@ #include #include +Array* AOTClassLocationConfig::_dumptime_jar_files = nullptr; AOTClassLocationConfig* AOTClassLocationConfig::_dumptime_instance = nullptr; const AOTClassLocationConfig* AOTClassLocationConfig::_runtime_instance = nullptr; @@ -479,6 +480,13 @@ void AOTClassLocationConfig::dumptime_init_helper(TRAPS) { _class_locations->at_put(i, tmp_array.at(i)); } + _dumptime_jar_files = MetadataFactory::new_array(ClassLoaderData::the_null_class_loader_data(), + tmp_array.length(), CHECK); + for (int i = 1; i < tmp_array.length(); i++) { + ClassPathZipEntry* jar_file = ClassLoader::create_class_path_zip_entry(tmp_array.at(i)->path()); + _dumptime_jar_files->at_put(i, jar_file); // may be null if the path is not a valid JAR file + } + const char* lcp = find_lcp(all_css.boot_and_app_cp(), _dumptime_lcp_len); if (_dumptime_lcp_len > 0) { os::free((void*)lcp); @@ -693,6 +701,29 @@ void AOTClassLocationConfig::check_nonempty_dirs() const { } } +// It's possible to use reflection+setAccessible to call into ClassLoader::defineClass() to +// pretend that a dynamically generated class comes from a JAR file in the classpath. +// Detect such classes so that they can be excluded from the archive. +bool AOTClassLocationConfig::is_valid_classpath_index(int classpath_index, InstanceKlass* ik) { + if (1 <= classpath_index && classpath_index < length()) { + ClassPathZipEntry *zip = _dumptime_jar_files->at(classpath_index); + if (zip != nullptr) { + JavaThread* current = JavaThread::current(); + ResourceMark rm(current); + const char* const class_name = ik->name()->as_C_string(); + const char* const file_name = ClassLoader::file_name_for_class_name(class_name, + ik->name()->utf8_length()); + if (!zip->has_entry(current, file_name)) { + log_warning(cds)("class %s cannot be archived because it was not defined from %s as claimed", + class_name, zip->name()); + return false; + } + } + } + + return true; +} + AOTClassLocationConfig* AOTClassLocationConfig::write_to_archive() const { Array* archived_copy = ArchiveBuilder::new_ro_array(_class_locations->length()); for (int i = 0; i < _class_locations->length(); i++) { diff --git a/src/hotspot/share/cds/aotClassLocation.hpp b/src/hotspot/share/cds/aotClassLocation.hpp index cb53e9c96e9..47f4dc86c98 100644 --- a/src/hotspot/share/cds/aotClassLocation.hpp +++ b/src/hotspot/share/cds/aotClassLocation.hpp @@ -34,6 +34,7 @@ class AllClassLocationStreams; class ClassLocationStream; +class ClassPathZipEntry; class LogStream; // An AOTClassLocation is a location where the application is configured to load Java classes @@ -139,6 +140,8 @@ class AOTClassLocationConfig : public CHeapObj { static const AOTClassLocationConfig* _runtime_instance; Array* _class_locations; // jrt -> -Xbootclasspath/a -> -classpath -> --module_path + static Array* _dumptime_jar_files; + int _boot_classpath_end; int _app_classpath_end; int _module_end; @@ -263,6 +266,8 @@ public: // Functions used only during runtime bool validate(bool has_aot_linked_classes, bool* has_extra_module_paths) const; + + bool is_valid_classpath_index(int classpath_index, InstanceKlass* ik); }; diff --git a/src/hotspot/share/classfile/classLoader.cpp b/src/hotspot/share/classfile/classLoader.cpp index 3ca1ec237e8..a7d6cc39614 100644 --- a/src/hotspot/share/classfile/classLoader.cpp +++ b/src/hotspot/share/classfile/classLoader.cpp @@ -305,6 +305,20 @@ ClassPathZipEntry::~ClassPathZipEntry() { FREE_C_HEAP_ARRAY(char, _zip_name); } +bool ClassPathZipEntry::has_entry(JavaThread* current, const char* name) { + ThreadToNativeFromVM ttn(current); + // check whether zip archive contains name + jint name_len; + jint filesize; + jzentry* entry = ZipLibrary::find_entry(_zip, name, &filesize, &name_len); + if (entry == nullptr) { + return false; + } else { + ZipLibrary::free_entry(_zip, entry); + return true; + } +} + u1* ClassPathZipEntry::open_entry(JavaThread* current, const char* name, jint* filesize, bool nul_terminate) { // enable call to C land ThreadToNativeFromVM ttn(current); diff --git a/src/hotspot/share/classfile/classLoader.hpp b/src/hotspot/share/classfile/classLoader.hpp index 7827b6066e5..d762e6caca7 100644 --- a/src/hotspot/share/classfile/classLoader.hpp +++ b/src/hotspot/share/classfile/classLoader.hpp @@ -93,6 +93,7 @@ class ClassPathZipEntry: public ClassPathEntry { const char* name() const { return _zip_name; } ClassPathZipEntry(jzfile* zip, const char* zip_name); virtual ~ClassPathZipEntry(); + bool has_entry(JavaThread* current, const char* name); u1* open_entry(JavaThread* current, const char* name, jint* filesize, bool nul_terminate); ClassFileStream* open_stream(JavaThread* current, const char* name); }; diff --git a/src/hotspot/share/classfile/classLoaderExt.cpp b/src/hotspot/share/classfile/classLoaderExt.cpp index 3a6fd0d933c..fb635133537 100644 --- a/src/hotspot/share/classfile/classLoaderExt.cpp +++ b/src/hotspot/share/classfile/classLoaderExt.cpp @@ -67,7 +67,7 @@ int ClassLoaderExt::compare_module_names(const char** p1, const char** p2) { return strcmp(*p1, *p2); } -void ClassLoaderExt::record_result(const s2 classpath_index, InstanceKlass* result, bool redefined) { +void ClassLoaderExt::record_result(s2 classpath_index, InstanceKlass* result, bool redefined) { assert(CDSConfig::is_dumping_archive(), "sanity"); // We need to remember where the class comes from during dumping. @@ -80,9 +80,17 @@ void ClassLoaderExt::record_result(const s2 classpath_index, InstanceKlass* resu classloader_type = ClassLoader::PLATFORM_LOADER; AOTClassLocationConfig::dumptime_set_has_platform_classes(); } + + if (CDSConfig::is_dumping_preimage_static_archive() || CDSConfig::is_dumping_dynamic_archive()) { + if (!AOTClassLocationConfig::dumptime()->is_valid_classpath_index(classpath_index, result)) { + classpath_index = -1; + } + } + AOTClassLocationConfig::dumptime_update_max_used_index(classpath_index); result->set_shared_classpath_index(classpath_index); result->set_shared_class_loader_type(classloader_type); + #if INCLUDE_CDS_JAVA_HEAP if (CDSConfig::is_dumping_heap() && AllowArchivingWithJavaAgent && classloader_type == ClassLoader::BOOT_LOADER && classpath_index < 0 && redefined) { diff --git a/src/hotspot/share/classfile/classLoaderExt.hpp b/src/hotspot/share/classfile/classLoaderExt.hpp index 86bd5cce7ba..f4a1cf1a9c7 100644 --- a/src/hotspot/share/classfile/classLoaderExt.hpp +++ b/src/hotspot/share/classfile/classLoaderExt.hpp @@ -40,7 +40,7 @@ public: static void append_boot_classpath(ClassPathEntry* new_entry); static int compare_module_names(const char** p1, const char** p2); - static void record_result(const s2 classpath_index, InstanceKlass* result, bool redefined); + static void record_result(s2 classpath_index, InstanceKlass* result, bool redefined); #endif // INCLUDE_CDS }; diff --git a/src/hotspot/share/utilities/zipLibrary.cpp b/src/hotspot/share/utilities/zipLibrary.cpp index f56e311ec2e..ae68fb9ef77 100644 --- a/src/hotspot/share/utilities/zipLibrary.cpp +++ b/src/hotspot/share/utilities/zipLibrary.cpp @@ -35,6 +35,7 @@ typedef void**(*ZIP_Open_t)(const char* name, char** pmsg); typedef void(*ZIP_Close_t)(jzfile* zip); typedef jzentry* (*ZIP_FindEntry_t)(jzfile* zip, const char* name, jint* sizeP, jint* nameLen); typedef jboolean(*ZIP_ReadEntry_t)(jzfile* zip, jzentry* entry, unsigned char* buf, char* namebuf); +typedef void(*ZIP_FreeEntry_t)(jzfile *zip, jzentry *entry); typedef jint(*ZIP_CRC32_t)(jint crc, const jbyte* buf, jint len); typedef const char* (*ZIP_GZip_InitParams_t)(size_t, size_t*, size_t*, int); typedef size_t(*ZIP_GZip_Fully_t)(char*, size_t, char*, size_t, char*, size_t, int, char*, char const**); @@ -43,6 +44,7 @@ static ZIP_Open_t ZIP_Open = nullptr; static ZIP_Close_t ZIP_Close = nullptr; static ZIP_FindEntry_t ZIP_FindEntry = nullptr; static ZIP_ReadEntry_t ZIP_ReadEntry = nullptr; +static ZIP_FreeEntry_t ZIP_FreeEntry = nullptr; static ZIP_CRC32_t ZIP_CRC32 = nullptr; static ZIP_GZip_InitParams_t ZIP_GZip_InitParams = nullptr; static ZIP_GZip_Fully_t ZIP_GZip_Fully = nullptr; @@ -79,6 +81,7 @@ static void store_function_pointers(const char* path, bool vm_exit_on_failure) { ZIP_Close = CAST_TO_FN_PTR(ZIP_Close_t, dll_lookup("ZIP_Close", path, vm_exit_on_failure)); ZIP_FindEntry = CAST_TO_FN_PTR(ZIP_FindEntry_t, dll_lookup("ZIP_FindEntry", path, vm_exit_on_failure)); ZIP_ReadEntry = CAST_TO_FN_PTR(ZIP_ReadEntry_t, dll_lookup("ZIP_ReadEntry", path, vm_exit_on_failure)); + ZIP_FreeEntry = CAST_TO_FN_PTR(ZIP_FreeEntry_t, dll_lookup("ZIP_FreeEntry", path, vm_exit_on_failure)); ZIP_CRC32 = CAST_TO_FN_PTR(ZIP_CRC32_t, dll_lookup("ZIP_CRC32", path, vm_exit_on_failure)); // The following entry points are most likely optional from a zip library implementation perspective. // Hence no vm_exit on a resolution failure. Further refactorings should investigate this, @@ -176,6 +179,12 @@ jboolean ZipLibrary::read_entry(jzfile* zip, jzentry* entry, unsigned char* buf, return ZIP_ReadEntry(zip, entry, buf, namebuf); } +void ZipLibrary::free_entry(jzfile* zip, jzentry* entry) { + initialize(); + assert(ZIP_FreeEntry != nullptr, "invariant"); + ZIP_FreeEntry(zip, entry); +} + jint ZipLibrary::crc32(jint crc, const jbyte* buf, jint len) { initialize(); assert(ZIP_CRC32 != nullptr, "invariant"); diff --git a/src/hotspot/share/utilities/zipLibrary.hpp b/src/hotspot/share/utilities/zipLibrary.hpp index 73745aca2e2..90f55c24c19 100644 --- a/src/hotspot/share/utilities/zipLibrary.hpp +++ b/src/hotspot/share/utilities/zipLibrary.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 2025, 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 @@ -46,6 +46,7 @@ class ZipLibrary : AllStatic { static void close(jzfile* zip); static jzentry* find_entry(jzfile* zip, const char* name, jint* sizeP, jint* nameLen); static jboolean read_entry(jzfile* zip, jzentry* entry, unsigned char* buf, char* namebuf); + static void free_entry(jzfile* zip, jzentry* entry); static jint crc32(jint crc, const jbyte* buf, jint len); static const char* init_params(size_t block_size, size_t* needed_out_size, size_t* needed_tmp_size, int level); static size_t compress(char* in, size_t in_size, char* out, size_t out_size, char* tmp, size_t tmp_size, int level, char* buf, const char** pmsg); diff --git a/src/java.base/share/native/libzip/zip_util.c b/src/java.base/share/native/libzip/zip_util.c index c327d337659..86223f22a43 100644 --- a/src/java.base/share/native/libzip/zip_util.c +++ b/src/java.base/share/native/libzip/zip_util.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 1995, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1995, 2025, 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 @@ -1116,7 +1116,7 @@ newEntry(jzfile *zip, jzcell *zc, AccessHint accessHint) * jzentry for each zip. This optimizes a common access pattern. */ -void +JNIEXPORT void ZIP_FreeEntry(jzfile *jz, jzentry *ze) { jzentry *last; diff --git a/src/java.base/share/native/libzip/zip_util.h b/src/java.base/share/native/libzip/zip_util.h index eef52b38b05..9825202fc7b 100644 --- a/src/java.base/share/native/libzip/zip_util.h +++ b/src/java.base/share/native/libzip/zip_util.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 1995, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1995, 2025, 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 @@ -276,7 +276,7 @@ void ZIP_Unlock(jzfile *zip); jint ZIP_Read(jzfile *zip, jzentry *entry, jlong pos, void *buf, jint len); -void +JNIEXPORT void ZIP_FreeEntry(jzfile *zip, jzentry *ze); jlong ZIP_GetEntryDataOffset(jzfile *zip, jzentry *entry); jzentry * ZIP_GetEntry2(jzfile *zip, char *name, jint ulen, jboolean addSlash); diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/FakeCodeLocation.java b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/FakeCodeLocation.java new file mode 100644 index 00000000000..5bec6623a08 --- /dev/null +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/FakeCodeLocation.java @@ -0,0 +1,170 @@ +/* + * Copyright (c) 2025, 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 Do not cache classes that are loaded from a fake location. + * @bug 8352001 + * @requires vm.cds.supports.aot.class.linking + * @comment work around JDK-8345635 + * @requires !vm.jvmci.enabled + * @library /test/jdk/lib/testlibrary /test/lib + * @build FakeCodeLocation + * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar app.jar FakeCodeLocationApp + * @run driver jdk.test.lib.helpers.ClassFileInstaller ClassNotInJar1 ClassNotInJar2 + * @run driver FakeCodeLocation + */ + +import java.lang.invoke.MethodHandles; +import java.lang.reflect.InaccessibleObjectException; +import java.lang.reflect.Method; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.security.ProtectionDomain; + +import jdk.test.lib.cds.CDSAppTester; +import jdk.test.lib.helpers.ClassFileInstaller; +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.StringArrayUtils; + +public class FakeCodeLocation { + static final String appJar = "app.jar"; + static final String mainClass = FakeCodeLocationApp.class.getName(); + + public static void main(String[] args) throws Exception { + (new Tester(false)).run(new String[] {"STATIC"}); + (new Tester(true )).run(new String[] {"STATIC"}); + (new Tester(false)).run(new String[] {"AOT"}); + (new Tester(true )).run(new String[] {"AOT"}); + } + + static class Tester extends CDSAppTester { + boolean addOpen;; + public Tester(boolean addOpen) { + super(mainClass); + this.addOpen = addOpen; + } + + @Override + public String classpath(RunMode runMode) { + return appJar; + } + + @Override + public String[] vmArgs(RunMode runMode) { + String[] args = new String[] { + "-Xlog:cds", + "-Xlog:cds+class=debug", + "-Xlog:class+load", + }; + if (addOpen) { + args = StringArrayUtils.concat(args, "--add-opens", "java.base/java.lang=ALL-UNNAMED", "-XX:-AOTClassLinking"); + } + return args; + } + + @Override + public String[] appCommandLine(RunMode runMode) { + return new String[] { + mainClass, + addOpen ? "hasAddedOpen" : "hasNotAddedOpen", + }; + } + + @Override + public void checkExecution(OutputAnalyzer out, RunMode runMode) throws Exception { + if (isDumping(runMode)) { + out.shouldMatch("cds,class.* FakeCodeLocationApp"); + out.shouldNotMatch("cds,class.* ClassNotInJar1"); + out.shouldNotMatch("cds,class.* ClassNotInJar2"); + } + + if (runMode.isProductionRun()) { + out.shouldMatch("class,load.* FakeCodeLocationApp .*source: shared objects file"); + out.shouldNotMatch("class,load.* ClassNotInJar1 .*source: shared objects file"); + out.shouldNotMatch("class,load.* ClassNotInJar2 .*source: shared objects file"); + } + } + } +} + +class FakeCodeLocationApp { + static boolean hasAddedOpen; + + public static void main(String args[]) throws Exception { + hasAddedOpen = args[0].equals("hasAddedOpen"); + testWithLookup(); + testWithSetAccessible(); + } + + // Define a class using Lookup.defineClass(). The ClassFileParser should see "__JVM_DefineClass__" + // as the source location, so this class will be excluded, as the location is not supported. + static void testWithLookup() throws Exception { + byte[] data = Files.readAllBytes(Paths.get("ClassNotInJar1.class")); + Class c = MethodHandles.lookup().defineClass(data); + System.out.println(c.getProtectionDomain()); + System.out.println(c.getProtectionDomain().getCodeSource()); + } + + // Use setAccessible to call into ClassLoader.defineClass(). In this case, the ClassFileParser + // sees "app.jar" as the source location, but the app.jar doesn't contain this class file, so we + // should exclude this class. + static void testWithSetAccessible() throws Exception { + Method m = ClassLoader.class.getDeclaredMethod("defineClass", String.class, byte[].class, int.class, int.class, ProtectionDomain.class); + System.out.println(m); + try { + m.setAccessible(true); + if (!hasAddedOpen) { + throw new RuntimeException("setAccessible() should have failed because '--add-opens java.base/java.lang=ALL-UNNAMED' was not specified"); + } + } catch (InaccessibleObjectException t) { + if (hasAddedOpen) { + throw new RuntimeException("setAccessible() failed even though '--add-opens java.base/java.lang=ALL-UNNAMED' was specified"); + } else { + System.out.println("\n\nExpected: " + t); + t.printStackTrace(System.out); + return; + } + } + + ProtectionDomain pd = FakeCodeLocationApp.class.getProtectionDomain(); + ClassLoader appLoader = FakeCodeLocationApp.class.getClassLoader(); + byte[] data = Files.readAllBytes(Paths.get("ClassNotInJar2.class")); + Class c = null; + try { + c = (Class)m.invoke(appLoader, "ClassNotInJar2", data, 0, data.length, pd); + } catch (Throwable t) { + System.out.println(t); + t.printStackTrace(System.out); + return; + } + + System.out.println(c); + System.out.println(c.getProtectionDomain()); + System.out.println(c.getProtectionDomain().getCodeSource()); + } +} + +class ClassNotInJar1 {} + +class ClassNotInJar2 {}