The patch fdb8e12cc2cc ("netfilter: ipset: fix performance regression in swap operation") missed to add the calls to gc cancellations at the error path of create operations and at module unload. Also, because the half of the destroy operations now executed by a function registered by call_rcu(), neither NFNL_SUBSYS_IPSET mutex or rcu read lock is held and therefore the checking of them results false warnings.
Reported-by: syzbot+52bbc0ad036f6f0d4a25@syzkaller.appspotmail.com Reported-by: Brad Spengler spender@grsecurity.net Reported-by: Стас Ничипорович stasn77@gmail.com Fixes: fdb8e12cc2cc ("netfilter: ipset: fix performance regression in swap operation") Tested-by: Brad Spengler spender@grsecurity.net Tested-by: Стас Ничипорович stasn77@gmail.com Signed-off-by: Jozsef Kadlecsik kadlec@netfilter.org --- net/netfilter/ipset/ip_set_core.c | 2 ++ net/netfilter/ipset/ip_set_hash_gen.h | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c index bcaad9c009fe..3184cc6be4c9 100644 --- a/net/netfilter/ipset/ip_set_core.c +++ b/net/netfilter/ipset/ip_set_core.c @@ -1154,6 +1154,7 @@ static int ip_set_create(struct sk_buff *skb, const struct nfnl_info *info, return ret;
cleanup: + set->variant->cancel_gc(set); set->variant->destroy(set); put_out: module_put(set->type->me); @@ -2378,6 +2379,7 @@ ip_set_net_exit(struct net *net) set = ip_set(inst, i); if (set) { ip_set(inst, i) = NULL; + set->variant->cancel_gc(set); ip_set_destroy_set(set); } } diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h index c62998b46f00..7f362cad8e68 100644 --- a/net/netfilter/ipset/ip_set_hash_gen.h +++ b/net/netfilter/ipset/ip_set_hash_gen.h @@ -431,7 +431,7 @@ mtype_ahash_destroy(struct ip_set *set, struct htable *t, bool ext_destroy) u32 i;
for (i = 0; i < jhash_size(t->htable_bits); i++) { - n = __ipset_dereference(hbucket(t, i)); + n = hbucket(t, i); if (!n) continue; if (set->extensions & IPSET_EXT_DESTROY && ext_destroy) @@ -451,7 +451,7 @@ mtype_destroy(struct ip_set *set) struct htype *h = set->data; struct list_head *l, *lt;
- mtype_ahash_destroy(set, ipset_dereference_nfnl(h->table), true); + mtype_ahash_destroy(set, h->table, true); list_for_each_safe(l, lt, &h->ad) { list_del(l); kfree(l);
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [PATCH 1/1] netfilter: ipset: Missing gc cancellations fixed Link: https://lore.kernel.org/stable/20240204152642.1394588-1-kadlec%40netfilter.o...
On 04.02.24 16:26, Jozsef Kadlecsik wrote:
The patch fdb8e12cc2cc ("netfilter: ipset: fix performance regression in swap operation") missed to add the calls to gc cancellations at the error path of create operations and at module unload. Also, because the half of the destroy operations now executed by a function registered by call_rcu(), neither NFNL_SUBSYS_IPSET mutex or rcu read lock is held and therefore the checking of them results false warnings.
Reported-by: syzbot+52bbc0ad036f6f0d4a25@syzkaller.appspotmail.com Reported-by: Brad Spengler spender@grsecurity.net Reported-by: Стас Ничипорович stasn77@gmail.com Fixes: fdb8e12cc2cc ("netfilter: ipset: fix performance regression in swap operation")
That afaics should be 97f7cf1cd80e ("netfilter: ipset: fix performance regression in swap operation").
Side note in case anyone cares: I first didn't add the problem to the regression tracking as I assumed the fix would get quickly reviewed and merged to mainline (for some patches going through -net that's the case), but now added it as nothing happened yet.
Ciao, Thorsten
linux-stable-mirror@lists.linaro.org