All of lore.kernel.org
 help / color / mirror / Atom feed
* [iptables PATCH] ruleparse: arp: Fix for all-zero mask on Big Endian
@ 2026-01-28 21:44 Phil Sutter
  2026-01-29 14:58 ` Florian Westphal
  0 siblings, 1 reply; 4+ messages in thread
From: Phil Sutter @ 2026-01-28 21:44 UTC (permalink / raw)
  To: netfilter-devel

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.

Fixes: ab2d5f8c7bbee ("nft-arp: add missing mask support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-ruleparse-arp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/iptables/nft-ruleparse-arp.c b/iptables/nft-ruleparse-arp.c
index b0671cb0dfe8f..0648b2748931f 100644
--- a/iptables/nft-ruleparse-arp.c
+++ b/iptables/nft-ruleparse-arp.c
@@ -90,7 +90,7 @@ static void nft_arp_parse_payload(struct nft_xt_ctx *ctx,
 		if (inv)
 			fw->arp.invflags |= IPT_INV_ARPHRD;
 		if (reg->bitwise.set)
-			fw->arp.arhrd_mask = reg->bitwise.mask[0];
+			fw->arp.arhrd_mask = ((uint16_t *)reg->bitwise.mask)[0];
 		break;
 	case offsetof(struct arphdr, ar_pro):
 		get_cmp_data(e, &ar_pro, sizeof(ar_pro), &inv);
@@ -99,7 +99,7 @@ static void nft_arp_parse_payload(struct nft_xt_ctx *ctx,
 		if (inv)
 			fw->arp.invflags |= IPT_INV_PROTO;
 		if (reg->bitwise.set)
-			fw->arp.arpro_mask = reg->bitwise.mask[0];
+			fw->arp.arpro_mask = ((uint16_t *)reg->bitwise.mask)[0];
 		break;
 	case offsetof(struct arphdr, ar_op):
 		get_cmp_data(e, &ar_op, sizeof(ar_op), &inv);
@@ -108,7 +108,7 @@ static void nft_arp_parse_payload(struct nft_xt_ctx *ctx,
 		if (inv)
 			fw->arp.invflags |= IPT_INV_ARPOP;
 		if (reg->bitwise.set)
-			fw->arp.arpop_mask = reg->bitwise.mask[0];
+			fw->arp.arpop_mask = ((uint16_t *)reg->bitwise.mask)[0];
 		break;
 	case offsetof(struct arphdr, ar_hln):
 		get_cmp_data(e, &ar_hln, sizeof(ar_hln), &inv);
@@ -117,7 +117,7 @@ static void nft_arp_parse_payload(struct nft_xt_ctx *ctx,
 		if (inv)
 			fw->arp.invflags |= IPT_INV_ARPHLN;
 		if (reg->bitwise.set)
-			fw->arp.arhln_mask = reg->bitwise.mask[0];
+			fw->arp.arhln_mask = ((uint8_t *)reg->bitwise.mask)[0];
 		break;
 	case offsetof(struct arphdr, ar_pln):
 		get_cmp_data(e, &ar_pln, sizeof(ar_pln), &inv);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [iptables PATCH] ruleparse: arp: Fix for all-zero mask on Big Endian
  2026-01-28 21:44 [iptables PATCH] ruleparse: arp: Fix for all-zero mask on Big Endian Phil Sutter
@ 2026-01-29 14:58 ` Florian Westphal
  2026-01-29 15:24   ` Phil Sutter
  0 siblings, 1 reply; 4+ messages in thread
From: Florian Westphal @ 2026-01-29 14:58 UTC (permalink / raw)
  To: Phil Sutter; +Cc: netfilter-devel

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];
      |                                                                             ^

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [iptables PATCH] ruleparse: arp: Fix for all-zero mask on Big Endian
  2026-01-29 14:58 ` Florian Westphal
@ 2026-01-29 15:24   ` Phil Sutter
  2026-01-29 16:24     ` Florian Westphal
  0 siblings, 1 reply; 4+ messages in thread
From: Phil Sutter @ 2026-01-29 15:24 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

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?

Cheers, Phil

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [iptables PATCH] ruleparse: arp: Fix for all-zero mask on Big Endian
  2026-01-29 15:24   ` Phil Sutter
@ 2026-01-29 16:24     ` Florian Westphal
  0 siblings, 0 replies; 4+ messages in thread
From: Florian Westphal @ 2026-01-29 16:24 UTC (permalink / raw)
  To: Phil Sutter, netfilter-devel

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-01-29 16:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-28 21:44 [iptables PATCH] ruleparse: arp: Fix for all-zero mask on Big Endian Phil Sutter
2026-01-29 14:58 ` Florian Westphal
2026-01-29 15:24   ` Phil Sutter
2026-01-29 16:24     ` Florian Westphal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.