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

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

On Thu, Apr 30, 2026 at 08:11:07AM +0200, Pablo Neira Ayuso wrote:
> On Mon, Apr 27, 2026 at 11:21:18AM +0200, Fernando Fernandez Mancera wrote:
> > For lshift and rshift, the shift operations are performed in a loop over
> > 32-bit words. The loop calculates the shifted value and write it to dst,
> > and then immediately reads from src to calculate the carry for the next
> > iteration. Because src and dst could point to the same memory location,
> > the carry is incorrectly calculated using the newly modified dst value
> > instead of the original src value.
> > 
> > Adding a temporary local variable to cache the original value before
> > writing to dst and using it for the carry calculation solves the
> > problem. In addition, partial overlap is rejected from control plane for
> > all kind of operations. This was tested with the following bytecode:
> > 
> > table test_table ip flags 0 use 1 handle 1
> > ip test_table test_chain use 3 type filter hook input prio 0 policy accept packets 0 bytes 0 flags 1
> > ip test_table test_chain 2
> >   [ immediate reg 1 0x44332211 0x88776655 ]
> >   [ bitwise reg 1 = ( reg 1 << 0x08000000 ) ]
> >   [ cmp eq reg 1 0x66443322 0x00887766 ]
> >   [ counter pkts 0 bytes 0 ]
> > ip test_table test_chain 4 3
> >   [ immediate reg 1 0x44332211 0x88776655 ]
> >   [ bitwise reg 1 = ( reg 1 << 0x08000000 ) ]
> >   [ cmp eq reg 1 0x55443322 0x00887766 ]
> >   [ counter pkts 21794 bytes 1917798 ]
> > 
> > Fixes: 567d746b55bc ("netfilter: bitwise: add support for shifts.")
> > Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
> > ---
> > v2: handled partially register overlap
> > v3: reject partially overlap from control plane
> > v4: applied the partial overlap check to all operations
> > ---
> >  net/netfilter/nft_bitwise.c | 19 +++++++++++++++----
> >  1 file changed, 15 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/netfilter/nft_bitwise.c b/net/netfilter/nft_bitwise.c
> > index 13808e9cd999..76e7ae96429d 100644
> > --- a/net/netfilter/nft_bitwise.c
> > +++ b/net/netfilter/nft_bitwise.c
[...]
> > @@ -264,6 +269,12 @@ static int nft_bitwise_init(const struct nft_ctx *ctx,
> >  	if (err < 0)
> >  		return err;
> >  
> > +	n = DIV_ROUND_UP(priv->len, sizeof(u32));
> > +	if (priv->sreg != priv->dreg &&
> > +	    priv->dreg < priv->sreg + n &&
> > +	    priv->sreg < priv->dreg + n)
> > +		return -EINVAL;
> 
> In some cases, there is also sreg2 that probably needs to be handled
> too.

And probably nft_byteorder needs something similar to check for
partial overlaps too for sreg and dreg. Also nft_lookup.

Maybe add this to a helper function and use it from there?