Skip to content

Add SafeAccess protection and frame type validation for signal handlers #349

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

Merged
jbachorik merged 4 commits into from
Feb 4, 2026
Merged

Add SafeAccess protection and frame type validation for signal handlers #349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
91a04d4
Use SafeAccess for frame pointer dereference in walkFP
jbachorik Jan 30, 2026
b0fc682
Sanitize J9 frame types to prevent invalid FrameTypeId values
jbachorik Jan 30, 2026
fcdfdd1
Use SafeAccess for NativeFunc::read_mark() in signal handler
jbachorik Feb 2, 2026
a26a694
Merge branch 'main' into jb/fp_safeaccess
jbachorik Feb 3, 2026
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
16 changes: 16 additions & 0 deletions ddprof-lib/src/main/cpp/codeCache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "codeCache.h"
#include "dwarf.h"
#include "os.h"
#include "safeAccess.h"

#include <stdint.h>
#include <stdlib.h>
Expand All @@ -23,6 +24,21 @@ char *NativeFunc::create(const char *name, short lib_index) {

void NativeFunc::destroy(char *name) { free(from(name)); }

char NativeFunc::read_mark(const char* name) {
if (name == nullptr) {
return 0;
}
NativeFunc* func = from(name);
if (!is_aligned(func, sizeof(func))) {
return 0;
}
// Use SafeAccess to read the mark field in signal handler context
// Read the first 4 bytes (lib_index + mark + reserved) and extract the mark byte
int32_t prefix = SafeAccess::safeFetch32((int32_t*)func, 0);
// Extract mark byte: shift right by 16 bits to skip lib_index (2 bytes), mask to 1 byte
return (char)((prefix >> 16) & 0xFF);
}

CodeCache::CodeCache(const char *name, short lib_index,
const void *min_address, const void *max_address,
const char* image_base, bool imports_patchable) {
Expand Down
11 changes: 1 addition & 10 deletions ddprof-lib/src/main/cpp/codeCache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,7 @@ class NativeFunc {
return read_mark(name) != 0;
}

static char read_mark(const char* name) {
if (name == nullptr) {
return 0;
}
NativeFunc* func = from(name);
if (!is_aligned(func, sizeof(func))) {
return 0;
}
return func->_mark;
}
static char read_mark(const char* name);

static void set_mark(const char* name, char value) {
if (name == nullptr) {
Expand Down
9 changes: 8 additions & 1 deletion ddprof-lib/src/main/cpp/frame.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ enum FrameTypeId {
FRAME_KERNEL = 5,
FRAME_C1_COMPILED = 6,
FRAME_NATIVE_REMOTE = 7, // Native frame with remote symbolication (build-id + pc-offset)
FRAME_TYPE_MAX = FRAME_NATIVE_REMOTE // Maximum valid frame type
};

class FrameType {
Expand All @@ -19,7 +20,13 @@ class FrameType {
}

static inline FrameTypeId decode(int bci) {
return (bci >> 24) > 0 ? (FrameTypeId)(bci >> 25) : FRAME_JIT_COMPILED;
if ((bci >> 24) <= 0) {
// Unencoded BCI (bit 24 not set) or negative special BCI values
return FRAME_JIT_COMPILED;
}
// Clamp to valid FrameTypeId range to defend against corrupted values
int raw_type = bci >> 25;
return (FrameTypeId)(raw_type <= FRAME_TYPE_MAX ? raw_type : FRAME_TYPE_MAX);
}
};

Expand Down
30 changes: 29 additions & 1 deletion ddprof-lib/src/main/cpp/j9Ext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,34 @@ struct jvmtiStackInfoExtended {

enum { SHOW_COMPILED_FRAMES = 4, SHOW_INLINED_FRAMES = 8 };

/**
* J9 frame type constants from ibmjvmti.h.
* These are the expected values returned in jvmtiFrameInfoExtended.type.
*/
enum J9FrameType {
J9_FRAME_NOT_JITTED = 0, // COM_IBM_STACK_FRAME_EXTENDED_NOT_JITTED
J9_FRAME_JITTED = 1, // COM_IBM_STACK_FRAME_EXTENDED_JITTED
J9_FRAME_INLINED = 2 // COM_IBM_STACK_FRAME_EXTENDED_INLINED
};

/**
* Validates and maps J9 frame type to FrameTypeId.
* J9's JVMTI extension may return unexpected values in the type field.
* This function ensures we only pass valid values to FrameType::encode().
*
* @param j9_type The frame type value from jvmtiFrameInfoExtended.type
* @return A valid FrameTypeId (FRAME_INTERPRETED, FRAME_JIT_COMPILED, or FRAME_INLINED)
*/
static inline int sanitizeJ9FrameType(jint j9_type) {
// J9 should only return 0, 1, or 2 for the frame type.
// Any other value is unexpected and we default to JIT compiled.
if (j9_type >= J9_FRAME_NOT_JITTED && j9_type <= J9_FRAME_INLINED) {
return j9_type; // Direct mapping: J9 values match our FrameTypeId values
}
// Unexpected value - default to JIT compiled as the safest assumption
return FRAME_JIT_COMPILED;
}

class J9Ext {
private:
static jvmtiEnv *_jvmti;
Expand Down Expand Up @@ -139,7 +167,7 @@ class J9Ext {
for (int j = 0; j < *count_ptr; j++) {
jvmtiFrameInfoExtended *fi = &buffer[j];
frame_buffer[j].method_id = fi->method;
frame_buffer[j].bci = FrameType::encode(fi->type, fi->location);
frame_buffer[j].bci = FrameType::encode(sanitizeJ9FrameType(fi->type), fi->location);
}
return JVMTI_ERROR_NONE;
}
Expand Down
2 changes: 1 addition & 1 deletion ddprof-lib/src/main/cpp/j9WallClock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ void J9WallClock::timerLoop() {
for (int j = 0; j < si->frame_count; j++) {
jvmtiFrameInfoExtended *fi = &si->frame_buffer[j];
frames[j].method_id = fi->method;
frames[j].bci = FrameType::encode(fi->type, fi->location);
frames[j].bci = FrameType::encode(sanitizeJ9FrameType(fi->type), fi->location);
}

int tid = J9Ext::GetOSThreadID(si->thread);
Expand Down
2 changes: 1 addition & 1 deletion ddprof-lib/src/main/cpp/stackWalker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ int StackWalker::walkFP(void* ucontext, const void** callchain, int max_depth, S
}

sp = fp + (FRAME_PC_SLOT + 1) * sizeof(void*);
fp = *(uintptr_t*)fp;
fp = (uintptr_t)SafeAccess::load((void**)fp);
}

if (truncated && depth > max_depth) {
Expand Down
43 changes: 37 additions & 6 deletions utils/run-docker-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#
# Usage: ./utils/run-docker-tests.sh [options]
# --libc=glibc|musl (default: glibc)
# --jdk=8|11|17|21|25 (default: 21)
# --jdk=8|11|17|21|25|8-j9|11-j9|17-j9|21-j9 (default: 21)
# --arch=x64|aarch64 (default: auto-detect)
# --config=debug|release (default: debug)
# --tests="TestPattern" (optional, specific test to run)
Expand Down Expand Up @@ -94,6 +94,24 @@ get_glibc_jdk_url() {
esac
}

# JDK Download URLs (IBM Semeru OpenJ9)
get_j9_jdk_url() {
local version=$1
local arch=$2

case "$version-$arch" in
8-x64) echo "https://github.com/ibmruntimes/semeru8-binaries/releases/download/jdk8u482-b08_openj9-0.57.0/ibm-semeru-open-jdk_x64_linux_8u482b08_openj9-0.57.0.tar.gz" ;;
8-aarch64) echo "https://github.com/ibmruntimes/semeru8-binaries/releases/download/jdk8u482-b08_openj9-0.57.0/ibm-semeru-open-jdk_aarch64_linux_8u482b08_openj9-0.57.0.tar.gz" ;;
11-x64) echo "https://github.com/ibmruntimes/semeru11-binaries/releases/download/jdk-11.0.30%2B7_openj9-0.57.0/ibm-semeru-open-jdk_x64_linux_11.0.30_7_openj9-0.57.0.tar.gz" ;;
11-aarch64) echo "https://github.com/ibmruntimes/semeru11-binaries/releases/download/jdk-11.0.30%2B7_openj9-0.57.0/ibm-semeru-open-jdk_aarch64_linux_11.0.30_7_openj9-0.57.0.tar.gz" ;;
17-x64) echo "https://github.com/ibmruntimes/semeru17-binaries/releases/download/jdk-17.0.18%2B8_openj9-0.57.0/ibm-semeru-open-jdk_x64_linux_17.0.18_8_openj9-0.57.0.tar.gz" ;;
17-aarch64) echo "https://github.com/ibmruntimes/semeru17-binaries/releases/download/jdk-17.0.18%2B8_openj9-0.57.0/ibm-semeru-open-jdk_aarch64_linux_17.0.18_8_openj9-0.57.0.tar.gz" ;;
21-x64) echo "https://github.com/ibmruntimes/semeru21-binaries/releases/download/jdk-21.0.10%2B7_openj9-0.57.0/ibm-semeru-open-jdk_x64_linux_21.0.9_10_openj9-0.56.0.tar.gz" ;;
21-aarch64) echo "https://github.com/ibmruntimes/semeru21-binaries/releases/download/jdk-21.0.10%2B7_openj9-0.57.0/ibm-semeru-open-jdk_aarch64_linux_21.0.9_10_openj9-0.56.0.tar.gz" ;;
*) echo "" ;;
esac
}

