Skip to content

fix(ebpf): zero-init container and pmc events for defense-in-depth#635

Open
yairfalse wants to merge 1 commit into
mainfrom
fix/ebpf-defensive-zero-init
Open

fix(ebpf): zero-init container and pmc events for defense-in-depth#635
yairfalse wants to merge 1 commit into
mainfrom
fix/ebpf-defensive-zero-init

Conversation

@yairfalse
Copy link
Copy Markdown
Collaborator

Summary

  • Zero container OOM events immediately after successful ringbuf reservation.
  • Zero container exit events immediately after successful ringbuf reservation.
  • Zero PMC sample events immediately after successful ringbuf reservation.

Why

These event structs are currently packed, but zeroing reserved ring-buffer events matches the other observers and prevents future padding additions from leaking kernel stack bytes.

Test plan

  • cargo check --workspace passes
  • eBPF programs still compile with: clang -O2 -g -target bpf -D__TARGET_ARCH_x86 -I ebpf/headers -c ebpf/container_monitor.c -o /tmp/container_monitor.o and clang -O2 -g -target bpf -D__TARGET_ARCH_x86 -I ebpf/headers -c ebpf/node_pmc_monitor.c -o /tmp/node_pmc_monitor.o
  • each container and PMC ringbuf reservation path zeroes the event before filling fields

Copy link
Copy Markdown
Collaborator Author

@yairfalse yairfalse left a comment

Choose a reason for hiding this comment

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

LGTM (review-only — self-authored, can't formally approve).

Three single-line __builtin_memset insertions at exactly the right places — immediately after each successful bpf_ringbuf_reserve in handle_oom, handle_exit, and sample_pmc. Matches the canonical pattern from storage_monitor.c:174 and network_monitor.c:214.

Today these structs are __attribute__((packed)) so there's no padding to leak, but this closes the door on a class of info-leak bugs that would otherwise appear silently the first time someone adds a non-aligned field.

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