From f79ff28dc2f8a7755ec670f2a5f380ef5a7a8473 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Thu, 27 Aug 2026 20:18:15 +0000 Subject: [PATCH 1/4] Possible out-of-bounds Context read --- ddprof-lib/src/main/cpp/context.h | 2 +- ddprof-lib/src/main/cpp/profiler.cpp | 5 + .../TooManyContextAttributesTest.java | 112 ++++++++++++++++++ 3 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/TooManyContextAttributesTest.java diff --git a/ddprof-lib/src/main/cpp/context.h b/ddprof-lib/src/main/cpp/context.h index ca4e3d5d82..b75facf9f2 100644 --- a/ddprof-lib/src/main/cpp/context.h +++ b/ddprof-lib/src/main/cpp/context.h @@ -31,7 +31,7 @@ class alignas(DEFAULT_CACHE_LINE_SIZE) Context { u64 rootSpanId; Tag tags[DD_TAGS_CAPACITY]; - Tag get_tag(int i) { return tags[i]; } + Tag get_tag(int i) { return i >= 0 && (u32)i < DD_TAGS_CAPACITY ? tags[i] : Tag{0}; } }; #endif /* _CONTEXT_H */ diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 7f9f208bc0..74d29b3085 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -1632,6 +1632,11 @@ Error Profiler::start(Arguments &args, bool reset) { // Always enable library trap to catch wasmtime loading and patch its broken sigaction switchLibraryTrap(true); + if (args._context_attributes.size() > DD_TAGS_CAPACITY) { + Log::warn("attributes: %zu attributes requested but capacity is %u; extra attributes will be ignored", + args._context_attributes.size(), DD_TAGS_CAPACITY); + args._context_attributes.resize(DD_TAGS_CAPACITY); + } JfrMetadata::reset(); JfrMetadata::initialize(args._context_attributes); _num_context_attributes = args._context_attributes.size(); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/TooManyContextAttributesTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/TooManyContextAttributesTest.java new file mode 100644 index 0000000000..87568207d4 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/TooManyContextAttributesTest.java @@ -0,0 +1,112 @@ +/* + * Copyright 2026, Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.datadoghq.profiler; + +import org.junitpioneer.jupiter.RetryingTest; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.ThreadLocalRandom; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Regression test: {@code attributes=} used to accept more names than the native + * {@code DD_TAGS_CAPACITY} (context.h), and {@code Recording::writeContextSnapshot} + * (flightRecorder.cpp) looped over that unbounded count calling the unchecked + * {@code Context::get_tag(i)} on a fixed {@code Tag tags[DD_TAGS_CAPACITY]} array - + * reading past the {@code Context} struct into adjacent native memory on every + * {@code datadog.HeapLiveObject} event. + * + *

