| Message ID | 20240204152642.1394588-1-kadlec@netfilter.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] netfilter: ipset: Missing gc cancellations fixed | expand |
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
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);
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(-)