Skip to content

8361447: [REDO] Checked version of JNI Release<type>ArrayElements needs to filter out known wrapped arrays #26177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+345 −26
Open

8361447: [REDO] Checked version of JNI Release<type>ArrayElements needs to filter out known wrapped arrays #26177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/hotspot/share/memory/guardedMemory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@
#include "nmt/memTag.hpp"
#include "runtime/os.hpp"

void* GuardedMemory::wrap_copy(const void* ptr, const size_t len, const void* tag) {
void* GuardedMemory::wrap_copy(const void* ptr, const size_t len,
const void* tag, const void* tag2) {
size_t total_sz = GuardedMemory::get_total_size(len);
void* outerp = os::malloc(total_sz, mtInternal);
if (outerp != nullptr) {
GuardedMemory guarded(outerp, len, tag);
GuardedMemory guarded(outerp, len, tag, tag2);
void* innerp = guarded.get_user_ptr();
if (ptr != nullptr) {
memcpy(innerp, ptr, len);
Expand Down Expand Up @@ -58,8 +59,8 @@ void GuardedMemory::print_on(outputStream* st) const {
return;
}
st->print_cr("GuardedMemory(" PTR_FORMAT ") base_addr=" PTR_FORMAT
" tag=" PTR_FORMAT " user_size=%zu user_data=" PTR_FORMAT,
p2i(this), p2i(_base_addr), p2i(get_tag()), get_user_size(), p2i(get_user_ptr()));
" tag=" PTR_FORMAT " tag2=" PTR_FORMAT " user_size=%zu user_data=" PTR_FORMAT,
p2i(this), p2i(_base_addr), p2i(get_tag()), p2i(get_tag2()), get_user_size(), p2i(get_user_ptr()));

Guard* guard = get_head_guard();
st->print_cr(" Header guard @" PTR_FORMAT " is %s", p2i(guard), (guard->verify() ? "OK" : "BROKEN"));
Expand Down
44 changes: 38 additions & 6 deletions src/hotspot/share/memory/guardedMemory.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2014, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2014, 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
Expand All @@ -26,6 +26,7 @@
#define SHARE_MEMORY_GUARDEDMEMORY_HPP

#include "memory/allocation.hpp"
#include "runtime/os.hpp"
#include "utilities/globalDefinitions.hpp"

/**
Expand All @@ -43,13 +44,14 @@
* |base_addr | 0xABABABABABABABAB | Head guard |
* |+16 | <size_t:user_size> | User data size |
* |+sizeof(uintptr_t) | <tag> | Tag word |
* |+sizeof(uintptr_t) | <tag2> | Tag word |
* |+sizeof(void*) | 0xF1 <user_data> ( | User data |
* |+user_size | 0xABABABABABABABAB | Tail guard |
* -------------------------------------------------------------
*
* Where:
* - guard padding uses "badResourceValue" (0xAB)
* - tag word is general purpose
* - tag word and tag2 word are general purpose
* - user data
* -- initially padded with "uninitBlockPad" (0xF1),
* -- to "freeBlockPad" (0xBA), when freed
Expand Down Expand Up @@ -111,6 +113,10 @@ class GuardedMemory : StackObj { // Wrapper on stack
}

bool verify() const {
// We may not be able to dereference directly.
if (!os::is_readable_range((const void*) _guard, (const void*) (_guard + GUARD_SIZE))) {
return false;
}
u_char* c = (u_char*) _guard;
u_char* end = c + GUARD_SIZE;
while (c < end) {
Expand All @@ -137,13 +143,18 @@ class GuardedMemory : StackObj { // Wrapper on stack
size_t _user_size;
};
void* _tag;
void* _tag2;
public:
void set_user_size(const size_t usz) { _user_size = usz; }
size_t get_user_size() const { return _user_size; }

void set_tag(const void* tag) { _tag = (void*) tag; }
void* get_tag() const { return _tag; }

void set_tag2(const void* tag2) { _tag2 = (void*) tag2; }
void* get_tag2() const { return _tag2; }


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe remove extra empty lines?

}; // GuardedMemory::GuardHeader

// Guarded Memory...
Expand All @@ -162,9 +173,11 @@ class GuardedMemory : StackObj { // Wrapper on stack
* @param base_ptr allocation wishing to be wrapped, must be at least "GuardedMemory::get_total_size()" bytes.
* @param user_size the size of the user data to be wrapped.
* @param tag optional general purpose tag.
* @param tag2 optional second general purpose tag.
*/
GuardedMemory(void* base_ptr, const size_t user_size, const void* tag = nullptr) {
wrap_with_guards(base_ptr, user_size, tag);
GuardedMemory(void* base_ptr, const size_t user_size,
const void* tag = nullptr, const void* tag2 = nullptr) {
wrap_with_guards(base_ptr, user_size, tag, tag2);
}

/**
Expand All @@ -189,16 +202,19 @@ class GuardedMemory : StackObj { // Wrapper on stack
* @param base_ptr allocation wishing to be wrapped, must be at least "GuardedMemory::get_total_size()" bytes.
* @param user_size the size of the user data to be wrapped.
* @param tag optional general purpose tag.
* @param tag2 optional second general purpose tag.
*
* @return user data pointer (inner pointer to supplied "base_ptr").
*/
void* wrap_with_guards(void* base_ptr, size_t user_size, const void* tag = nullptr) {
void* wrap_with_guards(void* base_ptr, size_t user_size,
const void* tag = nullptr, const void* tag2 = nullptr) {
assert(base_ptr != nullptr, "Attempt to wrap null with memory guard");
_base_addr = (u_char*)base_ptr;
get_head_guard()->build();
get_head_guard()->set_user_size(user_size);
get_tail_guard()->build();
set_tag(tag);
set_tag2(tag2);
set_user_bytes(uninitBlockPad);
assert(verify_guards(), "Expected valid memory guards");
return get_user_ptr();
Expand Down Expand Up @@ -230,6 +246,20 @@ class GuardedMemory : StackObj { // Wrapper on stack
*/
void* get_tag() const { return get_head_guard()->get_tag(); }

/**
* Set the second general purpose tag.
*
* @param tag general purpose tag.
*/
void set_tag2(const void* tag) { get_head_guard()->set_tag2(tag); }

/**
* Return the second general purpose tag.
*
* @return the second general purpose tag, defaults to null.
*/
void* get_tag2() const { return get_head_guard()->get_tag2(); }

/**
* Return the size of the user data.
*
Expand Down Expand Up @@ -302,10 +332,12 @@ class GuardedMemory : StackObj { // Wrapper on stack
* @param ptr the memory to be copied
* @param len the length of the copy
* @param tag optional general purpose tag (see GuardedMemory::get_tag())
* @param tag2 optional general purpose tag (see GuardedMemory::get_tag2())
*
* @return guarded wrapped memory pointer to the user area, or null if OOM.
*/
static void* wrap_copy(const void* p, const size_t len, const void* tag = nullptr);
static void* wrap_copy(const void* p, const size_t len,
const void* tag = nullptr, const void* tag2 = nullptr);

/**
* Free wrapped copy.
Expand Down
48 changes: 37 additions & 11 deletions src/hotspot/share/prims/jniCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,24 +350,33 @@ check_is_obj_array(JavaThread* thr, jarray jArray) {
}
}

// Arbitrary (but well-known) tag for GetStringChars
const void* STRING_TAG = (void*)0x47114711;

// Arbitrary (but well-known) tag for GetStringUTFChars
const void* STRING_UTF_TAG = (void*) 0x48124812;

// Arbitrary (but well-known) tag for GetPrimitiveArrayCritical
const void* CRITICAL_TAG = (void*)0x49134913;

/*
* Copy and wrap array elements for bounds checking.
* Remember the original elements (GuardedMemory::get_tag())
*/
static void* check_jni_wrap_copy_array(JavaThread* thr, jarray array,
void* orig_elements) {
void* orig_elements, jboolean is_critical = JNI_FALSE) {
void* result;
IN_VM(
oop a = JNIHandles::resolve_non_null(array);
size_t len = arrayOop(a)->length() <<
TypeArrayKlass::cast(a->klass())->log2_element_size();
result = GuardedMemory::wrap_copy(orig_elements, len, orig_elements);
result = GuardedMemory::wrap_copy(orig_elements, len, orig_elements, is_critical ? CRITICAL_TAG : nullptr);
)
return result;
}

static void* check_wrapped_array(JavaThread* thr, const char* fn_name,
void* obj, void* carray, size_t* rsz) {
void* obj, void* carray, size_t* rsz, jboolean is_critical) {
if (carray == nullptr) {
tty->print_cr("%s: elements vector null" PTR_FORMAT, fn_name, p2i(obj));
NativeReportJNIFatalError(thr, "Elements vector null");
Expand All @@ -386,6 +395,29 @@ static void* check_wrapped_array(JavaThread* thr, const char* fn_name,
DEBUG_ONLY(guarded.print_on(tty);) // This may crash.
NativeReportJNIFatalError(thr, err_msg("%s: unrecognized elements", fn_name));
}
if (orig_result == STRING_TAG || orig_result == STRING_UTF_TAG) {
bool was_utf = orig_result == STRING_UTF_TAG;
tty->print_cr("%s: called on something allocated by %s",
fn_name, was_utf ? "GetStringUTFChars" : "GetStringChars");
DEBUG_ONLY(guarded.print_on(tty);) // This may crash.
NativeReportJNIFatalError(thr, err_msg("%s called on something allocated by %s",
fn_name, was_utf ? "GetStringUTFChars" : "GetStringChars"));
}

if (is_critical && (guarded.get_tag2() != CRITICAL_TAG)) {
tty->print_cr("%s: called on something not allocated by GetPrimitiveArrayCritical", fn_name);
DEBUG_ONLY(guarded.print_on(tty);) // This may crash.
NativeReportJNIFatalError(thr, err_msg("%s called on something not allocated by GetPrimitiveArrayCritical",
fn_name));
}

if (!is_critical && (guarded.get_tag2() == CRITICAL_TAG)) {
tty->print_cr("%s: called on something allocated by GetPrimitiveArrayCritical", fn_name);
DEBUG_ONLY(guarded.print_on(tty);) // This may crash.
NativeReportJNIFatalError(thr, err_msg("%s called on something allocated by GetPrimitiveArrayCritical",
fn_name));
}

if (rsz != nullptr) {
*rsz = guarded.get_user_size();
}
Expand All @@ -395,7 +427,7 @@ static void* check_wrapped_array(JavaThread* thr, const char* fn_name,
static void* check_wrapped_array_release(JavaThread* thr, const char* fn_name,
void* obj, void* carray, jint mode, jboolean is_critical) {
size_t sz;
void* orig_result = check_wrapped_array(thr, fn_name, obj, carray, &sz);
void* orig_result = check_wrapped_array(thr, fn_name, obj, carray, &sz, is_critical);
switch (mode) {
case 0:
memcpy(orig_result, carray, sz);
Expand Down Expand Up @@ -1430,9 +1462,6 @@ JNI_ENTRY_CHECKED(jsize,
return result;
JNI_END

// Arbitrary (but well-known) tag
const void* STRING_TAG = (void*)0x47114711;

JNI_ENTRY_CHECKED(const jchar *,
checked_jni_GetStringChars(JNIEnv *env,
jstring str,
Expand Down Expand Up @@ -1535,9 +1564,6 @@ JNI_ENTRY_CHECKED(jlong,
return result;
JNI_END

// Arbitrary (but well-known) tag - different than GetStringChars
const void* STRING_UTF_TAG = (void*) 0x48124812;

JNI_ENTRY_CHECKED(const char *,
checked_jni_GetStringUTFChars(JNIEnv *env,
jstring str,
Expand Down Expand Up @@ -1859,7 +1885,7 @@ JNI_ENTRY_CHECKED(void *,
)
void *result = UNCHECKED()->GetPrimitiveArrayCritical(env, array, isCopy);
if (result != nullptr) {
result = check_jni_wrap_copy_array(thr, array, result);
result = check_jni_wrap_copy_array(thr, array, result, JNI_TRUE);
}
functionExit(thr);
return result;
Expand Down
17 changes: 12 additions & 5 deletions test/hotspot/gtest/memory/test_guardedMemory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ TEST(GuardedMemory, size) {
}

// Test the basic characteristics
TEST(GuardedMemory, basic) {
TEST_VM(GuardedMemory, basic) {
u_char* basep =
(u_char*) os::malloc(GuardedMemory::get_total_size(1), mtInternal);
GuardedMemory guarded(basep, 1, GEN_PURPOSE_TAG);
Expand All @@ -78,7 +78,7 @@ TEST(GuardedMemory, basic) {
}

// Test a number of odd sizes
TEST(GuardedMemory, odd_sizes) {
TEST_VM(GuardedMemory, odd_sizes) {
u_char* basep =
(u_char*) os::malloc(GuardedMemory::get_total_size(1), mtInternal);
GuardedMemory guarded(basep, 1, GEN_PURPOSE_TAG);
Expand All @@ -99,7 +99,7 @@ TEST(GuardedMemory, odd_sizes) {
}

// Test buffer overrun into head...
TEST(GuardedMemory, buffer_overrun_head) {
TEST_VM(GuardedMemory, buffer_overrun_head) {
u_char* basep =
(u_char*) os::malloc(GuardedMemory::get_total_size(1), mtInternal);
GuardedMemory guarded(basep, 1, GEN_PURPOSE_TAG);
Expand All @@ -111,7 +111,7 @@ TEST(GuardedMemory, buffer_overrun_head) {
}

// Test buffer overrun into tail with a number of odd sizes
TEST(GuardedMemory, buffer_overrun_tail) {
TEST_VM(GuardedMemory, buffer_overrun_tail) {
u_char* basep =
(u_char*) os::malloc(GuardedMemory::get_total_size(1), mtInternal);
GuardedMemory guarded(basep, 1, GEN_PURPOSE_TAG);
Expand All @@ -128,7 +128,7 @@ TEST(GuardedMemory, buffer_overrun_tail) {
}

// Test wrap_copy/wrap_free
TEST(GuardedMemory, wrap) {
TEST_VM(GuardedMemory, wrap) {
EXPECT_TRUE(GuardedMemory::free_copy(nullptr)) << "Expected free nullptr to be OK";

const char* str = "Check my bounds out";
Expand All @@ -146,3 +146,10 @@ TEST(GuardedMemory, wrap) {
EXPECT_TRUE(GuardedMemory::free_copy(no_data_copy))
<< "Expected valid guards even for no data copy";
}

// Test passing back a bogus GuardedMemory region
TEST_VM(GuardedMemory, unmapped) {
char* unmapped_base = (char*) (GuardedMemoryTest::get_guard_header_size() + 0x1000 + 1); // Avoids assert in constructor
GuardedMemory guarded(unmapped_base);
EXPECT_FALSE(guarded.verify_guards()) << "Guard was not broken as expected";
}
Loading