Re: [PATCH] hw/net: npcm7xx_emc: fix alignment to eth_hdr

[Patrick Venture]

On Thu, Feb 27, 2025 at 7:52 AM Peter Maydell <peter.mayd...@linaro.org>
wrote:

> On Thu, 27 Feb 2025 at 15:40, Patrick Venture <vent...@google.com> wrote:
> >
> > 'const struct eth_header', which requires 2 byte alignment
> >
> > Signed-off-by: Patrick Venture <vent...@google.com>
> > ---
> >  hw/net/npcm7xx_emc.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/net/npcm7xx_emc.c b/hw/net/npcm7xx_emc.c
> > index e06f652629..11ed4a9e6a 100644
> > --- a/hw/net/npcm7xx_emc.c
> > +++ b/hw/net/npcm7xx_emc.c
> > @@ -424,7 +424,12 @@ static bool emc_can_receive(NetClientState *nc)
> >  static bool emc_receive_filter1(NPCM7xxEMCState *emc, const uint8_t
> *buf,
> >                                  size_t len, const char **fail_reason)
> >  {
> > -    eth_pkt_types_e pkt_type =
> get_eth_packet_type(PKT_GET_ETH_HDR(buf));
> > +    struct eth_header eth_hdr = {};
> > +    eth_pkt_types_e pkt_type;
> > +
> > +    memcpy(&eth_hdr, PKT_GET_ETH_HDR(buf),
> > +           (sizeof(eth_hdr) > len) ? len : sizeof(eth_hdr));
> > +    pkt_type = get_eth_packet_type(&eth_hdr);
>
> Maybe better to mark struct eth_header as QEMU_PACKED?
> Compare commit f8b94b4c5201 ("net: mark struct ip_header as
> QEMU_PACKED"). The handling of these header structs in eth.h
> is in general pretty suspect IMHO. We do the same
> "get_eth_packet_type(PKT_GET_ETH_HDR(buf))" in other devices,
> so this isn't just this device's bug.
>
> thanks
>

Roger that. We saw this in the two NICs we happened to be testing that day,
and yeah, I grepped and just figured that those other NICs were doing
something with their buffer allocations that we didn't. I'll give
QEMU_PACKED  whirl.


> -- PMM
>