Re: [PATCH] netfilter: nft_counter: Fix reset of counters on 32bit archs

[David Laight <david.laight.linux@gmail.com>]

On Mon, 15 Dec 2025 13:36:11 +0100
Florian Westphal <fw@strlen.de> wrote:

> Anders Grahn <anders.grahn@gmail.com> wrote:
> > nft_counter_reset() calls u64_stats_add() with a negative value to reset
> > the counter. This will work on 64bit archs, hence the negative value
> > added will wrap as a 64bit value which then can wrap the stat counter as
> > well.
> > 
> > On 32bit archs, the added negative value will wrap as a 32bit value and
> > _not_ wrapping the stat counter properly. In most cases, this would just
> > lead to a very large 32bit value being added to the stat counter.
> > 
> > Fix by introducing u64_stats_sub().
> > 
> > Fixes: 4a1d3acd6ea8 ("netfilter: nft_counter: Use u64_stats_t for statistic")
> > Signed-off-by: Anders Grahn <anders.grahn@westermo.com>
> > ---
> >  include/linux/u64_stats_sync.h | 10 ++++++++++
> >  net/netfilter/nft_counter.c    |  4 ++--
> >  2 files changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/u64_stats_sync.h b/include/linux/u64_stats_sync.h
> > index 457879938fc1..9942d29b17e5 100644
> > --- a/include/linux/u64_stats_sync.h
> > +++ b/include/linux/u64_stats_sync.h
> > @@ -89,6 +89,11 @@ static inline void u64_stats_add(u64_stats_t *p, unsigned long val)
> >  	local64_add(val, &p->v);
> >  }
> >  
> > +static inline void u64_stats_sub(u64_stats_t *p, unsigned long val)
> > +{
> > +	local64_sub(val, &p->v);
> > +}  
> 
> That still truncates val on 32bit.  Maybe use "s64 val"?
> 

It probably depends on the type of total->bytes and total->packets.
The 'bytes' value will wrap 32bits quickly, so may need to be 64bit anyway.
And if (elsewhere) there are this_cpu->bytes += val; total->bytes += val;
pairs you can't 'undo' the adds once total->bytes has wrapped.
So should any of these types be 'long' at all?

I sometimes think that 'long' should be pretty much never used in the
kernel because of the 32bit/64bit portability issues.
But that should have been sorted over 20 years ago.
Maybe M$ had it right keeping long as 32bits :-)

	David