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

[Pablo Neira Ayuso <pablo@netfilter.org>]

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