On 2026/02/12 23:09, Peter Maydell wrote:
> The packet header structs eth_header, udp_header and tcp_header
> defined in eth.h are not actually guaranteed to be aligned --
> typically the network device models will work with headers in a
> byte-array packet data buffer. We attempt to deal with this in some
> places such as net_checksum_calculate() by using lduw_be_p() and so on
> to access the fields, but this is not sufficient to be correct,
> because even accessing a byte member within a misaligned struct is
> undefined behaviour.
>
> Back in 2024 in commit f8b94b4c520 ("net: mark struct ip_header as
> QEMU_PACKED") we fixed this for ip_header, but if you run a
> check-functional on a sanitizers-enabled build you will run
> into errors like this in the sifive_u_mmc functional test:
>
> ../../net/checksum.c:78:47: runtime error: member access within misaligned addre
> ss 0x561f52f35011 for type 'struct eth_header', which requires 2 byte alignment
> 0x561f52f35011: note: pointer points here
> 00 00 00 00 33 33 00 00 00 16 52 54 00 12 34 56 86 dd 60 00 00 00 00 24
> 00 01 00 00 00 00 00 00
> ^
> #0 0x561f20608459 in net_checksum_calculate /home/pm215/qemu/build/clang/../../net/checksum.c:78:47
> #1 0x561f20117bfa in gem_transmit /home/pm215/qemu/build/clang/../../hw/net/cadence_gem.c:1386:21
> #2 0x561f20115c61 in gem_write /home/pm215/qemu/build/clang/../../hw/net/cadence_gem.c:1650:13
>
> As with the previous ip_header fix, before we can mark the structs as
> QEMU_PACKED we need to adjust some code which takes the address of a
> packed-struct field, because gcc will refuse to compile that. The
> rocker device does this with eth_header::h_proto.
>
> I chose to keep the three "mark this struct as QEMU_PACKED" changes as
> separate patches just in case they cause issues I didn't catch with my
> compilation tests; that way it will be easier to diagnose/revert.
>
> The problem with eth_header came up at the beginning of last year
> in relation to the npcm7xx_emc device; none of this is particularly
> specific to any one network device, it's just that we happen to only
> be exercising the network in a way that triggers it with the sifive
> test case.
> https://lore.kernel.org/qemu-devel/CAFEAcA-gyMTz-KpmamyXcKX9QOL=yYHDMPRF2Xji_uJbG02WpA@mail.gmail.com/
>
> It would not surprise me to find that some other structs in this
> header also ought to be marked as packed, but I prefer only to
> tackle them when we have a sanitizer repro case so we can
> confirm the fixes.
>
> thanks
> -- PMM
>
> Peter Maydell (5):
> hw/net/rocker: Don't keep pointer to h_proto as uint16_t* in
> OfDpaFlowPktFields
> hw/net/rocker: Don't assume h_proto is aligned in eth_strip_vlan_ex()
> net: mark struct eth_header as QEMU_PACKED
> net: mark struct udp_header as QEMU_PACKED
> net: mark struct tcp_header as QEMU_PACKED
>
> hw/net/rocker/rocker_of_dpa.c | 19 ++++++++++++-------
> include/net/eth.h | 6 +++---
> net/eth.c | 6 +++---
> 3 files changed, 18 insertions(+), 13 deletions(-)
>
For the whole series:
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>