usage() {
head -n 19 "$0" | tail -n 16
exit 0
Expand Down Expand Up @@ -173,15 +191,28 @@ if [[ "$CONFIG" != "debug" && "$CONFIG" != "release" ]]; then
exit 1
fi

# Get JDK URL based on libc
if [[ "$LIBC" == "musl" ]]; then
JDK_URL=$(get_musl_jdk_url "$JDK_VERSION" "$ARCH")
# Parse JDK version and variant (e.g., "21-j9" -> version="21", variant="j9")
JDK_BASE_VERSION="${JDK_VERSION%%-*}"
JDK_VARIANT="${JDK_VERSION#*-}"
if [[ "$JDK_VARIANT" == "$JDK_VERSION" ]]; then
JDK_VARIANT="" # No variant specified
fi

# Get JDK URL based on variant and libc
if [[ "$JDK_VARIANT" == "j9" ]]; then
if [[ "$LIBC" == "musl" ]]; then
echo "Error: J9/OpenJ9 is not available for musl libc"
exit 1
fi
JDK_URL=$(get_j9_jdk_url "$JDK_BASE_VERSION" "$ARCH")
elif [[ "$LIBC" == "musl" ]]; then
JDK_URL=$(get_musl_jdk_url "$JDK_BASE_VERSION" "$ARCH")
else
JDK_URL=$(get_glibc_jdk_url "$JDK_VERSION" "$ARCH")
JDK_URL=$(get_glibc_jdk_url "$JDK_BASE_VERSION" "$ARCH")
fi

if [[ -z "$JDK_URL" ]]; then
echo "Error: --jdk must be one of: 8, 11, 17, 21, 25"
echo "Error: --jdk must be one of: 8, 11, 17, 21, 25, 8-j9, 11-j9, 17-j9, 21-j9"
exit 1
fi

Expand Down
Loading