Re: "nft reset counters" bug on 32-bit systems

[Florian Westphal <fw@strlen.de>]

Andreas Fried <afried@emlix.com> wrote:
> This was tested on an arm32 system running kernel 6.1.134 and nftables
> 1.0.9, but as far as I can see, the code is the same on current master.
> Looking at nft_counter_reset(), this is a problem for systems where long
> is 32 bits wide.

Good catch.

> nft_counter_reset() wants to subtract the current total from the
> counter, so it calls u64_stats_add() with a negative "val" argument
> (e.g. -total->packets, in our case -4). But that argument is an unsigned
> long (u32) being added to the u64 counter. That means that it actually
> adds 0xfffffffc to the counter, giving 0x100000000, a.k.a. 2**32.
> 
> Seeing that u64_stats are used all over the place, any change might
> break all sorts of other things. So I'm hesitating to suggest a patch,

I'd suggest to turn nft_counter_reset() into a variant of nft_counter_fetch()
that uses local_xchg() instead of reads.

Expensive but reset requests should be rare and its much clearer as to
what is happening vs. this
fetch-and-then-add-negative-total-to-one-pcpu-counter.