[PATCH 6.12.y 0/2] nft_byteorder: fix the uninitialised-stack read without removing multi-register support

[PATCH 6.12.y 0/2] nft_byteorder: fix the uninitialised-stack read without removing multi-register support

[Abdifatah Suruur]

Pablo pointed out that the full upstream removal (bb061d3) cannot be
applied to stable trees: nftables versions before 1.1.0 can generate
len != size byteorder expressions, and the removal rejects them, so
existing rulesets may fail to load.  The removal is tagged
stable+noautosel for that reason.

This series restores the multi-register path in 6.12.y and fixes the
uninitialised-stack read with a targeted initialisation:

1/2 reverts the removal currently in 6.12.y.
2/2 zeroes the destination span marked initialised by the control
    plane, but only when source and destination registers differ, so
    in-place conversions (sreg == dreg) keep their value.

This does not reject any rule and keeps the old userspace-visible
behaviour.  It removes the dependency on CONFIG_INIT_STACK_ALL_ZERO
zeroing the register file.

Compile-tested by reverting the removal and applying 2/2 to the
pre-removal tree; runtime testing on 6.12.y is welcome.

Abdifatah Suruur (2):
  Revert "netfilter: nft_byteorder: remove multi-register support"
  netfilter: nft_byteorder: initialise the marked destination span

Pablo, this is the stable-specific alternative we discussed. If a single
folded patch is preferred, I can resend it that way.

[PATCH 6.12.y 1/2] Revert "netfilter: nft_byteorder: remove multi-register support"

[Abdifatah Suruur]

This reverts commit 3c5fb3a6c8118d15e60295e76fb5b879021eef43.

The upstream change removes multi-register byteorder support and is
tagged stable+noautosel because nftables versions before 1.1.0 can
generate length/size combinations that the removal rejects.  Backporting
it to stable can make existing rulesets fail to load.

Restore the multi-register path so stable keeps the old userspace-visible
behaviour, and fix the uninitialised-stack read with a targeted
initialisation in the next patch.

Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
---

diff --git a/net/netfilter/nft_byteorder.c b/net/netfilter/nft_byteorder.c
index 391aade36ef3c..74548b4c114b9 100644
--- a/net/netfilter/nft_byteorder.c
+++ b/net/netfilter/nft_byteorder.c
@@ -19,6 +19,7 @@ struct nft_byteorder {
 	u8			sreg;
 	u8			dreg;
 	enum nft_byteorder_ops	op:8;
+	u8			len;
 	u8			size;
 };
 
