All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 6.18.y 0/2] nft_byteorder: fix the uninitialised-stack read without removing multi-register support
@ 2026-11-09 12:23 Abdifatah Suruur
  2026-11-09 12:23 ` [PATCH 6.18.y 1/2] Revert "netfilter: nft_byteorder: remove multi-register support" Abdifatah Suruur
  2026-11-09 12:23 ` [PATCH 6.18.y 2/2] netfilter: nft_byteorder: initialise the marked destination span Abdifatah Suruur
  0 siblings, 2 replies; 6+ messages in thread
From: Abdifatah Suruur @ 2026-11-09 12:23 UTC (permalink / raw)
  To: stable; +Cc: Pablo Neira Ayuso, Florian Westphal, Greg KH, Sasha Levin

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.18.y and fixes the
uninitialised-stack read with a targeted initialisation:

1/2 reverts the removal currently in 6.18.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.18.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.

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

* [PATCH 6.18.y 1/2] Revert "netfilter: nft_byteorder: remove multi-register support"
  2026-11-09 12:23 [PATCH 6.18.y 0/2] nft_byteorder: fix the uninitialised-stack read without removing multi-register support Abdifatah Suruur
@ 2026-11-09 12:23 ` Abdifatah Suruur
  2026-11-09 12:25   ` Pablo Neira Ayuso
  2026-11-09 12:23 ` [PATCH 6.18.y 2/2] netfilter: nft_byteorder: initialise the marked destination span Abdifatah Suruur
  1 sibling, 1 reply; 6+ messages in thread
From: Abdifatah Suruur @ 2026-11-09 12:23 UTC (permalink / raw)
  To: stable; +Cc: Pablo Neira Ayuso, Florian Westphal, Greg KH, Sasha Levin

This reverts commit dc29c0478b8564feb9371e715b5a0016eb8903fd.

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

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

* [PATCH 6.18.y 2/2] netfilter: nft_byteorder: initialise the marked destination span
  2026-11-09 12:23 [PATCH 6.18.y 0/2] nft_byteorder: fix the uninitialised-stack read without removing multi-register support Abdifatah Suruur
  2026-11-09 12:23 ` [PATCH 6.18.y 1/2] Revert "netfilter: nft_byteorder: remove multi-register support" Abdifatah Suruur
@ 2026-11-09 12:23 ` Abdifatah Suruur
  1 sibling, 0 replies; 6+ messages in thread
From: Abdifatah Suruur @ 2026-11-09 12:23 UTC (permalink / raw)
  To: stable; +Cc: Pablo Neira Ayuso, Florian Westphal, Greg KH, Sasha Levin

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

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

* Re: [PATCH 6.18.y 1/2] Revert "netfilter: nft_byteorder: remove multi-register support"
  2026-11-09 12:23 ` [PATCH 6.18.y 1/2] Revert "netfilter: nft_byteorder: remove multi-register support" Abdifatah Suruur
@ 2026-11-09 12:25   ` Pablo Neira Ayuso
  2026-11-09 12:35     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2026-11-09 12:25 UTC (permalink / raw)
  To: Abdifatah Suruur; +Cc: stable, Florian Westphal, Greg KH, Sasha Levin

On Mon, Nov 09, 2026 at 03:23:46PM +0300, Abdifatah Suruur wrote:
> This reverts commit dc29c0478b8564feb9371e715b5a0016eb8903fd.
> 
> 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.

Same thing here, if this can break stuff, this cannot be taken as is.

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

* Re: [PATCH 6.18.y 1/2] Revert "netfilter: nft_byteorder: remove multi-register support"
  2026-11-09 12:25   ` Pablo Neira Ayuso
@ 2026-11-09 12:35     ` Pablo Neira Ayuso
  2026-11-09 13:18       ` Abdifatah Suruur
  0 siblings, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2026-11-09 12:35 UTC (permalink / raw)
  To: Abdifatah Suruur; +Cc: stable, Florian Westphal, Greg KH, Sasha Levin

On Mon, Nov 09, 2026 at 01:26:02PM +0100, Pablo Neira Ayuso wrote:
> On Mon, Nov 09, 2026 at 03:23:46PM +0300, Abdifatah Suruur wrote:
> > This reverts commit dc29c0478b8564feb9371e715b5a0016eb8903fd.
> > 
> > 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.
> 
> Same thing here, if this can break stuff, this cannot be taken as is.

Apologies, please, withdraw this comment I made.

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

* Re: [PATCH 6.18.y 1/2] Revert "netfilter: nft_byteorder: remove multi-register support"
  2026-11-09 12:35     ` Pablo Neira Ayuso
@ 2026-11-09 13:18       ` Abdifatah Suruur
  0 siblings, 0 replies; 6+ messages in thread
From: Abdifatah Suruur @ 2026-11-09 13:18 UTC (permalink / raw)
  To: stable; +Cc: Pablo Neira Ayuso, Florian Westphal, Greg KH, Sasha Levin

Hi Pablo,

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.

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

Thanks,
Abdifatah

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

end of thread, other threads:[~2026-11-09 13:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-09 12:23 [PATCH 6.18.y 0/2] nft_byteorder: fix the uninitialised-stack read without removing multi-register support Abdifatah Suruur
2026-11-09 12:23 ` [PATCH 6.18.y 1/2] Revert "netfilter: nft_byteorder: remove multi-register support" Abdifatah Suruur
2026-11-09 12:25   ` Pablo Neira Ayuso
2026-11-09 12:35     ` Pablo Neira Ayuso
2026-11-09 13:18       ` Abdifatah Suruur
2026-11-09 12:23 ` [PATCH 6.18.y 2/2] netfilter: nft_byteorder: initialise the marked destination span Abdifatah Suruur

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.