[PATCH nf] netfilter: nf_tables: use RCU-safe list primitives for basechain hook list

Weiming Shi
2026-04-10T10:14:01Z

NFT_MSG_GETCHAIN runs as an NFNL_CB_RCU callback, so chain dumps
traverse basechain->hook_list under rcu_read_lock() without holding
commit_mutex. Meanwhile, nft_delchain_hook() mutates that same live
hook_list with plain list_move() and list_splice(), and the commit/abort
paths splice hooks back with plain list_splice(). None of these are
RCU-safe list operations.

A concurrent GETCHAIN dump can observe partially updated list pointers,
follow them into stack-local or transaction-private list heads, and
crash when container_of() produces a bogus struct nft_hook pointer.

The PoC triggers this by racing GETCHAIN dumps against aborting DELCHAIN
hook updates, reachable from an unprivileged user namespace since all
capability checks use ns_capable() with CONFIG_NF_TABLES=y (default):

Oops: general protection fault, probably for non-canonical address 0xdffffc0000000006: 0000 [#1] SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000030-0x0000000000000037]
RIP: 0010:strlen (lib/string.c:420 (discriminator 1))
Call Trace:
<TASK>
nf_tables_fill_chain_info (net/netfilter/nf_tables_api.c:1987 (discriminator 1) net/netfilter/nf_tables_api.c:1992 (discriminator 1) net/netfilter/nf_tables_api.c:2028 (discriminator 1) net/netfilter/nf_tables_api.c:2077 (discriminator 1))
nf_tables_dump_chains (net/netfilter/nf_tables_api.c:2173 (discriminator 1))
netlink_dump (net/netlink/af_netlink.c:2325 (discriminator 1))
__netlink_dump_start (net/netlink/af_netlink.c:2442)
nf_tables_getchain (net/netfilter/nf_tables_api.c:1314 net/netfilter/nf_tables_api.c:2212)
nfnetlink_rcv_msg (net/netfilter/nfnetlink.c:290)
netlink_rcv_skb (net/netlink/af_netlink.c:2550)
nfnetlink_rcv (net/netfilter/nfnetlink.c:653)
netlink_unicast (net/netlink/af_netlink.c:1319 net/netlink/af_netlink.c:1344)
netlink_sendmsg (net/netlink/af_netlink.c:1894)
__sys_sendto (net/socket.c:727 net/socket.c:742 net/socket.c:2206)
__x64_sys_sendto (net/socket.c:2209)
</TASK>

Replace list_move() in nft_delchain_hook() with list_del_rcu() plus an
intermediate pointer array, followed by synchronize_rcu() before the
deleted hooks' list pointers are reused to link them into the
transaction's private list. In the error paths, put hooks back with
list_add_tail_rcu() which is safe for concurrent RCU readers (they
either continue to the original successor or see the list head and
terminate the walk).

Add nft_hook_list_splice_rcu() helper that splices entries from a
private list into a live RCU-protected list using individual
list_add_tail_rcu() calls instead of plain list_splice(). Use it in
the commit and abort paths for NEWCHAIN updates and DELCHAIN rollback.

Fixes: 7d937b107108 ("netfilter: nf_tables: support for deleting devices in an existing netdev chain")
Reported-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
net/netfilter/nf_tables_api.c | 64 ++++++++++++++++++++++++++++++-----
1 file changed, 56 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 8c42247a176c7..62fcfefba7b0f 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -391,6 +391,22 @@ static void nft_netdev_unregister_hooks(struct net *net,
}
}

