Skip to content

Link libsqreen_jni with -z now to survive early GOT writes (APPSEC-69052) - #219

Merged
jandro996 merged 3 commits into
masterfrom
alejandro.gonzalez/APPSEC-69052-bind-now
Oct 30, 2026
Merged

jandro996 merged 3 commits into
masterfrom
alejandro.gonzalez/APPSEC-69052-bind-now

Conversation

@jandro996

@jandro996 jandro996 commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

What does this PR do?

Links libsqreen_jni.so with -Wl,-z,now -Wl,-z,relro on Linux (full RELRO / BIND_NOW), and adds two regression tests:

  • LazyBindingGotClobberTest reproduces the APPSEC-69052 crash deterministically. An LD_AUDIT module (src/test/c/got_clobber_audit.c) writes an absolute pointer into the malloc GOT slot of libsqreen_jni.so from la_objopen(), which runs after the library is mapped but before it is relocated. A child JVM then loads the library through Waf.initialize(false).
  • NativeLibBindNowTest parses the ELF dynamic section of every Linux libsqreen_jni.so on the classpath and requires BIND_NOW. It needs no compiler, so it also guards the manylinux release binaries in the Test matrix, where LazyBindingGotClobberTest is skipped.

Motivation

APPSEC-69052: SIGSEGV inside JNI_OnLoad while AppSec loads libsqreen_jni.so (linux-aarch64 glibc; seen on dd-trace-java 1.61.1 and on 1.67.1, which bundles libsqreen 17.5.0).

From the 1.67.1 crash register dump and the disassembly of the released 17.5.0 binary:

  • JNI_OnLoad+0xcb0 calls a helper that does bl malloc@plt at +0xc000; R30 = +0xc004.
  • The PLT stub loads the malloc GOT slot (R16 = base + 0x300b0) into x17 = 0x0001d9473aa63774, which is the faulting address.
  • That value is exactly base + a plausible absolute pointer.

The glibc builds use lazy binding (no BIND_NOW, .got.plt outside RELRO). In that mode glibc relocates JUMP_SLOT entries as *slot += l_addr (both aarch64 and x86_64). If any in-process component writes a GOT slot before the dynamic linker relocates the library, the slot becomes pointer + base and the first call through it crashes. With BIND_NOW, glibc overwrites the slot with the resolved symbol, so an early write is harmless. Full RELRO also turns a later wild write into a fault in the writer instead of in our library.

Which component writes the slot in production is still being investigated (the Datadog profiler, which patches GOT entries, is loaded in the crashing process). This change makes libsqreen_jni.so robust regardless.

The musl builds are already linked with BIND_NOW (Alpine toolchain default); only the glibc builds are affected.

Jira ticket: APPSEC-69052

Description of the Change

  • CMakeLists.txt: target_link_libraries(sqreen_jni PRIVATE -Wl,-z,now -Wl,-z,relro) in the existing non-MSVC/non-Apple block. target_link_options is not used because the minimum CMake version is 3.12.
  • src/test/c/got_clobber_audit.c: the LD_AUDIT module used by LazyBindingGotClobberTest, compiled by the test itself.
  • build.gradle: passes the path of the audit module source to the tests (gotClobberAuditSource).
  • LazyBindingGotClobberTest is skipped when not on Linux, on musl (no LD_AUDIT), when cc is unavailable, or under the ASan LD_PRELOAD. In CI it runs in Dev_Tests (ubuntu-22.04) and Coverage. It asserts that the audit module actually patched the slot, so it cannot pass without exercising the scenario. The child JVM output goes to a file and the wait is bounded with waitFor(timeout), so a hung child is always destroyed.

Additional Notes

CI evidence that the tests fail without the fix: run 40480401281 on a temporary commit that reverted only the CMakeLists.txt change (since dropped from the branch):

Job Result without -z now
Development tests (ubuntu-22.04) LazyBindingGotClobberTest and NativeLibBindNowTest fail (crash assertion after the GOT slot was patched); 203 other tests pass
Coverage same
ASAN on Linux NativeLibBindNowTest fails; LazyBindingGotClobberTest skipped (ASan LD_PRELOAD)

CI with the fix: run 40481133489 on 8ae7a07 is fully green, including the whole Test matrix on the release binaries built in CI (15 Linux jobs: glibc and musl, x86_64 and aarch64, HotSpot and Semeru, CentOS 6/7). There NativeLibBindNowTest is neither failed nor skipped (Gradle --info logs only skipped/failed tests), and LazyBindingGotClobberTest is skipped as expected (no C compiler in those images). In Development tests (ubuntu-22.04) and Coverage both new tests pass.

Verified locally in Linux containers:

