Re: [PATCH net] netfilter: ipset: list:set: defer ip_set_put_byindex to RCU callback

luoxuanqiang <[email protected]>
Newsgroups gmane.comp.security.firewalls.netfilter.devel,gmane.linux.network,gmane.linux.kernel
Message-ID <[email protected]>
在 2026/8/20 09:06, Cen Zhang (Microsoft) 写道:
> list_set_del() and list_set_replace() call ip_set_put_byindex() before
> call_rcu(), dropping the child set refcount to zero. Meanwhile,
> list_set_ktest() on another CPU may have already read e->id under
> rcu_read_lock(). A concurrent ip_set_destroy() sees refcount zero,
> NULLs ip_set_list[index], and frees the child set. list_set_ktest()
> then passes the now-dangling index to ip_set_test(), ip_set_rcu_get()
> returns NULL, and BUG_ON(!set) fires at ip_set_core.c:746.
>
>   kernel BUG at net/netfilter/ipset/ip_set_core.c:746!
>     ip_set_test+0x329/0x590
>     list_set_kadt+0x2a6/0x810
>     ip_set_test+0x24f/0x590
>     set_match_v1+0x1a9/0x280
>     ipt_do_table+0x83d/0x1360
>     nf_hook_slow+0xac/0x1e0
>
> Move ip_set_put_byindex() from list_set_del()/list_set_replace() into
> the __list_set_del_rcu() callback so the refcount is not decremented
> until all RCU readers have finished.
>
> Fixes: 439cd39ea136 ("netfilter: ipset: list:set: Decrease refcount synchronously on deletion and replace")
> Reported-by: [email protected]
> Reported-by: Xiang Mei (Microsoft) <[email protected]>
> Reported-by: Cen Zhang (Microsoft) <[email protected]>
> Signed-off-by: Cen Zhang (Microsoft) <[email protected]>
> ---
>   net/netfilter/ipset/ip_set_list_set.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/net/netfilter/ipset/ip_set_list_set.c b/net/netfilter/ipset/ip_set_list_set.c
> index f070088742d6..aeee0c6c314d 100644
> --- a/net/netfilter/ipset/ip_set_list_set.c
> +++ b/net/netfilter/ipset/ip_set_list_set.c
> @@ -144,7 +144,9 @@ __list_set_del_rcu(struct rcu_head * rcu)
>   {
>   	struct set_elem *e = container_of(rcu, struct set_elem, rcu);
>   	struct ip_set *set = e->set;
> +	struct list_set *map = set->data;
>   
> +	ip_set_put_byindex(map->net, e->id);
>   	ip_set_ext_destroy(set, e);
>   	kfree(e);
>   }
> @@ -152,21 +154,15 @@ __list_set_del_rcu(struct rcu_head * rcu)
>   static void
>   list_set_del(struct ip_set *set, struct set_elem *e)
>   {
> -	struct list_set *map = set->data;
> -
>   	set->elements--;
>   	list_del_rcu(&e->list);
> -	ip_set_put_byindex(map->net, e->id);
>   	call_rcu(&e->rcu, __list_set_del_rcu);
>   }
>   
>   static void
>   list_set_replace(struct ip_set *set, struct set_elem *e, struct set_elem *old)
>   {
> -	struct list_set *map = set->data;
> -
>   	list_replace_rcu(&old->list, &e->list);
> -	ip_set_put_byindex(map->net, old->id);
>   	call_rcu(&old->rcu, __list_set_del_rcu);
>   }
>   

Thanks for the patch.

But this is effectively a revert of 439cd39ea136 and brings back the
problem it fixed: within the grace period after the del, userspace still
sees a stale refcount, and destroy/rename are briefly rejected as well.

Please take a look at that commit's message first.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.