Skip to content

fix: mark wire-format header structs packed, enable UBSan alignment checking (#1104) - #1112

Merged
fklassen merged 1 commit into
4.6.1-beta1from
fix-alignment-ubsan
Jul 29, 2026
Merged

fix: mark wire-format header structs packed, enable UBSan alignment checking (#1104)#1112
fklassen merged 1 commit into
4.6.1-beta1from
fix-alignment-ubsan

Conversation

@fklassen

Copy link
Copy Markdown
Member

Fixes #1104.

Misaligned struct access was systemic, not the two isolated defects #1100 fixed: parsing a packet here means casting a buffer offset to a header struct, and an Ethernet payload starts 14 bytes in, so any 4-byte field in the header right after it is misaligned by construction - 15 sites across get.c, flows.c and en10mb.c, all the same shape (cast, then dereference).

The fix

Marked the five structs those 15 sites actually cast onto __attribute__((packed)): tcpr_ipv4_hdr, tcpr_ipv6_hdr, tcpr_in6_addr, tcpr_tcp_hdr, tcpr_icmpv4_hdr (src/tcpr.h).

None of them had compiler-inserted padding to begin with - every field already falls on a self-consistent offset - so this is a no-op for sizeof() and wire layout. Verified identical sizeof() before/after for all five:

ipv4_hdr: 20   ipv6_hdr: 40   in6_addr: 16   tcp_hdr: 20   icmpv4_hdr: 28

before and after, unchanged. packed here only removes the compiler's assumption that the struct's start address is naturally aligned - which it never was, since it's overlaid on packet buffers at whatever offset the preceding headers add up to.

Why not the other ~55 wire structs in tcpr.h

Deliberately left alone. They're not implicated by any UBSan finding the corpus has actually produced (BGP4, OSPF, DHCP, SEBEK, and the rest aren't exercised the same way), and packing them without that evidence would trade real, exercised coverage for speculative coverage of paths nothing here tests - same reasoning #1104 itself gave for not folding this into #1100.

CI

-fno-sanitize=alignment dropped from the asan job - alignment checking now runs alongside everything else UBSan already checked (integer overflow, shifts, bad enums, null dereference).

Verification

  • Alignment enabled with -fno-sanitize-recover (abort on first violation, not just log it): 78/78, 0 sanitizer reports.
  • Same result under the plain non-sanitizer autotools build (matching the tests CI job) and under CMake.
  • sudo make tcpreplay (full replay group): 19/19.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NBmWiWg46r8BLbdwozKo6v

…hecking (#1104)

Misaligned struct access was systemic, not the two isolated defects #1100
fixed: parsing a packet here means casting a buffer offset to a header
struct, and an Ethernet payload starts 14 bytes in, so any 4-byte field in
the header right after it is misaligned by construction. 15 sites across
get.c, flows.c and en10mb.c, all the same shape - cast, then dereference.

Marked the five structs those sites actually cast onto __attribute__
((packed)): tcpr_ipv4_hdr, tcpr_ipv6_hdr, tcpr_in6_addr, tcpr_tcp_hdr,
tcpr_icmpv4_hdr. None of them had compiler-inserted padding to begin with -
every field already falls on a self-consistent offset - so this is a no-op
for sizeof() and wire layout (verified: identical sizeof() before and after
for all five), and only removes the compiler's assumption that the struct's
start address is naturally aligned, which it never was. The other ~55 wire
structs in tcpr.h were deliberately left alone: they're not implicated by
any UBSan finding the corpus has actually produced, and packing them without
that evidence would be exchanging real, exercised coverage for speculative
coverage of paths (BGP4, OSPF, DHCP, SEBEK, ...) nothing here tests.

With that, alignment checking is enabled in the asan CI job alongside
everything else UBSan already checked - integer overflow, shifts, bad
enums, null dereference. Verified with alignment enabled and
-fno-sanitize-recover (abort on first violation, not just log it): 78/78,
0 sanitizer reports. Also verified under CMake and the plain non-sanitizer
autotools build (matching the "tests" CI job), both 78/78.

Fixes #1104
@fklassen
fklassen merged commit c855f3c into 4.6.1-beta1 Jul 29, 2026
6 checks passed
@fklassen
fklassen deleted the fix-alignment-ubsan branch July 29, 2026 12:46
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.

2 participants