mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH nf] netfilter: ipset: fix refcount race between list:set GC and swap
@ 2026-07-22 22:38 Xiang Mei (Microsoft)
  2026-07-23  7:32 ` Jozsef Kadlecsik
  0 siblings, 1 reply; 3+ messages in thread
From: Xiang Mei (Microsoft) @ 2026-07-22 22:38 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Florian Westphal, Phil Sutter
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Jozsef Kadlecsik, netfilter-devel, coreteam,
	netdev, linux-kernel, AutonomousCodeSecurity, tgopinath, kys,
	Xiang Mei (Microsoft)

__ip_set_put_byindex() resolved the index to a set pointer under RCU,
then took ip_set_ref_lock in __ip_set_put() to decrement set->ref.
ip_set_swap() holds that same lock while swapping both the ip_set_list
slots and the two sets' ref counters, so it can interleave between the
dereference and the lock acquisition, leaving the caller to decrement a
set whose reference already moved to the other index and hit
BUG_ON(set->ref == 0). list_set_gc() reaches this from timer softirq,
which the nfnl mutex does not serialize against swap: an expiring
list:set member calls list_set_del() -> ip_set_put_byindex() while
IPSET_CMD_SWAP runs on the referenced sets.

Resolve the index and decrement under ip_set_ref_lock, as ip_set_swap()
already does, keeping the refcount tied to the index rather than to a
stale set pointer.

  kernel BUG at net/netfilter/ipset/ip_set_core.c:685!
  Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
  RIP: 0010:ip_set_put_byindex (net/netfilter/ipset/ip_set_core.c:870)
  Call Trace:
   <IRQ>
   list_set_del (net/netfilter/ipset/ip_set_list_set.c:159)
   set_cleanup_entries (net/netfilter/ipset/ip_set_list_set.c:181)
   list_set_gc (net/netfilter/ipset/ip_set_list_set.c:578)
   call_timer_fn (kernel/time/timer.c:1748)
   __run_timers (kernel/time/timer.c:1799 kernel/time/timer.c:2374)
   run_timer_softirq (kernel/time/timer.c:2405)
   </IRQ>
  Kernel panic - not syncing: Fatal exception in interrupt

Fixes: 9076aea76538 ("netfilter: ipset: Increase the number of maximal sets automatically")
Reported-by: AutonomousCodeSecurity@microsoft.com
Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
---
 net/netfilter/ipset/ip_set_core.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 6cfad152d7d1..16d33c3e28cf 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -679,11 +679,18 @@ __ip_set_get(struct ip_set *set)
 }
 
 static void
-__ip_set_put(struct ip_set *set)
+__ip_set_put_locked(struct ip_set *set)
 {
-	write_lock_bh(&ip_set_ref_lock);
+	lockdep_assert_held(&ip_set_ref_lock);
 	BUG_ON(set->ref == 0);
 	set->ref--;
+}
+
+static void
+__ip_set_put(struct ip_set *set)
+{
+	write_lock_bh(&ip_set_ref_lock);
+	__ip_set_put_locked(set);
 	write_unlock_bh(&ip_set_ref_lock);
 }
 
@@ -854,11 +861,11 @@ __ip_set_put_byindex(struct ip_set_net *inst, ip_set_id_t index)
 {
 	struct ip_set *set;
 
-	rcu_read_lock();
-	set = rcu_dereference(inst->ip_set_list)[index];
+	write_lock_bh(&ip_set_ref_lock);
+	set = ip_set(inst, index);
 	if (set)
-		__ip_set_put(set);
-	rcu_read_unlock();
+		__ip_set_put_locked(set);
+	write_unlock_bh(&ip_set_ref_lock);
 }
 
 void
-- 
2.43.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH nf] netfilter: ipset: fix refcount race between list:set GC and swap
  2026-07-22 22:38 [PATCH nf] netfilter: ipset: fix refcount race between list:set GC and swap Xiang Mei (Microsoft)
@ 2026-07-23  7:32 ` Jozsef Kadlecsik
  2026-07-25 21:59   ` Xiang Mei
  0 siblings, 1 reply; 3+ messages in thread
From: Jozsef Kadlecsik @ 2026-07-23  7:32 UTC (permalink / raw)
  To: Xiang Mei (Microsoft)
  Cc: Pablo Neira Ayuso, Florian Westphal, Phil Sutter,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Jozsef Kadlecsik, netfilter-devel, coreteam,
	netdev, linux-kernel, AutonomousCodeSecurity, tgopinath, kys

On Wed, 22 Jul 2026, Xiang Mei (Microsoft) wrote:

> __ip_set_put_byindex() resolved the index to a set pointer under RCU,
> then took ip_set_ref_lock in __ip_set_put() to decrement set->ref.
> ip_set_swap() holds that same lock while swapping both the ip_set_list
> slots and the two sets' ref counters, so it can interleave between the
> dereference and the lock acquisition, leaving the caller to decrement a
> set whose reference already moved to the other index and hit
> BUG_ON(set->ref == 0). list_set_gc() reaches this from timer softirq,
> which the nfnl mutex does not serialize against swap: an expiring
> list:set member calls list_set_del() -> ip_set_put_byindex() while
> IPSET_CMD_SWAP runs on the referenced sets.
>
> Resolve the index and decrement under ip_set_ref_lock, as ip_set_swap()
> already does, keeping the refcount tied to the index rather than to a
> stale set pointer.
>
>  kernel BUG at net/netfilter/ipset/ip_set_core.c:685!
>  Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
>  RIP: 0010:ip_set_put_byindex (net/netfilter/ipset/ip_set_core.c:870)
>  Call Trace:
>   <IRQ>
>   list_set_del (net/netfilter/ipset/ip_set_list_set.c:159)
>   set_cleanup_entries (net/netfilter/ipset/ip_set_list_set.c:181)
>   list_set_gc (net/netfilter/ipset/ip_set_list_set.c:578)
>   call_timer_fn (kernel/time/timer.c:1748)
>   __run_timers (kernel/time/timer.c:1799 kernel/time/timer.c:2374)
>   run_timer_softirq (kernel/time/timer.c:2405)
>   </IRQ>
>  Kernel panic - not syncing: Fatal exception in interrupt
>
> Fixes: 9076aea76538 ("netfilter: ipset: Increase the number of maximal sets automatically")
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>

Acked-by: Jozsef Kadlecsik <kadlec@netfilter.org>

Best regards,
Jozsef

> ---
> net/netfilter/ipset/ip_set_core.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
> index 6cfad152d7d1..16d33c3e28cf 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -679,11 +679,18 @@ __ip_set_get(struct ip_set *set)
> }
>
> static void
> -__ip_set_put(struct ip_set *set)
> +__ip_set_put_locked(struct ip_set *set)
> {
> -	write_lock_bh(&ip_set_ref_lock);
> +	lockdep_assert_held(&ip_set_ref_lock);
> 	BUG_ON(set->ref == 0);
> 	set->ref--;
> +}
> +
> +static void
> +__ip_set_put(struct ip_set *set)
> +{
> +	write_lock_bh(&ip_set_ref_lock);
> +	__ip_set_put_locked(set);
> 	write_unlock_bh(&ip_set_ref_lock);
> }
>
> @@ -854,11 +861,11 @@ __ip_set_put_byindex(struct ip_set_net *inst, ip_set_id_t index)
> {
> 	struct ip_set *set;
>
> -	rcu_read_lock();
> -	set = rcu_dereference(inst->ip_set_list)[index];
> +	write_lock_bh(&ip_set_ref_lock);
> +	set = ip_set(inst, index);
> 	if (set)
> -		__ip_set_put(set);
> -	rcu_read_unlock();
> +		__ip_set_put_locked(set);
> +	write_unlock_bh(&ip_set_ref_lock);
> }
>
> void
> -- 
> 2.43.0
>
>
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH nf] netfilter: ipset: fix refcount race between list:set GC and swap
  2026-07-23  7:32 ` Jozsef Kadlecsik