Check Result
Released 17.5.0 jar + early GOT write, aarch64 SIGSEGV at libsqreen_jni.so+0xc000 <- JNI_OnLoad+0xcb0, same R16/R30 as production
Released 17.5.0 jar + early GOT write, x86_64 (emulated) SIGSEGV at libsqreen_jni.so+0xb85a, pc = written pointer + load base
manylinux aarch64 build without this change, both new tests both fail
manylinux aarch64 build with this change, both new tests both pass
Full test suite with the BIND_NOW library (aarch64, JDK 21) 205 tests, 0 failures (1 pre-existing skip)
./gradlew check (incl. CodeNarc, JDK 11) with the BIND_NOW library OK

The reproduction does not need -XX:+UseCompactObjectHeaders or JDK 25: it crashes on JDK 21 with default flags.

Ubuntu's gcc only enables -z now for PIE executables, so the debug library built in Dev_Tests is lazy-bound without this change, and LazyBindingGotClobberTest fails there.

Cost of BIND_NOW: 78 PLT symbols resolved once at dlopen. The library only requires GLIBC_2.17, and all imports (libc, libjvm, libddwaf) are always present.

Why full RELRO and not only -z now: with full RELRO the GOT becomes read-only after loading, so a component that patches it late must mprotect it first. Any GOT patcher already has to handle this, since many distro libraries are built with full RELRO; the Datadog profiler (ddprof) does import mprotect. A wild late write now faults in the writer instead of corrupting our library.

Out of scope, follow-up: the prebuilt libddwaf.so shipped from the libddwaf release tarball is also lazily bound (no BIND_NOW, 84-87 JUMP_SLOT relocations including malloc), so it is exposed to the same early-GOT-write corruption. It cannot be relinked here; it needs the same flag in the libddwaf build.

…052)

With lazy binding, glibc relocates PLT slots as "*slot += load_base".
If another in-process component writes a GOT slot of libsqreen_jni.so
before the dynamic linker relocates it, the slot ends up as a wild
pointer and the first call through it crashes inside JNI_OnLoad.
BIND_NOW makes glibc overwrite the slot with the resolved symbol, and
full RELRO keeps the GOT read-only afterwards.

Add LazyBindingGotClobberTest, which reproduces the crash with an
LD_AUDIT module that writes the malloc slot before relocation, and
NativeLibBindNowTest, which checks the release binaries for BIND_NOW.
Skip LazyBindingGotClobberTest when run outside Gradle, always destroy
the child JVM, keep the temp dir only on failure, and name the ELF
offsets parsed by NativeLibBindNowTest.
- Fix the CodeNarc violations that failed codenarcTest in every CI job
  running check (Println, BitwiseOperatorInConditional, NestedForLoop,
  UnnecessaryGetter, UnnecessaryGString)
- Redirect the child JVM output to a file and bound the wait with
  waitFor(timeout), so a hung child is always destroyed (a blocked pipe
  read ignored the JUnit timeout)
- Delete the temp dir when the test is skipped for lack of a C compiler
@jandro996
jandro996 force-pushed the alejandro.gonzalez/APPSEC-69052-bind-now branch 2 times, most recently from 2524212 to 8ae7a07 Compare October 27, 2026 09:29
@datadog-datadog-prod-us1-2

Copy link
Copy Markdown

