github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-07-27 17:47:41 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

netfilter: nf_conncount: gc and rcu fixes

Browse Source 
Another drive-by AI review:

1) tree_gc_worker fails to wrap around after it can't find more pending
   work.  Update data->gc_tree unconditionally.  If its 0, start from
   the first pending tree (which can be 0).

2) tree_gc_worker() iterates the rbtree without lock. This is never
   safe.  Move iteration under the spinlock.  If this takes too long
   (resched needed), save key of next node, drop lock, resched, re-lock,
   then search for the key (node).  In very rare cases this node might
   no longer exist, in that case we can just wait for next gc.

3) use disable_work_sync(), we don't want any restarts.

4) module exit function needs rcu_barrier before we zap the kmem cache.

Fixes: 5c789e131c ("netfilter: nf_conncount: Add list lock and gc worker, and RCU for init tree search")
Closes: https://sashiko.dev/#/patchset/20260525182924.28456-1-fw%40strlen.de
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This commit is contained in:
Florian Westphal 2026-06-05 15:11:23 +02:00 committed by Pablo Neira Ayuso
parent 635a10f6d0
commit 4eb7c3db5b
1 changed files with 32 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
net/netfilter/nf_conncount.c
View File

@ -595,47 +595,54 @@ static void tree_gc_worker(struct work_struct *work)
{
struct nf_conncount_data *data = container_of(work, struct nf_conncount_data, gc_work);
struct nf_conncount_rb *gc_nodes[CONNCOUNT_GC_MAX_NODES], *rbconn;
unsigned int tree, next_tree, gc_count = 0;
struct nf_conncount_root *root;
struct rb_node *node;
unsigned int tree, next_tree, gc_count = 0;
if (data->gc_tree == 0)
data->gc_tree = find_first_bit(data->pending_trees, CONNCOUNT_SLOTS);
tree = data->gc_tree % CONNCOUNT_SLOTS;
root = &data->root[tree];
local_bh_disable();
rcu_read_lock();
for (node = rb_first(&root->root); node ; node = rb_next(node)) {
rbconn = rb_entry(node, struct nf_conncount_rb, node);
if (nf_conncount_gc_list(data->net, &rbconn->list))
gc_count++;
}
rcu_read_unlock();
local_bh_enable();
cond_resched();
spin_lock_bh(&root->lock);
if (gc_count < ARRAY_SIZE(gc_nodes))
goto next; /* do not bother */
gc_count = 0;
node = rb_first(&root->root);
while (node != NULL) {
u32 key[MAX_KEYLEN];
bool drop_lock;
rbconn = rb_entry(node, struct nf_conncount_rb, node);
node = rb_next(node);
if (rbconn->list.count > 0)
continue;
if (nf_conncount_gc_list(data->net, &rbconn->list))
gc_nodes[gc_count++] = rbconn;
gc_nodes[gc_count++] = rbconn;
if (gc_count >= ARRAY_SIZE(gc_nodes)) {
drop_lock = need_resched();
if (drop_lock || gc_count >= ARRAY_SIZE(gc_nodes)) {
tree_nodes_free(root, gc_nodes, gc_count);
gc_count = 0;
}
if (!drop_lock || !node)
continue;
rbconn = rb_entry(node, struct nf_conncount_rb, node);
memcpy(key, rbconn->key, sizeof(key));
spin_unlock_bh(&root->lock);
cond_resched();
spin_lock_bh(&root->lock);
rbconn = find_tree_node(root, data, key);
if (IS_ERR_OR_NULL(rbconn)) /* rbconn was reaped */
break;
node = &rbconn->node;
}
tree_nodes_free(root, gc_nodes, gc_count);
next:
clear_bit(tree, data->pending_trees);
next_tree = (tree + 1) % CONNCOUNT_SLOTS;
@ -644,6 +651,8 @@ static void tree_gc_worker(struct work_struct *work)
if (next_tree < CONNCOUNT_SLOTS) {
data->gc_tree = next_tree;
schedule_work(work);
} else {
data->gc_tree = 0;
}
spin_unlock_bh(&root->lock);
@ -726,7 +735,7 @@ void nf_conncount_destroy(struct net *net, struct nf_conncount_data *data)
{
unsigned int i;
cancel_work_sync(&data->gc_work);
disable_work_sync(&data->gc_work);
for (i = 0; i < ARRAY_SIZE(data->root); ++i)
destroy_tree(&data->root[i]);
@ -752,6 +761,7 @@ static int __init nf_conncount_modinit(void)
static void __exit nf_conncount_modexit(void)
{
rcu_barrier();
kmem_cache_destroy(conncount_conn_cachep);
kmem_cache_destroy(conncount_rb_cachep);
}