@ 2026-07-25 21:59   ` Xiang Mei
  0 siblings, 0 replies; 3+ messages in thread
From: Xiang Mei @ 2026-07-25 21:59 UTC (permalink / raw)
  To: Jozsef Kadlecsik
  Cc: Pablo Neira Ayuso, Florian Westphal, Phil Sutter,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Jozsef Kadlecsik, netfilter-devel, coreteam,
	netdev, linux-kernel, AutonomousCodeSecurity, tgopinath, kys

On Thu, Jul 23, 2026 at 12:33 AM Jozsef Kadlecsik
<kadlec@blackhole.kfki.hu> wrote:
>
> On Wed, 22 Jul 2026, Xiang Mei (Microsoft) wrote:
>
> > __ip_set_put_byindex() resolved the index to a set pointer under RCU,
> > then took ip_set_ref_lock in __ip_set_put() to decrement set->ref.
> > ip_set_swap() holds that same lock while swapping both the ip_set_list
> > slots and the two sets' ref counters, so it can interleave between the
> > dereference and the lock acquisition, leaving the caller to decrement a
> > set whose reference already moved to the other index and hit
> > BUG_ON(set->ref == 0). list_set_gc() reaches this from timer softirq,
> > which the nfnl mutex does not serialize against swap: an expiring
> > list:set member calls list_set_del() -> ip_set_put_byindex() while
> > IPSET_CMD_SWAP runs on the referenced sets.
> >
> > Resolve the index and decrement under ip_set_ref_lock, as ip_set_swap()
> > already does, keeping the refcount tied to the index rather than to a
> > stale set pointer.
> >
> >  kernel BUG at net/netfilter/ipset/ip_set_core.c:685!
> >  Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
> >  RIP: 0010:ip_set_put_byindex (net/netfilter/ipset/ip_set_core.c:870)
> >  Call Trace:
> >   <IRQ>
> >   list_set_del (net/netfilter/ipset/ip_set_list_set.c:159)
> >   set_cleanup_entries (net/netfilter/ipset/ip_set_list_set.c:181)
> >   list_set_gc (net/netfilter/ipset/ip_set_list_set.c:578)
> >   call_timer_fn (kernel/time/timer.c:1748)
> >   __run_timers (kernel/time/timer.c:1799 kernel/time/timer.c:2374)
> >   run_timer_softirq (kernel/time/timer.c:2405)
> >   </IRQ>
> >  Kernel panic - not syncing: Fatal exception in interrupt
> >
> > Fixes: 9076aea76538 ("netfilter: ipset: Increase the number of maximal sets automatically")
> > Reported-by: AutonomousCodeSecurity@microsoft.com
> > Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
>
> Acked-by: Jozsef Kadlecsik <kadlec@netfilter.org>
>
> Best regards,
> Jozsef

Awesome, thank you for your review!

-Xiang
>
> > ---
> > net/netfilter/ipset/ip_set_core.c | 19 +++++++++++++------
> > 1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
> > index 6cfad152d7d1..16d33c3e28cf 100644
> > --- a/net/netfilter/ipset/ip_set_core.c
> > +++ b/net/netfilter/ipset/ip_set_core.c
> > @@ -679,11 +679,18 @@ __ip_set_get(struct ip_set *set)
> > }
> >
> > static void
> > -__ip_set_put(struct ip_set *set)
> > +__ip_set_put_locked(struct ip_set *set)
> > {
> > -     write_lock_bh(&ip_set_ref_lock);
> > +     lockdep_assert_held(&ip_set_ref_lock);
> >       BUG_ON(set->ref == 0);
> >       set->ref--;
> > +}
> > +
> > +static void
> > +__ip_set_put(struct ip_set *set)
> > +{
> > +     write_lock_bh(&ip_set_ref_lock);
> > +     __ip_set_put_locked(set);
> >       write_unlock_bh(&ip_set_ref_lock);
> > }
> >
> > @@ -854,11 +861,11 @@ __ip_set_put_byindex(struct ip_set_net *inst, ip_set_id_t index)
> > {
> >       struct ip_set *set;
> >
> > -     rcu_read_lock();
> > -     set = rcu_dereference(inst->ip_set_list)[index];
> > +     write_lock_bh(&ip_set_ref_lock);
> > +     set = ip_set(inst, index);
> >       if (set)
> > -             __ip_set_put(set);
> > -     rcu_read_unlock();
> > +             __ip_set_put_locked(set);
> > +     write_unlock_bh(&ip_set_ref_lock);
> > }
> >
> > void
> > --
> > 2.43.0
> >
> >
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-25 21:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-22 22:38 [PATCH nf] netfilter: ipset: fix refcount race between list:set GC and swap Xiang Mei (Microsoft)
2026-07-23  7:32 ` Jozsef Kadlecsik
2026-07-25 21:59   ` Xiang Mei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®