@@ -27,8 +28,13 @@ void nft_byteorder_eval(const struct nft_expr *expr,
 			const struct nft_pktinfo *pkt)
 {
 	const struct nft_byteorder *priv = nft_expr_priv(expr);
-	const u32 *src = &regs->data[priv->sreg];
+	u32 *src = &regs->data[priv->sreg];
 	u32 *dst = &regs->data[priv->dreg];
+	u16 *s16, *d16;
+	unsigned int i;
+
+	s16 = (void *)src;
+	d16 = (void *)dst;
 
 	switch (priv->size) {
 	case 8: {
@@ -37,14 +43,18 @@ void nft_byteorder_eval(const struct nft_expr *expr,
 
 		switch (priv->op) {
 		case NFT_BYTEORDER_NTOH:
-			src64 = nft_reg_load64(src);
-
-			nft_reg_store64(dst64, be64_to_cpu((__force __be64)src64));
+			for (i = 0; i < priv->len / 8; i++) {
+				src64 = nft_reg_load64(&src[i]);
+				nft_reg_store64(&dst64[i],
+						be64_to_cpu((__force __be64)src64));
+			}
 			break;
 		case NFT_BYTEORDER_HTON:
-			src64 = (__force __u64)cpu_to_be64(nft_reg_load64(src));
-
-			nft_reg_store64(dst64, src64);
+			for (i = 0; i < priv->len / 8; i++) {
+				src64 = (__force __u64)
+					cpu_to_be64(nft_reg_load64(&src[i]));
+				nft_reg_store64(&dst64[i], src64);
+			}
 			break;
 		}
 		break;
@@ -52,20 +62,24 @@ void nft_byteorder_eval(const struct nft_expr *expr,
 	case 4:
 		switch (priv->op) {
 		case NFT_BYTEORDER_NTOH:
-			*dst = ntohl((__force __be32)*src);
+			for (i = 0; i < priv->len / 4; i++)
+				dst[i] = ntohl((__force __be32)src[i]);
 			break;
 		case NFT_BYTEORDER_HTON:
-			*dst = (__force __u32)htonl(*src);
+			for (i = 0; i < priv->len / 4; i++)
+				dst[i] = (__force __u32)htonl(src[i]);
 			break;
 		}
 		break;
 	case 2:
 		switch (priv->op) {
 		case NFT_BYTEORDER_NTOH:
-			nft_reg_store16(dst, ntohs(nft_reg_load_be16(src)));
+			for (i = 0; i < priv->len / 2; i++)
+				d16[i] = ntohs((__force __be16)s16[i]);
 			break;
 		case NFT_BYTEORDER_HTON:
-			nft_reg_store_be16(dst, htons(nft_reg_load16(src)));
+			for (i = 0; i < priv->len / 2; i++)
+				d16[i] = (__force __u16)htons(s16[i]);
 			break;
 		}
 		break;
@@ -123,22 +137,20 @@ static int nft_byteorder_init(const struct nft_ctx *ctx,
 	if (err < 0)
 		return err;
 
-	/* no longer support multi-reg conversions */
-	if (len != size)
-		return -EOPNOTSUPP;
+	priv->len = len;
 
 	err = nft_parse_register_load(ctx, tb[NFTA_BYTEORDER_SREG], &priv->sreg,
-				      len);
+				      priv->len);
 	if (err < 0)
 		return err;
 
 	err = nft_parse_register_store(ctx, tb[NFTA_BYTEORDER_DREG],
 				       &priv->dreg, NULL, NFT_DATA_VALUE,
-				       len);
+				       priv->len);
 	if (err < 0)
 		return err;
 
-	if (nft_reg_overlap(priv->sreg, priv->dreg, len))
+	if (nft_reg_overlap(priv->sreg, priv->dreg, priv->len))
 		return -EINVAL;
 
 	return 0;
@@ -155,10 +167,9 @@ static int nft_byteorder_dump(struct sk_buff *skb,
 		goto nla_put_failure;
 	if (nla_put_be32(skb, NFTA_BYTEORDER_OP, htonl(priv->op)))
 		goto nla_put_failure;
-	if (nla_put_be32(skb, NFTA_BYTEORDER_SIZE, htonl(priv->size)))
+	if (nla_put_be32(skb, NFTA_BYTEORDER_LEN, htonl(priv->len)))
 		goto nla_put_failure;
-	/* compatibility for old userspace which permitted size != len */
-	if (nla_put_be32(skb, NFTA_BYTEORDER_LEN, htonl(priv->size)))
+	if (nla_put_be32(skb, NFTA_BYTEORDER_SIZE, htonl(priv->size)))
 		goto nla_put_failure;
 	return 0;
 
-- 
2.53.0

[PATCH 6.12.y 2/2] netfilter: nft_byteorder: initialise the marked destination span

[Abdifatah Suruur]

nft_byteorder_eval() writes only priv->len / priv->size whole elements,
while nft_parse_register_store() marks DIV_ROUND_UP(priv->len, 4) full
32-bit words as initialised.  For len/size combinations that do not fill
the last marked word -- for example size 2 with len 1, which writes no
element at all -- the control plane still accepts a later load of the
whole word from another expression.  That load reads stale bytes from
the struct nft_regs stack slot in nft_do_chain(), which is not
zero-initialised.

Zero the span that the control plane marks as initialised before the
conversion, but only when the source and destination registers differ.
nft_reg_overlap() allows an in-place conversion (sreg == dreg), and the
source is already initialised in that case; zeroing it would corrupt
the value.

This preserves multi-register support and does not reject any rule, so
old nftables versions that generate len != size continue to load.  It
only removes the dependency on some other initialisation of the
register file.  Builds with CONFIG_INIT_STACK_ALL_ZERO mask the leak,
but nft_byteorder must not rely on that.

Fixes: 14fb07130c7d ("netfilter: nf_tables: allow loads only when register is initialized")
Cc: stable@vger.kernel.org
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
---

diff --git a/net/netfilter/nft_byteorder.c b/net/netfilter/nft_byteorder.c
--- a/net/netfilter/nft_byteorder.c
+++ b/net/netfilter/nft_byteorder.c
@@ -33,6 +33,17 @@ void nft_byteorder_eval(const struct nft_expr *expr,
 	u16 *s16, *d16;
 	unsigned int i;
 
+	/*
+	 * nft_parse_register_store() marks DIV_ROUND_UP(priv->len, 4)
+	 * words as initialised, but this function only writes whole
+	 * len/size elements.  Zero the marked span so that a later load
+	 * cannot read uninitialised stack data.  Do not touch it when
+	 * source and destination are the same register: the source is
+	 * already initialised and zeroing it would corrupt the value.
+	 */
+	if (priv->sreg != priv->dreg)
+		memset(dst, 0, DIV_ROUND_UP(priv->len, 4) * sizeof(u32));
+
 	s16 = (void *)src;
 	d16 = (void *)dst;
 
-- 
2.53.0

Re: [PATCH 6.12.y 1/2] Revert "netfilter: nft_byteorder: remove multi-register support"

[Pablo Neira Ayuso]

Hi,

On Mon, Nov 09, 2026 at 03:22:54PM +0300, Abdifatah Suruur wrote:
> This reverts commit 3c5fb3a6c8118d15e60295e76fb5b879021eef43.
> 
> The upstream change removes multi-register byteorder support and is
> tagged stable+noautosel because nftables versions before 1.1.0 can
> generate length/size combinations that the removal rejects.  Backporting
> it to stable can make existing rulesets fail to load.

Yes, so why are you posting this as is?

Re: [PATCH 6.12.y 2/2] netfilter: nft_byteorder: initialise the marked destination span

[Pablo Neira Ayuso]

On Mon, Nov 09, 2026 at 03:22:59PM +0300, Abdifatah Suruur wrote:
> nft_byteorder_eval() writes only priv->len / priv->size whole elements,
> while nft_parse_register_store() marks DIV_ROUND_UP(priv->len, 4) full
> 32-bit words as initialised.  For len/size combinations that do not fill
> the last marked word -- for example size 2 with len 1, which writes no
> element at all -- the control plane still accepts a later load of the
> whole word from another expression.  That load reads stale bytes from
> the struct nft_regs stack slot in nft_do_chain(), which is not
> zero-initialised.
> 
> Zero the span that the control plane marks as initialised before the
> conversion, but only when the source and destination registers differ.
> nft_reg_overlap() allows an in-place conversion (sreg == dreg), and the
> source is already initialised in that case; zeroing it would corrupt
> the value.
> 
> This preserves multi-register support and does not reject any rule, so
> old nftables versions that generate len != size continue to load.  It
> only removes the dependency on some other initialisation of the
> register file.  Builds with CONFIG_INIT_STACK_ALL_ZERO mask the leak,
> but nft_byteorder must not rely on that.
> 
> Fixes: 14fb07130c7d ("netfilter: nf_tables: allow loads only when register is initialized")
> Cc: stable@vger.kernel.org
> Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
> ---
> 
> diff --git a/net/netfilter/nft_byteorder.c b/net/netfilter/nft_byteorder.c
> --- a/net/netfilter/nft_byteorder.c
> +++ b/net/netfilter/nft_byteorder.c
> @@ -33,6 +33,17 @@ void nft_byteorder_eval(const struct nft_expr *expr,
>  	u16 *s16, *d16;
>  	unsigned int i;
>  
> +	/*
> +	 * nft_parse_register_store() marks DIV_ROUND_UP(priv->len, 4)
> +	 * words as initialised, but this function only writes whole
> +	 * len/size elements.  Zero the marked span so that a later load
> +	 * cannot read uninitialised stack data.  Do not touch it when
> +	 * source and destination are the same register: the source is
> +	 * already initialised and zeroing it would corrupt the value.
> +	 */
> +	if (priv->sreg != priv->dreg)
> +		memset(dst, 0, DIV_ROUND_UP(priv->len, 4) * sizeof(u32));

Yes, this should be fine.

> +
>  	s16 = (void *)src;
>  	d16 = (void *)dst;
>  
> -- 
> 2.53.0

Re: [PATCH 6.12.y 1/2] Revert "netfilter: nft_byteorder: remove multi-register support"

[Pablo Neira Ayuso]

On Mon, Nov 09, 2026 at 01:25:08PM +0100, Pablo Neira Ayuso wrote:
> Hi,
> 
> On Mon, Nov 09, 2026 at 03:22:54PM +0300, Abdifatah Suruur wrote:
> > This reverts commit 3c5fb3a6c8118d15e60295e76fb5b879021eef43.
> > 
> > The upstream change removes multi-register byteorder support and is
> > tagged stable+noautosel because nftables versions before 1.1.0 can
> > generate length/size combinations that the removal rejects.  Backporting
> > it to stable can make existing rulesets fail to load.
> 
> Yes, so why are you posting this as is?

Apologies, I got this all wrong.

I thought you wanted to pull in "netfilter: nft_byteorder: remove
 multi-register support" but this already slipped through -stable?

So your series look correct.

Sorry for the fuzz.

Re: [PATCH 6.12.y 1/2] Revert "netfilter: nft_byteorder: remove multi-register support"

[Sasha Levin]

On Mon, Nov 09, 2026 at 01:34:44PM +0100, Pablo Neira Ayuso wrote:
>On Mon, Nov 09, 2026 at 01:25:08PM +0100, Pablo Neira Ayuso wrote:
>> Hi,
>>
>> On Mon, Nov 09, 2026 at 03:22:54PM +0300, Abdifatah Suruur wrote:
>> > This reverts commit 3c5fb3a6c8118d15e60295e76fb5b879021eef43.
>> >
>> > The upstream change removes multi-register byteorder support and is
>> > tagged stable+noautosel because nftables versions before 1.1.0 can
>> > generate length/size combinations that the removal rejects.  Backporting
>> > it to stable can make existing rulesets fail to load.
>>
>> Yes, so why are you posting this as is?
>
>Apologies, I got this all wrong.
>
>I thought you wanted to pull in "netfilter: nft_byteorder: remove
> multi-register support" but this already slipped through -stable?

We were asked[1] to backport it.


[1] https://lore.kernel.org/all/ataIP3xm8DDtO82v@strlen.de/

-- 
Thanks,
Sasha

Re: [PATCH 6.12.y 1/2] Revert "netfilter: nft_byteorder: remove multi-register support"

[Abdifatah Suruur]

Hi Pablo, Sasha,

Thanks for the review and the correction. The series is unchanged: the
revert restores multi-register support and 2/2 closes the uninitialised
read with the in-place guard. Sasha, understood that the removal backport
was requested; this series keeps the security fix while restoring the old
userspace behaviour.

Please queue the 6.12.y series when you get a chance.

Thanks,
Abdifatah