Describe the bug
Misaligned struct access is systemic in the packet parsers, not the two isolated defects #1100 was filed for. With -fsanitize=alignment and recovery on, the existing test suite reports 15 distinct sites across three files:
en10mb.c:789 en10mb.c:872 en10mb.c:881
flows.c:221 flows.c:228 flows.c:229 flows.c:230 flows.c:231
flows.c:292 flows.c:293 flows.c:308 flows.c:309
get.c:639 get.c:681 get.c:737
They are all the same shape:
ip_hdr = (ipv4_hdr_t *)(packet + l2len);
...
if (ip_hdr->ip_v != 4) /* misaligned member access */
This is close to inherent to how the codebase parses packets. An Ethernet header is 14 bytes, so the IP header that follows starts at offset 14 — 2 mod 4 — and every 4-byte field in it is misaligned by construction. Casting a buffer offset to a header struct and dereferencing it is undefined behaviour regardless of what the hardware does about it.
Why it matters
x86 and modern ARM fix these up transparently, which is why nothing has ever been noticed. docs/INSTALL lists Solaris and the BSDs among supported platforms; on SPARC a misaligned load traps, and while the kernel emulates it, that is a large hidden performance cost at line rate — and on some configurations it is a fault, not a fixup.
It also means the alignment check has to stay off in CI, so nothing catches a new misaligned access that lands somewhere genuinely dangerous.
Suggested fix
Read multi-byte fields through memcpy into a local, as parse_mpls() now does after #1100:
uint32_t entry;
memcpy(&entry, cursor, sizeof(entry));
entry = ntohl(entry);
Modern compilers turn that back into a single load on architectures that permit one, so it costs nothing where it does not matter. Doing it across three files and the DLT plugins is a real refactor with real regression risk on the packet-parsing hot path, which is why it is filed separately rather than bolted onto #1100.
An alternative worth weighing: mark the header structs __attribute__((packed)), which tells the compiler the alignment is not guaranteed and makes the accesses defined. That is a smaller diff but changes code generation everywhere those structs are used.
Once fixed
Drop -fno-sanitize=alignment from the asan job in .github/workflows/github-actions-ci.yml — it is one flag, and the comment there points back here.
Additional context
Found while fixing #1100. Everything else UBSan checks — integer overflow, shifts, bad enums, null dereference — is clean across the whole suite, so alignment is the only thing keeping the check disabled.
Describe the bug
Misaligned struct access is systemic in the packet parsers, not the two isolated defects #1100 was filed for. With
-fsanitize=alignmentand recovery on, the existing test suite reports 15 distinct sites across three files:They are all the same shape:
This is close to inherent to how the codebase parses packets. An Ethernet header is 14 bytes, so the IP header that follows starts at offset 14 — 2 mod 4 — and every 4-byte field in it is misaligned by construction. Casting a buffer offset to a header struct and dereferencing it is undefined behaviour regardless of what the hardware does about it.
Why it matters
x86 and modern ARM fix these up transparently, which is why nothing has ever been noticed.
docs/INSTALLlists Solaris and the BSDs among supported platforms; on SPARC a misaligned load traps, and while the kernel emulates it, that is a large hidden performance cost at line rate — and on some configurations it is a fault, not a fixup.It also means the alignment check has to stay off in CI, so nothing catches a new misaligned access that lands somewhere genuinely dangerous.
Suggested fix
Read multi-byte fields through
memcpyinto a local, asparse_mpls()now does after #1100:Modern compilers turn that back into a single load on architectures that permit one, so it costs nothing where it does not matter. Doing it across three files and the DLT plugins is a real refactor with real regression risk on the packet-parsing hot path, which is why it is filed separately rather than bolted onto #1100.
An alternative worth weighing: mark the header structs
__attribute__((packed)), which tells the compiler the alignment is not guaranteed and makes the accesses defined. That is a smaller diff but changes code generation everywhere those structs are used.Once fixed
Drop
-fno-sanitize=alignmentfrom theasanjob in.github/workflows/github-actions-ci.yml— it is one flag, and the comment there points back here.Additional context
Found while fixing #1100. Everything else UBSan checks — integer overflow, shifts, bad enums, null dereference — is clean across the whole suite, so alignment is the only thing keeping the check disabled.