Requesting more attributes than the native capacity must no longer crash (an + * ASan build turns the out-of-bounds read into a heap-buffer-overflow abort) and the + * profiler must cap the attribute list it advertises/serializes at + * {@link JavaProfiler#MAX_CONTEXT_SLOTS}, keeping the JFR metadata schema and the + * per-event field count consistent. See {@link MaxContextSlotsTest} for the + * companion drift guard between {@code JavaProfiler.MAX_CONTEXT_SLOTS} and {@code DD_TAGS_CAPACITY}. + */ +public class TooManyContextAttributesTest extends AbstractProfilerTest { + + private static final int REQUESTED_ATTRIBUTES = JavaProfiler.MAX_CONTEXT_SLOTS + 3; + + @Override + protected String getProfilerCommand() { + String attrs = IntStream.range(0, REQUESTED_ATTRIBUTES) + .mapToObj(i -> "tag" + i) + .collect(Collectors.joining(";")); + // memory=...:L enables liveness tracking, which is the only path that writes + // datadog.HeapLiveObject events via the vulnerable Recording::writeContextSnapshot. + return "memory=256:L,attributes=" + attrs; + } + + @Override + protected boolean isPlatformSupported() { + // Liveness tracking requires Java 11+ and specific JVM types (see LivenessTrackingTest). + return !(Platform.isJavaVersion(8) || Platform.isJ9() || Platform.isZing()); + } + + @RetryingTest(5) + public void moreAttributesThanCapacityDoesNotCrashAndIsCapped() throws Exception { + // Generate enough live allocation volume to clear the 256 KB sampling interval many + // times over, mirroring the workload LivenessTrackingTest uses to reliably produce + // datadog.HeapLiveObject samples. + List liveObjects = new ArrayList<>(); + for (int i = 0; i < 1000; i++) { + for (int j = 0; j < 10; j++) { + liveObjects.add(new byte[ThreadLocalRandom.current().nextInt(1024, 4096)]); + } + } + Thread.sleep(100); + for (int i = 0; i < 6; i++) { + System.gc(); + Thread.sleep(100); + } + Thread.sleep(300); + + stopProfiler(); + assertFalse(liveObjects.isEmpty()); // keep allocations reachable through the GC/dump above + + // If the pre-fix out-of-bounds read had fired, an ASan build would already have + // aborted the JVM above. On any build, a mismatched schema/field count would make + // this parse fail or throw - reaching here with samples already proves the fix. + JfrEvents liveObjectEvents = verifyEvents("datadog.HeapLiveObject", false); + assertTrue(liveObjectEvents.hasItems(), "expected datadog.HeapLiveObject samples"); + + Set recordedContextAttributes = new HashSet<>(); + for (JfrEvent item : verifyEvents("jdk.ActiveSetting")) { + if ("contextattribute".equals(item.getString("name"))) { + recordedContextAttributes.add(item.getString("value")); + } + } + assertEquals(JavaProfiler.MAX_CONTEXT_SLOTS, recordedContextAttributes.size(), + "attributes= list must be capped at JavaProfiler.MAX_CONTEXT_SLOTS (" + JavaProfiler.MAX_CONTEXT_SLOTS + + "), got: " + recordedContextAttributes); + for (int i = 0; i < JavaProfiler.MAX_CONTEXT_SLOTS; i++) { + assertTrue(recordedContextAttributes.contains("tag" + i), + "expected tag" + i + " to survive capping, got: " + recordedContextAttributes); + } + for (int i = JavaProfiler.MAX_CONTEXT_SLOTS; i < REQUESTED_ATTRIBUTES; i++) { + assertFalse(recordedContextAttributes.contains("tag" + i), + "tag" + i + " exceeds capacity and must have been dropped"); + } + } +} From 8c820edeee4885a944ac777577417319eaafc5b3 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Tue, 1 Sep 2026 15:41:11 +0000 Subject: [PATCH 2/4] Tighten API --- ddprof-lib/src/main/cpp/context.h | 14 ++++++++++++-- ddprof-lib/src/main/cpp/flightRecorder.cpp | 2 +- ddprof-lib/src/main/cpp/threadLocalData.cpp | 2 +- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/ddprof-lib/src/main/cpp/context.h b/ddprof-lib/src/main/cpp/context.h index b75facf9f2..0245d101bb 100644 --- a/ddprof-lib/src/main/cpp/context.h +++ b/ddprof-lib/src/main/cpp/context.h @@ -18,6 +18,7 @@ #define _CONTEXT_H #include "arch.h" +#include static const u32 DD_TAGS_CAPACITY = 10; @@ -29,9 +30,18 @@ class alignas(DEFAULT_CACHE_LINE_SIZE) Context { public: u64 spanId; u64 rootSpanId; +private: Tag tags[DD_TAGS_CAPACITY]; - - Tag get_tag(int i) { return i >= 0 && (u32)i < DD_TAGS_CAPACITY ? tags[i] : Tag{0}; } +public: + u32 getTag(int i) { + assert(i >= 0 && (u32)i < DD_TAGS_CAPACITY); + return i >= 0 && (u32)i < DD_TAGS_CAPACITY ? tags[i].value : 0; + } + + void setTag(int i, u32 value) { + assert(i >= 0 && (u32)i < DD_TAGS_CAPACITY); + tags[i].value = value; + } }; #endif /* _CONTEXT_H */ diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index 6d6b46003b..99919ae343 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -2115,7 +2115,7 @@ void Recording::writeContextSnapshot(Buffer *buf, Context &context) { buf->putVar64(context.rootSpanId); for (size_t i = 0; i < Profiler::instance()->numContextAttributes(); i++) { - buf->putVar32(context.get_tag(i).value); + buf->putVar32(context.getTag(i)); } } diff --git a/ddprof-lib/src/main/cpp/threadLocalData.cpp b/ddprof-lib/src/main/cpp/threadLocalData.cpp index 07c63bf74c..21fa414508 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.cpp +++ b/ddprof-lib/src/main/cpp/threadLocalData.cpp @@ -140,7 +140,7 @@ Context ProfiledThread::snapshotContext(size_t numAttrs) { ctx.rootSpanId = root_span_id; size_t count = numAttrs < DD_TAGS_CAPACITY ? numAttrs : DD_TAGS_CAPACITY; for (size_t i = 0; i < count; i++) { - ctx.tags[i].value = _otel_tag_encodings[i]; + ctx.setTag(i, _otel_tag_encodings[i]); } } return ctx; From 503d41a5f19ef8fd9cf3784e54aa05898f68b8c6 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Tue, 1 Sep 2026 18:01:30 +0000 Subject: [PATCH 3/4] Fix --- ddprof-lib/src/main/cpp/context.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ddprof-lib/src/main/cpp/context.h b/ddprof-lib/src/main/cpp/context.h index 0245d101bb..cc958c955f 100644 --- a/ddprof-lib/src/main/cpp/context.h +++ b/ddprof-lib/src/main/cpp/context.h @@ -40,7 +40,9 @@ class alignas(DEFAULT_CACHE_LINE_SIZE) Context { void setTag(int i, u32 value) { assert(i >= 0 && (u32)i < DD_TAGS_CAPACITY); - tags[i].value = value; + if (i >= 0 && (u32)i < DD_TAGS_CAPACITY) { + tags[i].value = value; + } } }; From fdfffbe2f3c5f2f1eabf7cfcb282367458744423 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Tue, 1 Sep 2026 19:40:13 +0000 Subject: [PATCH 4/4] Dedup code --- ddprof-lib/src/main/cpp/context.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ddprof-lib/src/main/cpp/context.h b/ddprof-lib/src/main/cpp/context.h index cc958c955f..e914adb662 100644 --- a/ddprof-lib/src/main/cpp/context.h +++ b/ddprof-lib/src/main/cpp/context.h @@ -32,15 +32,19 @@ class alignas(DEFAULT_CACHE_LINE_SIZE) Context { u64 rootSpanId; private: Tag tags[DD_TAGS_CAPACITY]; + + static bool isValidIndex(int i) { + return i >= 0 && (u32)i < DD_TAGS_CAPACITY; + } public: u32 getTag(int i) { - assert(i >= 0 && (u32)i < DD_TAGS_CAPACITY); - return i >= 0 && (u32)i < DD_TAGS_CAPACITY ? tags[i].value : 0; + assert(isValidIndex(i)); + return isValidIndex(i) ? tags[i].value : 0; } void setTag(int i, u32 value) { - assert(i >= 0 && (u32)i < DD_TAGS_CAPACITY); - if (i >= 0 && (u32)i < DD_TAGS_CAPACITY) { + assert(isValidIndex(i)); + if (isValidIndex(i)) { tags[i].value = value; } }