Re: [PATCH nf] netfilter: ipset: fix refcount race between list:set GC and swap

On Thu, Jul 23, 2026 at 12:33 AM Jozsef Kadlecsik
<[email protected]> 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: [email protected]
> > Signed-off-by: Xiang Mei (Microsoft) <[email protected]>
>
> Acked-by: Jozsef Kadlecsik <[email protected]>
>
> 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
> >
> >
> >