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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251215135301.75986b89@pumpkin>
Date: Mon, 15 Dec 2025 13:53:01 +0000
From: David Laight <david.laight.linux@...il.com>
To: Florian Westphal <fw@...len.de>
Cc: Anders Grahn <anders.grahn@...il.com>, Pablo Neira Ayuso
 <pablo@...filter.org>, Jozsef Kadlecsik <kadlec@...filter.org>, Phil Sutter
 <phil@....cc>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
 <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, Sebastian Andrzej
 Siewior <bigeasy@...utronix.de>, Anders Grahn <anders.grahn@...termo.com>,
 linux-kernel@...r.kernel.org, netfilter-devel@...r.kernel.org,
 coreteam@...filter.org, netdev@...r.kernel.org
Subject: Re: [PATCH] netfilter: nft_counter: Fix reset of counters on 32bit
 archs

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

> Anders Grahn <anders.grahn@...il.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@...termo.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