Pipelines

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 56.46% (+0.05%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 8ae7a07 | Docs | Give us feedback!

@jandro996
jandro996 marked this pull request as ready for review October 27, 2026 10:39
@jandro996
jandro996 requested a review from a team as a code owner October 27, 2026 10:39
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-27T10:40:56.783592Z 8ae7a07 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@jandro996
jandro996 requested a review from smola October 27, 2026 12:42

@cataphract cataphract left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For the reason presented in the test, you only need -z now, though I don't see anything wrong with passing -z now -z relro to the linker. You might as well combine it with -fno-plt too.

I'm a bit worried, however, that you did not find the ultimate cause of the crash. There must be some other component misbehaving, something other than glibc writing to the GOT. If there is indeed a race:

  1. You clobber the other components pointers, if done before the relocation, losing an intended override.
  2. The other component crashes trying to write to read-only memory.

Can't you tell from the crash telemetry and the exact binary the symbol that corresponds to the corrupted got entry? Telemetry should also give you all the loaded components and you can usually infer their exact versions via the sizes of the mapped regions (provided you have access to these binaries ofc). With this, you could scan all the loaded binaries in search for the culprit.

@cataphract

Copy link
Copy Markdown
Contributor

I missed that the description already identifies the corrupted entry as the malloc GOT slot. So whatever wrote it most likely wanted to intercept allocations made by libsqreen_jni.so and a profiler that hooks malloc by patching GOT entries is the obvious suspect.

The crash dump also tells us what was written. With lazy binding glibc did *slot += l_addr on top of the early write, so:

x17 = P + l_addr   =>   P = x17 - l_addr

l_addr (the load bias of libsqreen_jni.so) is recoverable from the same dump: R16 holds the slot address, which is base + 0x300b0, so l_addr = R16 - 0x300b0. As a sanity check, x17 = 0x0001d9473aa63774 is above the 48-bit user VA limit on aarch64, which is what you'd expect from adding two user-space addresses.

Given P, look it up in the memory map from the crash telemetry.

  • If P falls inside some module's text, that module is very likely the culprit, and P is its malloc wrapper. The mapping sizes should pin down the exact version, and from there you can confirm in the binary that it patches GOT entries.
  • If P lands in libc's malloc, the writer stored the real address, e.g. when unhooking or restoring, and you'd have to check which of the loaded modules do GOT patching at all.

This is still somewhat speculative, but it should either confirm or rule out the profiler quickly.

It would also be good to understand how the writer gets to see libsqreen_jni.so after it is mapped but before it is relocated. For example, it might scan /proc/self/maps or enumerate loaded objects from another thread while the JVM is inside dlopen(). That race is a bug on the writer's side.

@jandro996

Copy link
Copy Markdown
Member Author

Thanks for the detailed review!

I followed your suggestion. From the crash registers:

  • l_addr = R16 - 0x300b0 = 0xeca390790000 (matches the base reported by the telemetry)
  • P = x17 - l_addr = 0xeca3aa2d3774

The telemetry only resolves one other module near it: libjvm.so at 0xeca3a85f0000, which would put P at libjvm+0x1ce3774. But the exact Zulu 25.36+15 aarch64 libjvm.so ends at 0x1523948, so P is not in libjvm. It also doesn't match malloc in the usual arm64 Debian/Ubuntu glibc builds (none ends in 0x3774), so it's either libc's malloc from another distro or a hook in some other module. Crash telemetry doesn't include the full memory map, so I've asked the service owners (k9-findings-platform, internal staging service) for the hs_err file and the profiler configuration to resolve P, and I'll follow up with the profiling team once we know the module.

On the side effects you mention:

  1. Losing an intended override: true for a write that lands before relocation, but today that same case is a SIGSEGV in JNI_OnLoad, so losing the hook is strictly better. A write that lands after relocation is preserved.
  2. Writer crashing on read-only memory: the old binary already had partial RELRO from the toolchain default, so with -z now the GOT ends up read-only either way; the explicit -z relro just makes that independent of the toolchain. ddprof makes the import range writable before patching (mprotect RW in CodeCache::makeImportsPatchable), and the musl builds have shipped with BIND_NOW + full RELRO without issues.

I'd keep -fno-plt out of this PR: it's a codegen change that would also need to apply to the libddwaf build, and it doesn't change this failure mode.

Are you ok with this? Do you think we can continue with this fix in the meantime?

@jandro996

Copy link
Copy Markdown
Member Author

Update: root cause confirmed with the service owners. The writer is the native ddprof profiler v0.19.0 (the JVM runs as ddprof --preset cpu_live_heap java ...). In v0.19.0's embedded libdd_profiling-embedded.so, ddprof::(anonymous namespace)::MallocHook::hook(unsigned long) is at 0x23774, so P - 0x23774 = 0xeca3aa2b0000 is a 64K-aligned lib base, and P + l_addr is exactly the x17 from the crash. Both occurrences (May and October) ran ddprof v0.19.0.

So it's the race you described: ddprof's symbol overrides patched our malloc slot after libsqreen_jni.so was mapped but before ld.so relocated it. I'm reporting it to the profiling team.

On your two concerns, now with the actual writer:

  1. Lost override: if the race hits, -z now makes ld.so overwrite ddprof's hook with the real malloc, so ddprof just won't track allocations from libsqreen_jni.so in that process. No crash.
  2. Read-only GOT: native ddprof calls mprotect(PROT_READ | PROT_WRITE) before each write (override_entry in src/lib/elfutils.cc), so full RELRO doesn't break it.

Does that sound OK to move forward with this PR?

@cataphract cataphract left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is PR harmless and likely works most of the time as a workaround for the rare cases when the race with the buggy profiler happens.

Depending exactly how ddprof works (I haven't checked), there may still be lingering problems, e.g. between the time ddprof mprotect(RW) and it does the write, glibc could make the pages read-only, but it likely is a rare event on top of a rare event.

@jandro996
jandro996 merged commit 0321e69 into master Oct 30, 2026
43 checks passed
@jandro996
jandro996 deleted the alejandro.gonzalez/APPSEC-69052-bind-now branch October 30, 2026 08:57
gh-worker-dd-mergequeue-cf854d Bot pushed a commit to DataDog/dd-trace-java that referenced this pull request Oct 30, 2026
Bump io.sqreen:libsqreen to 17.6.0

libsqreen 17.6.0 links libsqreen_jni.so with BIND_NOW (DataDog/libddwaf-java#219). This fixes a SIGSEGV in JNI_OnLoad when the native ddprof profiler writes its malloc hook into the lazily-bound GOT slot before ld.so relocates the library (APPSEC-69052).

Update lockfiles for libsqreen 17.6.0

Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants