Conversation
📝 WalkthroughWalkthroughIntroduces buf_len field tracking to HTTP events across eBPF kernel and Go userspace layers. The httpevent struct in eBPF now stores the actual amount of data copied; Go-side accessors expose this value, and a GetValidBuf helper uses buf_len to parse only valid buffer portions during HTTP request/response handling. Changes
Sequence DiagramsequenceDiagram
participant eBPF as eBPF Kernel
participant RingBuf as Ring Buffer
participant GoParser as Go Parser
participant HTTPParse as HTTP Parse
eBPF->>eBPF: Compute buf_copy_len<br/>(min of total_size, MAX_DATAEVENT_BUFFER)
eBPF->>eBPF: Store buf_copy_len<br/>into dataevent.buf_len
eBPF->>eBPF: Copy data via bpf_probe_read_user<br/>using buf_copy_len
eBPF->>RingBuf: Send httpevent with buf_len populated
RingBuf->>GoParser: Deliver event with buf_len field
GoParser->>GoParser: Call GetBufLen() to read buf_len value
GoParser->>GoParser: Call GetValidBuf(event)<br/>returns buf[:buf_len]
GoParser->>HTTPParse: Pass validated buffer portion
HTTPParse->>HTTPParse: Parse only valid bytes<br/>(no garbage/trailing data)
HTTPParse-->>GoParser: Return parsed HTTP request/response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
YakirOren
changed the title
YakirOren
force-pushed
the
fix/http-buf-len
branch
3 times, most recently
from
59a7c77 to
9ca5fa2
Compare
Signed-off-by: Yakir Oren <yakiroren@gmail.com>
YakirOren
force-pushed
the
fix/http-buf-len
branch
from
9ca5fa2 to
beda638
Compare
YakirOren
changed the title
matthyx
reviewed
Mar 5, 2026
matthyx
previously approved these changes
Mar 5, 2026
Signed-off-by: Yakir Oren <yakiroren@gmail.com>
YakirOren
force-pushed
the
fix/http-buf-len
branch
from
77e31ac to
45f4d2b
Compare
Signed-off-by: Yakir Oren <yakiroren@gmail.com>
matthyx
approved these changes
Mar 5, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
pkg/containerwatcher/v2/tracers/httpparse_test.go (1)
255-268: Tighten the firstGetValidBufsubtests to explicitly exercisebuf_len.At Line 255 and Line 262, these cases use
utils.StructEvent, soGetBufLen()defaults tolen(Buf). Consider usingmockHttpRawEventwith a full ring-style buffer plus explicitbufLento make the assertions directly validate truncation behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/containerwatcher/v2/tracers/httpparse_test.go` around lines 255 - 268, The two subtests for GetValidBuf currently use utils.StructEvent (which makes GetBufLen() default to len(Buf)); replace those with mockHttpRawEvent instances that supply a full ring-style buffer and an explicit bufLen so the test actually exercises truncation logic: for the "uses buf_len when set" case create a mockHttpRawEvent with Buf containing httpData plus extra garbage and BufLen set to len(httpData) and assert GetValidBuf returns only httpData; for the "buf_len truncates garbage from ring buffer" case continue to use makeBPFBuffer but wrap it into mockHttpRawEvent with BufLen explicitly set to len(httpData) to validate garbage is truncated. Ensure you reference GetValidBuf and mockHttpRawEvent in the updated subtests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/containerwatcher/v2/tracers/httpparse.go`:
- Around line 180-183: The code currently returns the whole buffer when
event.GetBufLen() is zero or out-of-range, which can reintroduce stale data;
update the logic in the httpparse.go slice-return site to return an empty slice
when n := event.GetBufLen() is <= 0 or int(n) > len(buf) so only a valid
positive length yields buf[:n]; specifically modify the branch that uses
event.GetBufLen() and buf to fail-closed (return empty slice) instead of
returning buf.
In `@pkg/ebpf/gadgets/http/program.bpf.c`:
- Around line 231-233: The code currently assigns dataevent->buf_len before
checking whether bpf_probe_read_user succeeded, allowing stale/invalid data to
be published; change the flow in the functions using dataevent, buf_copy_len,
and bpf_probe_read_user so that you first call bpf_probe_read_user and check its
return value, and only on success set dataevent->buf_len and keep/publish the
event — on failure either discard the reserved event (return early) or set
buf_len to 0 and avoid publishing; apply the same change for the second
occurrence that uses packet->buf and buf_copy_len to ensure buffer length and
data remain consistent with the copy result.
In `@pkg/utils/struct_event.go`:
- Around line 119-121: GetBufLen currently returns uint16(len(e.Buf)) which
loses the original kernel buf_len marker for fixed-capacity Buf; change
StructEvent to carry an explicit buf_len field (e.g., BufLen uint16) and
implement GetBufLen to return that field instead of len(e.Buf). Ensure the code
that constructs/parses StructEvent sets BufLen from the kernel/event metadata
and update any consumers (notably GetValidBuf) to rely on the explicit BufLen
when deciding truncation/valid length.
---
Nitpick comments:
In `@pkg/containerwatcher/v2/tracers/httpparse_test.go`:
- Around line 255-268: The two subtests for GetValidBuf currently use
utils.StructEvent (which makes GetBufLen() default to len(Buf)); replace those
with mockHttpRawEvent instances that supply a full ring-style buffer and an
explicit bufLen so the test actually exercises truncation logic: for the "uses
buf_len when set" case create a mockHttpRawEvent with Buf containing httpData
plus extra garbage and BufLen set to len(httpData) and assert GetValidBuf
returns only httpData; for the "buf_len truncates garbage from ring buffer" case
continue to use makeBPFBuffer but wrap it into mockHttpRawEvent with BufLen
explicitly set to len(httpData) to validate garbage is truncated. Ensure you
reference GetValidBuf and mockHttpRawEvent in the updated subtests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b55cfacf-b198-4d4b-9dbe-16a95586c67c
📒 Files selected for processing (9)
pkg/containerwatcher/v2/tracers/http.gopkg/containerwatcher/v2/tracers/http_test.gopkg/containerwatcher/v2/tracers/httpparse.gopkg/containerwatcher/v2/tracers/httpparse_test.gopkg/ebpf/gadgets/http/program.bpf.cpkg/ebpf/gadgets/http/program.hpkg/utils/datasource_event.gopkg/utils/events.gopkg/utils/struct_event.go
Comment on lines
+180
to
+183
| if n := event.GetBufLen(); n > 0 && int(n) <= len(buf) { | ||
| return buf[:n] | ||
| } | ||
| return buf |
There was a problem hiding this comment.
Fail closed when buf_len is zero or invalid.
On Line 180 and Line 183, buf_len == 0 (or out-of-range) falls back to the full buffer, which can reintroduce stale-data parsing. For this bug class, safer behavior is returning an empty slice when length metadata is invalid/unset.
🔧 Proposed fix
func GetValidBuf(event utils.HttpRawEvent) []byte {
buf := event.GetBuf()
- if n := event.GetBufLen(); n > 0 && int(n) <= len(buf) {
- return buf[:n]
- }
- return buf
+ n := int(event.GetBufLen())
+ if n <= 0 || n > len(buf) {
+ return buf[:0]
+ }
+ return buf[:n]
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/containerwatcher/v2/tracers/httpparse.go` around lines 180 - 183, The
code currently returns the whole buffer when event.GetBufLen() is zero or
out-of-range, which can reintroduce stale data; update the logic in the
httpparse.go slice-return site to return an empty slice when n :=
event.GetBufLen() is <= 0 or int(n) > len(buf) so only a valid positive length
yields buf[:n]; specifically modify the branch that uses event.GetBufLen() and
buf to fail-closed (return empty slice) instead of returning buf.
Comment on lines
+231
to
+233
| __u64 buf_copy_len = min_size(total_size, MAX_DATAEVENT_BUFFER); | ||
| dataevent->buf_len = (__u16)buf_copy_len; | ||
| bpf_probe_read_user(&dataevent->buf, buf_copy_len, (void *)packet->buf); |
There was a problem hiding this comment.
Validate payload copy success before publishing buf_len.
At Line 233 and Line 323, buf_len is set before verifying bpf_probe_read_user succeeded. If the copy fails, the event can still advertise valid bytes and reintroduce parse corruption/stale-data behavior.
Please gate publication on successful copy (or discard the reserved event on failure) so buf_len and buf stay consistent.
Also applies to: 322-323
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/ebpf/gadgets/http/program.bpf.c` around lines 231 - 233, The code
currently assigns dataevent->buf_len before checking whether bpf_probe_read_user
succeeded, allowing stale/invalid data to be published; change the flow in the
functions using dataevent, buf_copy_len, and bpf_probe_read_user so that you
first call bpf_probe_read_user and check its return value, and only on success
set dataevent->buf_len and keep/publish the event — on failure either discard
the reserved event (return early) or set buf_len to 0 and avoid publishing;
apply the same change for the second occurrence that uses packet->buf and
buf_copy_len to ensure buffer length and data remain consistent with the copy
result.
Comment on lines
+119
to
+121
| func (e *StructEvent) GetBufLen() uint16 { | ||
| return uint16(len(e.Buf)) | ||
| } |
There was a problem hiding this comment.
StructEvent.GetBufLen() loses explicit kernel buf_len semantics.
GetBufLen() currently derives from len(Buf), which does not preserve the actual buf_len marker when Buf is fixed-capacity. This can skip truncation in GetValidBuf for StructEvent paths.
Suggested fix
type StructEvent struct {
Addresses []string `json:"addresses,omitempty" yaml:"addresses,omitempty"`
Args []string `json:"args,omitempty" yaml:"args,omitempty"`
AttrSize uint32 `json:"attrSize,omitempty" yaml:"attrSize,omitempty"`
Buf []byte `json:"buf,omitempty" yaml:"buf,omitempty"`
+ BufLen uint16 `json:"bufLen,omitempty" yaml:"bufLen,omitempty"`
CapName string `json:"capName,omitempty" yaml:"capName,omitempty"`
...
}
func (e *StructEvent) GetBufLen() uint16 {
- return uint16(len(e.Buf))
+ if e.BufLen > 0 {
+ return e.BufLen
+ }
+ return uint16(len(e.Buf))
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/utils/struct_event.go` around lines 119 - 121, GetBufLen currently
returns uint16(len(e.Buf)) which loses the original kernel buf_len marker for
fixed-capacity Buf; change StructEvent to carry an explicit buf_len field (e.g.,
BufLen uint16) and implement GetBufLen to return that field instead of
len(e.Buf). Ensure the code that constructs/parses StructEvent sets BufLen from
the kernel/event metadata and update any consumers (notably GetValidBuf) to rely
on the explicit BufLen when deciding truncation/valid length.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Tests