+/* Splice hooks from a private list into a live (RCU-protected) hook list.
+ * Each entry is published individually via list_add_tail_rcu() so that
+ * concurrent RCU readers walking the destination list never observe torn
+ * list pointers.
+ */
+static void nft_hook_list_splice_rcu(struct list_head *from,
+ struct list_head *to)
+{
+ struct nft_hook *hook, *next;
+
+ list_for_each_entry_safe(hook, next, from, list) {
+ list_del(&hook->list);
+ list_add_tail_rcu(&hook->list, to);
+ }
+}
+
static int nf_tables_register_hook(struct net *net,
const struct nft_table *table,
struct nft_chain *chain)
@@ -3162,9 +3178,11 @@ static int nft_delchain_hook(struct nft_ctx *ctx,
const struct nlattr * const *nla = ctx->nla;
struct nft_chain_hook chain_hook = {};
struct nft_hook *this, *hook;
+ struct nft_hook **del_hooks;
LIST_HEAD(chain_del_list);
struct nft_trans *trans;
- int err;
+ int err, n = 0, i;
+ int max_hooks = 0;

if (ctx->table->flags & __NFT_TABLE_F_UPDATE)
return -EOPNOTSUPP;
@@ -3174,19 +3192,38 @@ static int nft_delchain_hook(struct nft_ctx *ctx,
if (err < 0)
return err;

+ list_for_each_entry(this, &chain_hook.list, list)
+ max_hooks++;
+
+ del_hooks = kcalloc(max_hooks, sizeof(*del_hooks), GFP_KERNEL);
+ if (!del_hooks) {
+ nft_chain_release_hook(&chain_hook);
+ return -ENOMEM;
+ }
+
list_for_each_entry(this, &chain_hook.list, list) {
hook = nft_hook_list_find(&basechain->hook_list, this);
if (!hook) {
err = -ENOENT;
goto err_chain_del_hook;
}
- list_move(&hook->list, &chain_del_list);
+ list_del_rcu(&hook->list);
+ del_hooks[n++] = hook;
}

+ /* Wait for any concurrent RCU readers (e.g. GETCHAIN dumps walking
+ * basechain->hook_list) to finish before modifying the removed hooks'
+ * list pointers to link them into the transaction's private list.
+ */
+ synchronize_rcu();
+
+ for (i = 0; i < n; i++)
+ list_add_tail(&del_hooks[i]->list, &chain_del_list);
+
trans = nft_trans_alloc_chain(ctx, NFT_MSG_DELCHAIN);
if (!trans) {
err = -ENOMEM;
- goto err_chain_del_hook;
+ goto err_chain_add_back;
}

nft_trans_basechain(trans) = basechain;
@@ -3194,13 +3231,24 @@ static int nft_delchain_hook(struct nft_ctx *ctx,
INIT_LIST_HEAD(&nft_trans_chain_hooks(trans));
list_splice(&chain_del_list, &nft_trans_chain_hooks(trans));
nft_chain_release_hook(&chain_hook);
+ kfree(del_hooks);

nft_trans_commit_list_add_tail(ctx->net, trans);

return 0;

+err_chain_add_back:
+ for (i = 0; i < n; i++)
+ list_add_tail_rcu(&del_hooks[i]->list, &basechain->hook_list);
+ kfree(del_hooks);
+ nft_chain_release_hook(&chain_hook);
+
+ return err;
+
err_chain_del_hook:
- list_splice(&chain_del_list, &basechain->hook_list);
+ for (i = 0; i < n; i++)
+ list_add_tail_rcu(&del_hooks[i]->list, &basechain->hook_list);
+ kfree(del_hooks);
nft_chain_release_hook(&chain_hook);

return err;
@@ -10912,8 +10960,8 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
nft_chain_commit_update(nft_trans_container_chain(trans));
nf_tables_chain_notify(&ctx, NFT_MSG_NEWCHAIN,
&nft_trans_chain_hooks(trans));
- list_splice(&nft_trans_chain_hooks(trans),
- &nft_trans_basechain(trans)->hook_list);
+ nft_hook_list_splice_rcu(&nft_trans_chain_hooks(trans),
+ &nft_trans_basechain(trans)->hook_list);
/* trans destroyed after rcu grace period */
} else {
nft_chain_commit_drop_policy(nft_trans_container_chain(trans));
@@ -11231,8 +11279,8 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
case NFT_MSG_DELCHAIN:
case NFT_MSG_DESTROYCHAIN:
if (nft_trans_chain_update(trans)) {
- list_splice(&nft_trans_chain_hooks(trans),
- &nft_trans_basechain(trans)->hook_list);
+ nft_hook_list_splice_rcu(&nft_trans_chain_hooks(trans),
+ &nft_trans_basechain(trans)->hook_list);
} else {
nft_use_inc_restore(&table->use);
nft_clear(trans->net, nft_trans_chain(trans));
--
2.43.0