Re: [PATCH nf v3] netfilter: nft_bitwise: fix dst corruption in same register shifts

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

Hi Fernando,

On Fri, Apr 24, 2026 at 05:03:17PM +0200, Fernando Fernandez Mancera wrote:
> On 4/24/26 1:18 PM, Pablo Neira Ayuso wrote:
> > Hi Fernando,
> > 
> > On Thu, Apr 23, 2026 at 05:54:53PM +0200, Fernando Fernandez Mancera wrote:
> > [...]
> > > @@ -201,6 +206,11 @@ static int nft_bitwise_init_shift(struct nft_bitwise *priv,
> > >   		return -EINVAL;
> > >   	}
> > > +	if (priv->sreg != priv->dreg &&
> > > +	    priv->dreg < priv->sreg + n &&
> > > +	    priv->sreg < priv->dreg + n)
> > 
> > Is this enough? Just to make sure we are on the same page.
> > 
> > NFT_REG_1
> > NFT_REG_2
> > NFT_REG_3
> > NFT_REG_4
> > 
> > have a size of 128 bytes.
> > 
> 
> Right but if I am not wrong these registers are mapped/normalized. That
> happens during nft_parse_register() earlier in the init() path.

Indeed, registers has been already normalized by nft_parse_register()
at this stage.

> Therefore we must expect priv->sreg/dreg in the range [4, 19].
> 
> > Then, NFT_REG32_00, NFT_REG32_01, NFT_REG32_02 and NFT_REG32_03
> > basically overlap with NFT_REG_1. They split the 128 bytes of
> > NFT_REG_1 in 4 registers of 32 bytes.
> > 
> > Is this check above enough to deal with the partial overlaps?
> > 
> 
> I am not very good at math but as long as we have the length of the data we
> can calculate the overlap in 4 bytes segments. Of course if from userspace
> you mix both APIs the math should hold up.
> 
> let's say we have NFT_REG_1 as sreg and NFT_REG32_O1 as dreg and length of 8
> bytes.
> 
> That is after normalization:
> 
> sreg: 4 and dreg: 5
> 
> sreg expands through registers 4 and 5
> dreg expands through registers 5 and 6
> 
> The check is able to catch it. Of course, if the length would be 4 bytes,
> the check would pass but that is fine.
> 
> At the end NFT_REG_1 is mapped to 32bits register number 4 while
> NFT_REG32_O1 is mapped to 32 bits register number 5.
> 
> Does this make sense? Anyway, AI suggested if this should be applied XOR,
> OR, AND, etc. I think yes, as the partial overlap could corrupt the result
> there too. So a v4 is needed anyway.

OK, let's do that.

Thanks.