mbox series

[v2,0/2] net: Make ip_header struct QEMU_PACKED

Message ID 20241114141619.806652-1-peter.maydell@linaro.org
Headers show
Series net: Make ip_header struct QEMU_PACKED | expand

Message

Peter Maydell Nov. 14, 2024, 2:16 p.m. UTC 
This patchset aims to fix some clang undefined-behavior
sanitizer warnings that you see if you run the arm
functional-tests:

         Stopping network: ../../net/checksum.c:106:9: runtime error: member access within misaligned address 0x556aad9b502e for type 'struct ip_header', which requires 4 byte alignment
        0x556aad9b502e: note: pointer points here
         34 56 08 00 45 00  01 48 a5 09 40 00 40 11  7c 8b 0a 00 02 0f 0a 00  02 02 00 44 00 43 01 34  19 56
                     ^
        SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../net/checksum.c:106:9 in
        ../../net/checksum.c:106:9: runtime error: load of misaligned address 0x556aad9b502e for type 'uint8_t' (aka 'unsigned char'), which requires 4 byte alignment
        0x556aad9b502e: note: pointer points here
         34 56 08 00 45 00  01 48 a5 09 40 00 40 11  7c 8b 0a 00 02 0f 0a 00  02 02 00 44 00 43 01 34  19 56
                     ^

These result from the net_checksum_calculate() function
creating a 'struct ip_header *' from an unaligned address.
We try to handle the non-alignment by using functions like
lduw_be_p(), but this is insufficient because even accessing
a byte member within an unaligned struct is UB.

This patchset fixes this by marking 'struct ip_header'
as QEMU_PACKED, which tells the compiler that it might
be at an unaligned address.

For that to work, we need to first fix the places in virtio-net.c
which take the address of a field within an ip_header:
the compiler will warn/error if we try to do that for a
field in a struct which is marked QEMU_PACKED.

Probably other structs in eth.h should be marked packed
too, but I am here only addressing the case that produces
warnings that I have seen.

v1->v2 changes:
 * patch 1 now uses the correct lduw_be_p/stw_be_p

thanks
-- PMM

Peter Maydell (2):
  hw/net/virtio-net.c: Don't assume IP length field is aligned
  net: mark struct ip_header as QEMU_PACKED

 include/hw/virtio/virtio-net.h |  2 +-
 include/net/eth.h              |  2 +-
 hw/net/virtio-net.c            | 23 +++++++++++++++++++----
 3 files changed, 21 insertions(+), 6 deletions(-)

Comments

Philippe Mathieu-Daudé Nov. 18, 2024, 12:40 p.m. UTC | #1 
On 14/11/24 14:16, Peter Maydell wrote:

> Peter Maydell (2):
>    hw/net/virtio-net.c: Don't assume IP length field is aligned
>    net: mark struct ip_header as QEMU_PACKED

Series queued to hw-misc, thanks.