[PATCH nf] netfilter: nf_tables: reject duplicate device on updates

[PATCH nf] netfilter: nf_tables: reject duplicate device on updates

[Pablo Neira Ayuso]

A chain/flowtable update with duplicated devices in the same batch is
possible. Unfortunately, netdev event path only removes the first
device that is found, leaving unregistered the hook of the duplicated
device.

Check if a duplicated device exists in the transaction batch, bail out
with EEXIST in such case.

WARNING is hit when unregistering the hook:

 [49042.221275] WARNING: CPU: 4 PID: 8425 at net/netfilter/core.c:340 nf_hook_entry_head+0xaa/0x150
 [49042.221375] CPU: 4 UID: 0 PID: 8425 Comm: nft Tainted: G S                  6.16.0+ #170 PREEMPT(full)
 [...]
 [49042.221382] RIP: 0010:nf_hook_entry_head+0xaa/0x150

Fixes: 78d9f48f7f44 ("netfilter: nf_tables: add devices to existing flowtable")
Fixes: b9703ed44ffb ("netfilter: nf_tables: support for adding new devices to an existing netdev chain")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 13d0ed9d1895..58c5425d61c2 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2803,6 +2803,7 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 	struct nft_chain *chain = ctx->chain;
 	struct nft_chain_hook hook = {};
 	struct nft_stats __percpu *stats = NULL;
+	struct nftables_pernet *nft_net;
 	struct nft_hook *h, *next;
 	struct nf_hook_ops *ops;
 	struct nft_trans *trans;
@@ -2845,6 +2846,20 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 				if (nft_hook_list_find(&basechain->hook_list, h)) {
 					list_del(&h->list);
 					nft_netdev_hook_free(h);
+					continue;
+				}
+
+				nft_net = nft_pernet(ctx->net);
+				list_for_each_entry(trans, &nft_net->commit_list, list) {
+					if (trans->msg_type != NFT_MSG_NEWCHAIN ||
+					    trans->table != ctx->table ||
+					    !nft_trans_chain_update(trans))
+						continue;
+
+					if (nft_hook_list_find(&nft_trans_chain_hooks(trans), h)) {
+						nft_chain_release_hook(&hook);
+						return -EEXIST;
+					}
 				}
 			}
 		} else {
@@ -9060,6 +9075,7 @@ static int nft_flowtable_update(struct nft_ctx *ctx, const struct nlmsghdr *nlh,
 {
 	const struct nlattr * const *nla = ctx->nla;
 	struct nft_flowtable_hook flowtable_hook;
+	struct nftables_pernet *nft_net;
 	struct nft_hook *hook, *next;
 	struct nf_hook_ops *ops;
 	struct nft_trans *trans;
@@ -9076,6 +9092,20 @@ static int nft_flowtable_update(struct nft_ctx *ctx, const struct nlmsghdr *nlh,
 		if (nft_hook_list_find(&flowtable->hook_list, hook)) {
 			list_del(&hook->list);
 			nft_netdev_hook_free(hook);
+			continue;
+		}
+
+		nft_net = nft_pernet(ctx->net);
+		list_for_each_entry(trans, &nft_net->commit_list, list) {
+			if (trans->msg_type != NFT_MSG_NEWFLOWTABLE ||
+			    trans->table != ctx->table ||
+			    !nft_trans_flowtable_update(trans))
+				continue;
+
+			if (nft_hook_list_find(&nft_trans_flowtable_hooks(trans), hook)) {
+				err = -EEXIST;
+				goto err_flowtable_update_hook;
+			}
 		}
 	}
 
-- 
2.30.2

Re: [PATCH nf] netfilter: nf_tables: reject duplicate device on updates

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> A chain/flowtable update with duplicated devices in the same batch is
> possible. Unfortunately, netdev event path only removes the first
> device that is found, leaving unregistered the hook of the duplicated
> device.
> 
> Check if a duplicated device exists in the transaction batch, bail out
> with EEXIST in such case.
> 
> WARNING is hit when unregistering the hook:
> 
>  [49042.221275] WARNING: CPU: 4 PID: 8425 at net/netfilter/core.c:340 nf_hook_entry_head+0xaa/0x150
>  [49042.221375] CPU: 4 UID: 0 PID: 8425 Comm: nft Tainted: G S                  6.16.0+ #170 PREEMPT(full)
>  [...]
>  [49042.221382] RIP: 0010:nf_hook_entry_head+0xaa/0x150

Thanks Pablo.

Just to confirm: this doesn't result in anything other than
the unreg splat, correct?

Or does this leak memory too?

FTR, i placed this in nf.git:testing.

Re: [PATCH nf] netfilter: nf_tables: reject duplicate device on updates

[Pablo Neira Ayuso]

On Wed, Aug 13, 2025 at 08:36:07AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > A chain/flowtable update with duplicated devices in the same batch is
> > possible. Unfortunately, netdev event path only removes the first
> > device that is found, leaving unregistered the hook of the duplicated
> > device.
> >
> > Check if a duplicated device exists in the transaction batch, bail out
> > with EEXIST in such case.
> >
> > WARNING is hit when unregistering the hook:
> >
> >  [49042.221275] WARNING: CPU: 4 PID: 8425 at net/netfilter/core.c:340 nf_hook_entry_head+0xaa/0x150
> >  [49042.221375] CPU: 4 UID: 0 PID: 8425 Comm: nft Tainted: G S                  6.16.0+ #170 PREEMPT(full)
> >  [...]
> >  [49042.221382] RIP: 0010:nf_hook_entry_head+0xaa/0x150
>
> Thanks Pablo.
>
> Just to confirm: this doesn't result in anything other than
> the unreg splat, correct?
>
> Or does this leak memory too?

It seems I tested on a kernel without CONFIG_KASAN, with it, it
reports UaF.

[   97.140749] ==================================================================
[   97.140762] BUG: KASAN: slab-use-after-free in nf_hook_entry_head+0xd9/0x140
[   97.140774] Read of size 8 at addr ffff88814feba108 by task nft/1097

> FTR, i placed this in nf.git:testing.

Thanks.