8226406: JVM fails to detect mismatched or corrupt CDS archive
Summary: Check important archive header fields such as _jvm_ident before processing other fields.
Reviewed-by: iklam, jiangli
--- a/src/hotspot/share/memory/filemap.cpp Fri Jul 12 10:57:27 2019 +0200
+++ b/src/hotspot/share/memory/filemap.cpp Fri Jul 12 08:40:37 2019 -0700
@@ -155,6 +155,8 @@
const char *vm_version = VM_Version::internal_vm_info_string();
const int version_len = (int)strlen(vm_version);
+ memset(header_version, 0, JVM_IDENT_MAX);
+
if (version_len < (JVM_IDENT_MAX-1)) {
strcpy(header_version, vm_version);
@@ -170,6 +172,8 @@
sprintf(&header_version[JVM_IDENT_MAX-9], "%08x", hash);
header_version[JVM_IDENT_MAX-1] = 0; // Null terminate.
}
+
+ assert(header_version[JVM_IDENT_MAX-1] == 0, "must be");
}
FileMapInfo::FileMapInfo(bool is_static) {
@@ -730,10 +734,59 @@
fail_continue("Unable to read the file header.");
return false;
}
+
+ if (!Arguments::has_jimage()) {
+ FileMapInfo::fail_continue("The shared archive file cannot be used with an exploded module build.");
+ return false;
+ }
+
+ unsigned int expected_magic = is_static ? CDS_ARCHIVE_MAGIC : CDS_DYNAMIC_ARCHIVE_MAGIC;
+ if (_header->_magic != expected_magic) {
+ log_info(cds)("_magic expected: 0x%08x", expected_magic);
+ log_info(cds)(" actual: 0x%08x", _header->_magic);
+ FileMapInfo::fail_continue("The shared archive file has a bad magic number.");
+ return false;
+ }
+
if (_header->_version != CURRENT_CDS_ARCHIVE_VERSION) {
+ log_info(cds)("_version expected: %d", CURRENT_CDS_ARCHIVE_VERSION);
+ log_info(cds)(" actual: %d", _header->_version);
fail_continue("The shared archive file has the wrong version.");
return false;
}
+
+ if (_header->_header_size != sz) {
+ log_info(cds)("_header_size expected: " SIZE_FORMAT, sz);
+ log_info(cds)(" actual: " SIZE_FORMAT, _header->_header_size);
+ FileMapInfo::fail_continue("The shared archive file has an incorrect header size.");
+ return false;
+ }
+
+ if (_header->_jvm_ident[JVM_IDENT_MAX-1] != 0) {
+ FileMapInfo::fail_continue("JVM version identifier is corrupted.");
+ return false;
+ }
+
+ char header_version[JVM_IDENT_MAX];
+ get_header_version(header_version);
+ if (strncmp(_header->_jvm_ident, header_version, JVM_IDENT_MAX-1) != 0) {
+ log_info(cds)("_jvm_ident expected: %s", header_version);
+ log_info(cds)(" actual: %s", _header->_jvm_ident);
+ FileMapInfo::fail_continue("The shared archive file was created by a different"
+ " version or build of HotSpot");
+ return false;
+ }
+
+ if (VerifySharedSpaces) {
+ int expected_crc = _header->compute_crc();
+ if (expected_crc != _header->_crc) {
+ log_info(cds)("_crc expected: %d", expected_crc);
+ log_info(cds)(" actual: %d", _header->_crc);
+ FileMapInfo::fail_continue("Header checksum verification failed.");
+ return false;
+ }
+ }
+
_file_offset = n;
size_t info_size = _header->_paths_misc_info_size;
@@ -748,10 +801,6 @@
_file_offset += n + _header->_base_archive_name_size; // accounts for the size of _base_archive_name
if (is_static) {
- if (_header->_magic != CDS_ARCHIVE_MAGIC) {
- fail_continue("Incorrect static archive magic number");
- return false;
- }
// just checking the last region is sufficient since the archive is written
// in sequential order
size_t len = lseek(fd, 0, SEEK_END);
@@ -1589,33 +1638,7 @@
// This function should only be called during run time with UseSharedSpaces enabled.
bool FileMapHeader::validate() {
- if (VerifySharedSpaces && compute_crc() != _crc) {
- FileMapInfo::fail_continue("Header checksum verification failed.");
- return false;
- }
- if (!Arguments::has_jimage()) {
- FileMapInfo::fail_continue("The shared archive file cannot be used with an exploded module build.");
- return false;
- }
-
- if (_version != CURRENT_CDS_ARCHIVE_VERSION) {
- FileMapInfo::fail_continue("The shared archive file is the wrong version.");
- return false;
- }
- if (_magic != CDS_ARCHIVE_MAGIC && _magic != CDS_DYNAMIC_ARCHIVE_MAGIC) {
- FileMapInfo::fail_continue("The shared archive file has a bad magic number.");
- return false;
- }
- char header_version[JVM_IDENT_MAX];
- get_header_version(header_version);
- if (strncmp(_jvm_ident, header_version, JVM_IDENT_MAX-1) != 0) {
- log_info(class, path)("expected: %s", header_version);
- log_info(class, path)("actual: %s", _jvm_ident);
- FileMapInfo::fail_continue("The shared archive file was created by a different"
- " version or build of HotSpot");
- return false;
- }
if (_obj_alignment != ObjectAlignmentInBytes) {
FileMapInfo::fail_continue("The shared archive file's ObjectAlignmentInBytes of %d"
" does not equal the current ObjectAlignmentInBytes of " INTX_FORMAT ".",
--- a/src/hotspot/share/prims/cdsoffsets.cpp Fri Jul 12 10:57:27 2019 +0200
+++ b/src/hotspot/share/prims/cdsoffsets.cpp Fri Jul 12 08:40:37 2019 -0700
@@ -49,6 +49,7 @@
ADD_NEXT(_all, "FileMapHeader::_magic", offset_of(FileMapHeader, _magic)); \
ADD_NEXT(_all, "FileMapHeader::_crc", offset_of(FileMapHeader, _crc)); \
ADD_NEXT(_all, "FileMapHeader::_version", offset_of(FileMapHeader, _version)); \
+ ADD_NEXT(_all, "FileMapHeader::_jvm_ident", offset_of(FileMapHeader, _jvm_ident)); \
ADD_NEXT(_all, "FileMapHeader::_space[0]", offset_of(FileMapHeader, _space)); \
ADD_NEXT(_all, "CDSFileMapRegion::_crc", offset_of(CDSFileMapRegion, _crc)); \
ADD_NEXT(_all, "CDSFileMapRegion::_used", offset_of(CDSFileMapRegion, _used)); \
--- a/test/hotspot/jtreg/runtime/appcds/SharedArchiveConsistency.java Fri Jul 12 10:57:27 2019 +0200
+++ b/test/hotspot/jtreg/runtime/appcds/SharedArchiveConsistency.java Fri Jul 12 08:40:37 2019 -0700
@@ -61,13 +61,17 @@
public class SharedArchiveConsistency {
public static WhiteBox wb;
- public static int offset_magic; // FileMapHeader::_magic
- public static int sp_offset_crc; // CDSFileMapRegion::_crc
+ public static int offset_magic; // CDSFileMapHeaderBase::_magic
+ public static int offset_version; // CDSFileMapHeaderBase::_version
+ public static int offset_jvm_ident; // FileMapHeader::_jvm_ident
+ public static int sp_offset_crc; // CDSFileMapRegion::_crc
+ public static int offset_paths_misc_info_size;
public static int file_header_size = -1;// total size of header, variant, need calculation
public static int CDSFileMapRegion_size; // size of CDSFileMapRegion
public static int sp_offset; // offset of CDSFileMapRegion
public static int sp_used_offset; // offset of CDSFileMapRegion::_used
public static int size_t_size; // size of size_t
+ public static int int_size; // size of int
public static File jsa; // will be updated during test
public static File orgJsaFile; // kept the original file not touched.
@@ -94,6 +98,8 @@
public static void getFileOffsetInfo() throws Exception {
wb = WhiteBox.getWhiteBox();
offset_magic = wb.getOffsetForName("FileMapHeader::_magic");
+ offset_version = wb.getOffsetForName("FileMapHeader::_version");
+ offset_jvm_ident = wb.getOffsetForName("FileMapHeader::_jvm_ident");
sp_offset_crc = wb.getOffsetForName("CDSFileMapRegion::_crc");
try {
int nonExistOffset = wb.getOffsetForName("FileMapHeader::_non_exist_offset");
@@ -113,13 +119,13 @@
return file_header_size;
}
// this is not real header size, it is struct size
- int int_size = wb.getOffsetForName("int_size");
+ int_size = wb.getOffsetForName("int_size");
file_header_size = wb.getOffsetForName("file_header_size");
- int offset_path_misc_info = wb.getOffsetForName("FileMapHeader::_paths_misc_info_size") -
+ offset_paths_misc_info_size = wb.getOffsetForName("FileMapHeader::_paths_misc_info_size") -
offset_magic;
- int path_misc_info_size = (int)readInt(fc, offset_path_misc_info, int_size);
- file_header_size += path_misc_info_size; //readInt(fc, offset_path_misc_info, size_t_size);
- System.out.println("offset_path_misc_info = " + offset_path_misc_info);
+ int path_misc_info_size = (int)readInt(fc, offset_paths_misc_info_size, int_size);
+ file_header_size += path_misc_info_size;
+ System.out.println("offset_paths_misc_info_size = " + offset_paths_misc_info_size);
System.out.println("path_misc_info_size = " + path_misc_info_size);
System.out.println("file_header_size = " + file_header_size);
file_header_size = (int)align_up_page(file_header_size);
@@ -160,15 +166,15 @@
fc.force(true);
}
- public static FileChannel getFileChannel() throws Exception {
+ public static FileChannel getFileChannel(File jsaFile) throws Exception {
List<StandardOpenOption> arry = new ArrayList<StandardOpenOption>();
arry.add(READ);
arry.add(WRITE);
- return FileChannel.open(jsa.toPath(), new HashSet<StandardOpenOption>(arry));
+ return FileChannel.open(jsaFile.toPath(), new HashSet<StandardOpenOption>(arry));
}
- public static void modifyJsaContentRandomly() throws Exception {
- FileChannel fc = getFileChannel();
+ public static void modifyJsaContentRandomly(File jsaFile) throws Exception {
+ FileChannel fc = getFileChannel(jsaFile);
// corrupt random area in the data areas
long[] used = new long[num_regions]; // record used bytes
long start0, start, end, off;
@@ -210,8 +216,8 @@
return used;
}
- public static boolean modifyJsaContent(int region) throws Exception {
- FileChannel fc = getFileChannel();
+ public static boolean modifyJsaContent(int region, File jsaFile) throws Exception {
+ FileChannel fc = getFileChannel(jsaFile);
byte[] buf = new byte[4096];
ByteBuffer bbuf = ByteBuffer.wrap(buf);
@@ -248,8 +254,8 @@
return true;
}
- public static void modifyJsaHeader() throws Exception {
- FileChannel fc = getFileChannel();
+ public static void modifyJsaHeader(File jsaFile) throws Exception {
+ FileChannel fc = getFileChannel(jsaFile);
// screw up header info
byte[] buf = new byte[getFileHeaderSize(fc)];
ByteBuffer bbuf = ByteBuffer.wrap(buf);
@@ -259,6 +265,30 @@
}
}
+ public static void modifyJvmIdent() throws Exception {
+ FileChannel fc = getFileChannel(jsa);
+ int headerSize = getFileHeaderSize(fc);
+ System.out.println(" offset_jvm_ident " + offset_jvm_ident);
+ byte[] buf = new byte[256];
+ ByteBuffer bbuf = ByteBuffer.wrap(buf);
+ writeData(fc, (long)offset_jvm_ident, bbuf);
+ if (fc.isOpen()) {
+ fc.close();
+ }
+ }
+
+ public static void modifyHeaderIntField(long offset, int value) throws Exception {
+ FileChannel fc = getFileChannel(jsa);
+ int headerSize = getFileHeaderSize(fc);
+ System.out.println(" offset " + offset);
+ byte[] buf = ByteBuffer.allocate(4).putInt(value).array();
+ ByteBuffer bbuf = ByteBuffer.wrap(buf);
+ writeData(fc, offset, bbuf);
+ if (fc.isOpen()) {
+ fc.close();
+ }
+ }
+
public static void copyFile(File from, File to) throws Exception {
if (to.exists()) {
if(!to.delete()) {
@@ -348,10 +378,10 @@
// test, should pass
System.out.println("1. Normal, should pass but may fail\n");
- String[] execArgs = {"-cp", jarFile, "Hello"};
+ String[] execArgs = {"-Xlog:cds", "-cp", jarFile, "Hello"};
// tests that corrupt contents of the archive need to run with
// VerifySharedSpaces enabled to detect inconsistencies
- String[] verifyExecArgs = {"-XX:+VerifySharedSpaces", "-cp", jarFile, "Hello"};
+ String[] verifyExecArgs = {"-Xlog:cds", "-XX:+VerifySharedSpaces", "-cp", jarFile, "Hello"};
OutputAnalyzer output = TestCommon.execCommon(execArgs);
@@ -373,10 +403,36 @@
orgJsaFile = new File(new File(currentDir), "appcds.jsa.bak");
copyFile(jsa, orgJsaFile);
-
// modify jsa header, test should fail
System.out.println("\n2. Corrupt header, should fail\n");
- modifyJsaHeader();
+ modifyJsaHeader(jsa);
+ output = TestCommon.execCommon(execArgs);
+ output.shouldContain("The shared archive file has a bad magic number");
+ output.shouldNotContain("Checksum verification failed");
+
+ copyFile(orgJsaFile, jsa);
+ // modify _jvm_ident and _paths_misc_info_size, test should fail
+ System.out.println("\n2a. Corrupt _jvm_ident and _paths_misc_info_size, should fail\n");
+ modifyJvmIdent();
+ modifyHeaderIntField(offset_paths_misc_info_size, Integer.MAX_VALUE);
+ output = TestCommon.execCommon(execArgs);
+ output.shouldContain("The shared archive file was created by a different version or build of HotSpot");
+ output.shouldNotContain("Checksum verification failed");
+
+ copyFile(orgJsaFile, jsa);
+ // modify _magic and _paths_misc_info_size, test should fail
+ System.out.println("\n2b. Corrupt _magic and _paths_misc_info_size, should fail\n");
+ modifyHeaderIntField(offset_magic, 0x00000000);
+ modifyHeaderIntField(offset_paths_misc_info_size, Integer.MAX_VALUE);
+ output = TestCommon.execCommon(execArgs);
+ output.shouldContain("The shared archive file has a bad magic number");
+ output.shouldNotContain("Checksum verification failed");
+
+ copyFile(orgJsaFile, jsa);
+ // modify _version and _paths_misc_info_size, test should fail
+ System.out.println("\n2c. Corrupt _version and _paths_misc_info_size, should fail\n");
+ modifyHeaderIntField(offset_version, 0x00000000);
+ modifyHeaderIntField(offset_paths_misc_info_size, Integer.MAX_VALUE);
output = TestCommon.execCommon(execArgs);
output.shouldContain("The shared archive file has the wrong version");
output.shouldNotContain("Checksum verification failed");
@@ -387,8 +443,9 @@
for (int i=0; i<num_regions; i++) {
newJsaFile = new File(TestCommon.getNewArchiveName(shared_region_name[i]));
copyFile(orgJsaFile, newJsaFile);
- if (modifyJsaContent(i)) {
- testAndCheck(execArgs);
+ TestCommon.setCurrentArchiveName(newJsaFile.toString());
+ if (modifyJsaContent(i, newJsaFile)) {
+ testAndCheck(verifyExecArgs);
}
}
@@ -396,15 +453,17 @@
System.out.println("\n4. Corrupt Header and Content, should fail\n");
newJsaFile = new File(TestCommon.getNewArchiveName("header-and-content"));
copyFile(orgJsaFile, newJsaFile);
- modifyJsaHeader();
- modifyJsaContent(0); // this will not be reached since failed on header change first
+ TestCommon.setCurrentArchiveName(newJsaFile.toString());
+ modifyJsaHeader(newJsaFile);
+ modifyJsaContent(0, newJsaFile); // this will not be reached since failed on header change first
output = TestCommon.execCommon(execArgs);
- output.shouldContain("The shared archive file has the wrong version");
+ output.shouldContain("The shared archive file has a bad magic number");
output.shouldNotContain("Checksum verification failed");
// delete bytes in data section
System.out.println("\n5. Delete bytes at beginning of data section, should fail\n");
copyFile(orgJsaFile, jsa, true);
+ TestCommon.setCurrentArchiveName(jsa.toString());
testAndCheck(verifyExecArgs);
// insert bytes in data section forward
@@ -415,7 +474,8 @@
System.out.println("\n7. modify Content in random areas, should fail\n");
newJsaFile = new File(TestCommon.getNewArchiveName("random-areas"));
copyFile(orgJsaFile, newJsaFile);
- modifyJsaContentRandomly();
+ TestCommon.setCurrentArchiveName(newJsaFile.toString());
+ modifyJsaContentRandomly(newJsaFile);
testAndCheck(verifyExecArgs);
}
}
--- a/test/hotspot/jtreg/runtime/appcds/TestCommon.java Fri Jul 12 10:57:27 2019 +0200
+++ b/test/hotspot/jtreg/runtime/appcds/TestCommon.java Fri Jul 12 08:40:37 2019 -0700
@@ -90,6 +90,10 @@
return currentArchiveName;
}
+ public static void setCurrentArchiveName(String archiveName) {
+ currentArchiveName = archiveName;
+ }
+
public static String getNewArchiveName() {
return getNewArchiveName(null);
}