Re: [iptables PATCH] ruleparse: arp: Fix for all-zero mask on Big Endian

[Florian Westphal <fw@strlen.de>]

Phil Sutter <phil@nwl.cc> wrote:
> On Thu, Jan 29, 2026 at 03:58:33PM +0100, Florian Westphal wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > > With 16bit mask values, the first two bytes of bitwise.mask in struct
> > > nft_xt_ctx_reg are significant. Reading the first 32bit-sized field
> > > works only on Little Endian, on Big Endian the mask appears in the upper
> > > two bytes which are discarded when assigning to a 16bit variable.
> > 
> > nft-ruleparse-arp.c: In function 'nft_arp_parse_payload':
> > nft-ruleparse-arp.c:93:77: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
> >    93 |                         fw->arp.arhrd_mask = ((uint16_t *)reg->bitwise.mask)[0];
> >       |                                                                             ^
> > nft-ruleparse-arp.c:102:77: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
> >   102 |                         fw->arp.arpro_mask = ((uint16_t *)reg->bitwise.mask)[0];
> >       |                                                                             ^
> > nft-ruleparse-arp.c:111:77: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
> >   111 |                         fw->arp.arpop_mask = ((uint16_t *)reg->bitwise.mask)[0];
> >       |                                                                             ^
> 
> Oops! I didn't notice this because my build script passed -O0 in CFLAGS,
> probably from code coverage analysis. So back to the previous approach
> involving a union which I had deemed too much for just those four cases:
> 
> diff --git a/iptables/nft-ruleparse.h b/iptables/nft-ruleparse.h
> index 0377e4ae17a6e..dc755b361ec72 100644
> --- a/iptables/nft-ruleparse.h
> +++ b/iptables/nft-ruleparse.h
> @@ -36,7 +36,11 @@ struct nft_xt_ctx_reg {
>         };
>  
>         struct {
> -               uint32_t mask[4];
> +               union {
> +                       uint32_t mask[4];
> +                       uint16_t mask16[];
> +                       uint8_t  mask8[];
> +               };
>                 uint32_t xor[4];
>                 bool set;
>         } bitwise;
> 
> Or simply use mempcy() instead of the assignment?

Or add:

static uint16_t get_u16p(uint32_t *t) { const uint16_t *p = (uint16_t *)t; return *p; }
static uint8_t get_u8p(uint32_t *t) { const uint8_t *p = (uint8_t *)t; return *p; }

... and use that.  But up to you.