Skip to content

fix: free native context if a stream constructor fails after creating it#13

Merged
dfa1 merged 2 commits into
mainfrom
fix/stream-ctor-native-leaks
Jun 26, 2026
Merged

fix: free native context if a stream constructor fails after creating it#13
dfa1 merged 2 commits into
mainfrom
fix/stream-ctor-native-leaks

Conversation

@dfa1

@dfa1 dfa1 commented Jun 26, 2026

Copy link
Copy Markdown
Owner

In-depth memory-leak / security review of the zstd module

Bug found & fixed — native context leak on constructor failure

All four streaming wrappers — ZstdOutputStream, ZstdInputStream, ZstdCompressStream, ZstdDecompressStream — create a native ZSTD_CCtx/ZSTD_DCtx and then make a second native call (set compression level / load dictionary). If that second call threw (e.g. an out-of-range parameter, a malformed dictionary, OOM), the just-created context leaked:

  • the java.io streams closed only the Arena in their catch, never the cctx/dctx;
  • the segment drivers rethrew from the static create() helper before super(...) could hand the pointer to NativeObject for tracked cleanup.

Fix: free the context on every constructor error path. For the java.io streams the post-setup buffer allocations also moved inside the guarded block, so a failure there releases both the context and the arena. Low-probability paths, but a real native leak each time.

Reviewed and found clean

  • NativeObjectAtomicReference swap to NULL makes tryClose exactly-once and idempotent under repeated/concurrent close.
  • Context/dict creators (ZstdCompressCtx/ZstdDecompressCtx/ZstdCompressDict/ZstdDecompressDict) — allocate the native handle as the last step, so an earlier failure leaks nothing.
  • Zstd.copyIn/copyOutMath.toIntExact guards the >2 GiB overflow, max(len,1) avoids zero-length allocation.
  • Decode/encode entry pointsrequireNative on every segment, Objects.checkFromIndexSize on array ranges, NULL-pointer checks on create, negative-sentinel handling via Zstd.call.
  • Dictionary trainers — all arena-scoped; ZDICT writes into an arena buffer, no native handle to leak.
  • Loader — bundled-only, owner-only temp dir (hardened in security: bundled-only native loader; fix Sonar vuln + bug #8).

No integer-overflow, TOCTOU (beyond the already-hardened loader), or unbounded-read issues found (errorName reads a static zstd string).

Test

./mvnw -pl zstd,integration-tests -am verify — 138 unit + 79 integration green, checkstyle + javadoc clean.

🤖 Generated with Claude Code

dfa1 and others added 2 commits June 26, 2026 20:15
The four streaming wrappers create a native ZSTD_C/DCtx and then make a
further native call (set parameter / load dictionary). If that second
call threw, the freshly created context was leaked: the java.io streams
closed only the arena (not the cctx/dctx), and the segment drivers
rethrew from create() before super() could take ownership.

Free the context on every constructor error path, and move the java.io
streams' post-setup buffer allocations inside the guarded block so a
failure there frees the context and the arena too.

Found during a memory-leak / security review of the zstd module. The
rest of the module checked out: NativeObject's atomic-swap close is
exactly-once and idempotent; the ctx/dict creators allocate the native
handle as their last step; copy helpers guard with toIntExact and
requireNative; decode entry points validate bounds and native-ness.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Segment drivers now take ownership first (super(createCtx())) and run
setup in the constructor body, so a setup failure is released by the
existing close() — drops the duplicated free helper. The java.io streams
share one freeCctx/freeDctx best-effort helper between the constructor
error path and close().

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dfa1 dfa1 merged commit 49e01b4 into main Jun 26, 2026
1 check passed
@dfa1 dfa1 deleted the fix/stream-ctor-native-leaks branch June 26, 2026 18:21
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.

1 participant