[PATCH net v2] netfilter: ipset: add synchronize_rcu() in destroy to close use-after-free race
From: Cen Zhang (Microsoft) <hidden>
Date: 2026-08-25 20:02:58
Also in:
lkml, netfilter-devel
Subsystem:
netfilter, networking [general], the rest · Maintainers:
Pablo Neira Ayuso, Florian Westphal, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
The child set refcount decrement in list_set_del() was moved from an RCU
callback to the synchronous path, so that userspace sees accurate
reference counts immediately. However, this broke an implicit invariant:
previously ref could only reach zero after an RCU grace period,
guaranteeing all RCU readers had finished before destroy could proceed.
Now ref can hit zero while readers still hold a stale index, and
ip_set_destroy() NULLs the slot out from under them:
CPU 0 (softirq) CPU 1 (control path)
--- ---
rcu_read_lock()
index = e->id
list_set_del():
list_del_rcu(e)
ip_set_put_byindex(index) // ref->0
ip_set_destroy():
ip_set_list[index] = NULL
ip_set_rcu_get(index) -> NULL
BUG_ON(!set) // crash
kernel BUG at net/netfilter/ipset/ip_set_core.c:754!
ip_set_test <- list_set_kadt <- ip_set_test <- set_match_v1
Insert synchronize_rcu() in ip_set_destroy() after confirming ref == 0
but before NULLing the slot, so it only pays the RCU wait cost when
actually destroying. Because synchronize_rcu() sleeps, ip_set_ref_lock
must be dropped first, which splits the critical section in two. A
recheck of ref/ref_netlink is therefore needed in the second section,
since a concurrent netlink dump continuation (which does not hold
nfnl_lock) may have incremented ref_netlink in the interim.
The bulk _destroy_all_sets() path does not need this recheck because
its is_destroyed flag prevents dump from taking new references.
Fixes: 439cd39ea136 ("netfilter: ipset: list:set: Decrease refcount synchronously on deletion and replace")
Reported-by: AutonomousCodeSecurity@microsoft.com
Reported-by: Xiang Mei (Microsoft) <redacted>
Reported-by: Cen Zhang (Microsoft) <redacted>
Closes: https://lore.kernel.org/all/20260820010617.46851-1-blbllhy@gmail.com/ (local)
Signed-off-by: Cen Zhang (Microsoft) <redacted>
---
net/netfilter/ipset/ip_set_core.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 0a86a170ba90..1295bea7944a 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c@@ -1225,6 +1225,8 @@ _destroy_all_sets(struct ip_set_net *inst) /* Must wait for flush to be really finished */ if (need_wait) rcu_barrier(); + /* Wait for RCU readers before NULLing slots */ + synchronize_rcu(); for (i = 0; i < inst->ip_set_max; i++) { set = ip_set(inst, i); if (set) {
@@ -1286,6 +1288,17 @@ static int ip_set_destroy(struct sk_buff *skb, const struct nfnl_info *info, ret = -IPSET_ERR_BUSY; goto out; } + read_unlock_bh(&ip_set_ref_lock); + + /* Wait for RCU readers before NULLing slot */ + synchronize_rcu(); + + read_lock_bh(&ip_set_ref_lock); + /* Dump may have taken a ref while lock was dropped */ + if (s->ref || s->ref_netlink) { + ret = -IPSET_ERR_BUSY; + goto out; + } features = s->type->features; ip_set(inst, i) = NULL; read_unlock_bh(&ip_set_ref_lock);
--
2.55.0