[fault-injection] Unprotected access to JVM class loader data mutex and method list

ddprof-lib/src/main/cpp/context.h

@@ -18,6 +18,7 @@
 #define _CONTEXT_H
 
 #include "arch.h"
+#include <cassert>
 
 static const u32 DD_TAGS_CAPACITY = 10;
 
@@ -29,9 +30,24 @@ class alignas(DEFAULT_CACHE_LINE_SIZE) Context {
 public:
   u64 spanId;
   u64 rootSpanId;
+private:
   Tag tags[DD_TAGS_CAPACITY];
 
-  Tag get_tag(int i) { return tags[i]; }
+  static bool isValidIndex(int i) {
+    return i >= 0 && (u32)i < DD_TAGS_CAPACITY;
+  }
+public:
+  u32 getTag(int i) {
+    assert(isValidIndex(i));
+    return isValidIndex(i) ? tags[i].value : 0;
+  }
+
+  void setTag(int i, u32 value) {
+    assert(isValidIndex(i));
+    if (isValidIndex(i)) {
+      tags[i].value = value;
+    }
+  }
 };
 
 #endif /* _CONTEXT_H */

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));
   }
 }
 

ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp

@@ -1335,22 +1335,90 @@ int HotspotSupport::walkJavaStack(StackWalkRequest& request) {
   return java_frames;
 }
 
+class LockState {
+private:
+    VMClassLoaderData* volatile _cld;
+public:
+    LockState() : _cld(nullptr) {}
+    ~LockState() { reset(); }
+    void lock(VMClassLoaderData* cld);
+    void reset();
+};
+
+void LockState::lock(VMClassLoaderData* cld) {
+    cld->lock();
+    _cld = cld;
+}
+
+void LockState::reset() {
+    // Assume: if _cld->lock() did not fail, _cld->unlock() should not
+    // fail as well.
+    // The siglongjmp cannot protect _cld->lock()/unlock() calls
+    if (_cld != nullptr) {
+        _cld->unlock();
+        _cld = nullptr;
+    }
+}
+
 static void patchClassLoaderData(JNIEnv* jni, jclass klass) {
   bool needs_patch = VM::hotspot_version() == 8;
   if (needs_patch) {
     // Workaround for JVM bug https://bugs.openjdk.org/browse/JDK-8062116
     // Preallocate space for jmethodIDs at the beginning of the list (rather than at the end)
     // This is relevant only for JDK 8 - later versions do not have this bug
     if (VMStructs::hasClassLoaderData()) {
+      ProfiledThread* prof_thread = ProfiledThread::initCurrentThreadSignalSafe();
+      assert(prof_thread != nullptr);
+      JmpCtxScope jmp_scope(prof_thread);
+      sigjmp_buf crash_protection_ctx;
+      LockState state;
+      if (sigsetjmp(crash_protection_ctx, 1) != 0) {
+        jmp_scope.restore();
+        state.reset();        
+        return;
+      }
+      jmp_scope.install(&crash_protection_ctx);
       VMKlass *vmklass = VMKlass::fromJavaClass(jni, klass);
       int method_count = vmklass->methodCount();
       if (method_count > 0) {
-        VMClassLoaderData *cld = vmklass->classLoaderData();
-        cld->lock();
-        for (int i = 0; i < method_count; i += MethodList::SIZE) {
-          *cld->methodList() = new MethodList(*cld->methodList());
+        // patchClassLoaderData() re-runs for the same class on every ClassPrepare
+        // replay (profiler restart via loadAllMethodIDsIfNeeded()), RedefineClasses
+        // and RetransformClasses -- none of which change method_count in practice.
+        // Without this tag, each re-run would prepend another full set of
+        // MethodList blocks onto the classloader-wide list that nothing ever frees.
+        // The tag lives on the jclass itself, so it disappears with the class --
+        // no separate bookkeeping to leak or to clean up on unload.
+        jvmtiEnv* jvmti = VM::jvmti();
+        jlong already_patched = 0;
+        if (jvmti == nullptr || jvmti->GetTag(klass, &already_patched) != JVMTI_ERROR_NONE) {
+          already_patched = 0;
+        }
+        if (method_count > already_patched) {
+          VMClassLoaderData *cld = vmklass->classLoaderData();
+          if (cld == nullptr) {
+            return;
+          }
+          state.lock(cld);
+          // Re-check under cld's lock: the GetTag() above is not serialized
+          // against a concurrent patchClassLoaderData() call for the same
+          // class (e.g. RetransformClasses on one thread racing the <clinit>
+          // fallback on the JFR dump thread), so another caller may have
+          // already patched (and updated the tag) while this thread was
+          // waiting for the lock. Only the mutation below is exclusive, so
+          // the decision to mutate must be re-validated inside it.
+          if (jvmti == nullptr || jvmti->GetTag(klass, &already_patched) != JVMTI_ERROR_NONE) {
+            already_patched = 0;
+          }
+          if (method_count > already_patched) {
+            int i;
+            for (i = (int) already_patched; i < method_count; i += MethodList::SIZE) {
+              *cld->methodList() = new MethodList(*cld->methodList());
+            }
+            if (jvmti != nullptr) {
+              jvmti->SetTag(klass, i);
+            }
+          }
         }
-        cld->unlock();
       }
     }
   }

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();

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;

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.
+ *
+ * <p>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<byte[]> 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<String> 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");
+        }
+    }
+}