Skip to content

fix: deep-copy Data in MakeHttpEvent to prevent nil pointer panic#804

Open
matthyx wants to merge 1 commit into
mainfrom
fix/http-tracer-nil-data-panic
Open

fix: deep-copy Data in MakeHttpEvent to prevent nil pointer panic#804
matthyx wants to merge 1 commit into
mainfrom
fix/http-tracer-nil-data-panic

Conversation

@matthyx
Copy link
Copy Markdown
Contributor

@matthyx matthyx commented May 12, 2026

Summary

  • MakeHttpEvent was assigning e.Data directly to the new DatasourceEvent, sharing the same pooled *data pointer
  • When Release() was called on the original bpfEvent, it zeroed the inner Data *DataElement field of the shared *data and returned the object to the pool
  • Any HttpEvent stored in the HTTP tracer's eventsMap then held a stale reference; transmitOrphanRequests subsequently called GetTimestamp()d.payload()d.Data.Payload where d.Data == nilSIGSEGV at addr=0x8

Fix: call e.Datasource.DeepCopy(e.Data) so the returned HttpEvent owns an independent copy of the data, completely decoupled from the original event's release lifecycle.

Stack trace

goroutine 988 [running]:
github.com/inspektor-gadget/inspektor-gadget/pkg/datasource.(*data).payload(...)
    datasource/data.go:83 +0x4           ← d.Data is nil after Release()
github.com/inspektor-gadget/inspektor-gadget/pkg/datasource.(*fieldAccessor).Get(...)
    datasource/accessors.go:217 +0x42
github.com/inspektor-gadget/inspektor-gadget/pkg/datasource.(*fieldAccessor).Uint64(...)
    datasource/accessors.go:463 +0x26
github.com/kubescape/node-agent/pkg/utils.(*DatasourceEvent).GetTimestamp(...)
    pkg/utils/datasource_event.go:740 +0xaf
(*HTTPTracer).transmitOrphanRequests(...)
    tracers/http.go:179 +0x124

Test plan

  • Verify build passes: go build ./pkg/utils/... ./pkg/containerwatcher/...
  • Run HTTP tracer under load and confirm no nil pointer panics in transmitOrphanRequests
  • Confirm orphan request timestamps are valid (non-zero)

🤖 Generated with Claude Code

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 12, 2026

Warning

Rate limit exceeded

@matthyx has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 53 minutes and 20 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e7f1e5a9-d72d-4968-bd58-cd53192936c6

📥 Commits

Reviewing files that changed from the base of the PR and between cc59fa0 and 008a6f0.

📒 Files selected for processing (1)
  • pkg/utils/datasource_event.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/http-tracer-nil-data-panic

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

MakeHttpEvent was sharing e.Data (a pooled *data pointer) with the
returned HttpEvent. When Release() was called on the original bpfEvent
after the HTTP tracer callback returned, it zeroed the shared *data's
inner Data field and returned it to the pool. Any stored HttpEvent in
eventsMap then held a dangling reference, causing a nil pointer
dereference (SIGSEGV) when transmitOrphanRequests called GetTimestamp().

Fix by calling Datasource.DeepCopy so the HttpEvent owns an independent
copy of Data, decoupled from the original event's lifecycle.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
@matthyx matthyx force-pushed the fix/http-tracer-nil-data-panic branch from b511089 to 008a6f0 Compare May 12, 2026 14:37
@matthyx matthyx moved this to Needs Reviewer in KS PRs tracking May 12, 2026
@github-actions
Copy link
Copy Markdown

Performance Benchmark Results

Node-Agent Resource Usage
Metric BEFORE AFTER Delta
Avg CPU (cores) 0.180 0.178 -1.2%
Peak CPU (cores) 0.191 0.189 -1.1%
Avg Memory (MiB) 347.211 281.722 -18.9%
Peak Memory (MiB) 350.441 292.316 -16.6%
Dedup Effectiveness (AFTER only)
Event Type Passed Deduped Ratio
capabilities 0 0 N/A
hardlink 6000 0 0.0%
http 1708 119455 98.6%
network 902 77938 98.9%
open 46605 624356 93.1%
symlink 6000 0 0.0%
syscall 977 1891 65.9%
Event Counters
Metric BEFORE AFTER
capability_counter 9 11
dns_counter 1411 1442
exec_counter 7086 7244
network_counter 93127 95234
open_counter 793856 814005
syscall_counter 3496 3634

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs Reviewer

Development

Successfully merging this pull request may close these issues.

1 participant