[PATCH request stable 6.12 6.6 6.1] netfilter: nf_tables_netdev_event UAF of binding chain

[PATCH request stable 6.12 6.6 6.1] netfilter: nf_tables_netdev_event UAF of binding chain

[Yu Junzhe]

Hello,

Please consider a backport of the following use-after-free to the
longterm trees that still have it: 6.12, 6.6, and 6.1. 6.18 and newer
are not affected.

There is no single upstream commit to cherry-pick. On current linux.git
the netdev notifier no longer calls __nft_release_basechain(); it only
unregisters the per-device hook. That change arrived as part of the
netdev-hook rework and does not apply to these stable trees.

Bug
===

nf_tables_netdev_event() walks table->chains with
list_for_each_entry_safe. On NETDEV_UNREGISTER of the last hook,
nft_netdev_event() calls __nft_release_basechain(). Releasing the base
chain deactivates a JUMP to an NFT_CHAIN_BINDING chain, which
nft_chain_del()s that successor and nft_immediate_destroy() kfree()s
it. The walker's saved nr still points at the freed nft_chain.

Still present in:

  linux-6.12.y  (6.12.111)  nft_chain_filter.c: __nft_release_basechain()
  linux-6.6.y   (6.6.157)
  linux-6.1.y   (6.1.188)

Gone in linux-6.18.y and linux-7.0.y.

Reproducer (nftables 1.0.9)
===========================

An anonymous jump is what sets NFT_CHAIN_BINDING (chain flags 0x4).
Commit the base chain first, then add the jump in a second command.
Chains are appended to table->chains, and the walker frees the chain
that follows the base chain. One nft -f that contains both creates the
binding chain first and does not hit this path.

  ip link add br0 type bridge
  ip link set br0 up
  nft -f - <<'EOF'
  table netdev t {
          chain in {
                  type filter hook ingress device "br0" priority 0; policy accept;
          }
  }
  EOF
  nft add rule netdev t in 'jump { accept; }'
  ip link del br0

The namespace must still be alive. Netns teardown is too late: nft
pernet exit wins that race. Needs CAP_NET_ADMIN. Do not run on a host
kernel.

Confirmed on Linux 6.6.144 KASAN (same __nft_release_basechain() walk
as 6.6.y and 6.12.y). I have not booted 6.12.111 or 6.1.188.

  BUG: KASAN: slab-use-after-free in nf_tables_netdev_event+0x635/0x970
  Read of size 8 at addr ff110000037faa20 by task ip/229
  Freed by task 229:
    nft_immediate_destroy
    __nft_release_basechain_now
    __nft_release_basechain
    nf_tables_netdev_event

The read is 8 bytes at offset 32 of a kmalloc-128 object
(nft_chain.list).

Thanks,
Yu Junzhe
FuzzAnything <fuzzanything@gmail.com>

Re: [PATCH request stable 6.12 6.6 6.1] netfilter: nf_tables_netdev_event UAF of binding chain

[Greg Kroah-Hartman]

On Fri, Sep 25, 2026 at 02:00:25AM +0000, Yu Junzhe wrote:
> Hello,
> 
> Please consider a backport of the following use-after-free to the
> longterm trees that still have it: 6.12, 6.6, and 6.1. 6.18 and newer
> are not affected.
> 
> There is no single upstream commit to cherry-pick. On current linux.git
> the netdev notifier no longer calls __nft_release_basechain(); it only
> unregisters the per-device hook. That change arrived as part of the
> netdev-hook rework and does not apply to these stable trees.

Great, please provide a working, and tested, patch and we will glad to
review it and apply it if it passes review.

Or you can just submit the same upstream patches backported, which is
probably easier.

thanks,

greg k-h

Re: [PATCH request stable 6.12 6.6 6.1] netfilter: nf_tables_netdev_event UAF of binding chain

[Yu Junzhe]

Hello Greg,

The two upstream commits cherry-pick cleanly. They are Phil Sutter's
"Tolerate chains with no remaining hooks" (fc0133428e7a) and "Simplify
chain netdev notifier" (375f222800bc). The second is the follow-up that
makes list_for_each_entry() safe once the notifier no longer deletes the
base chain.

The 6.12 diff does not apply to 6.6 or 6.1: those trees still check
NETDEV_CHANGENAME in nf_tables_netdev_event(). So this is two series,
not one:

  [PATCH 6.12 1/2] and [PATCH 6.12 2/2]
  [PATCH 6.6 6.1 1/2] and [PATCH 6.6 6.1 2/2]

The 6.6 series applies cleanly on linux-6.1.y as well. Neither series
touches the rest of the netdev-hook rework.

Tested on KASAN guests, with the anonymous-jump ruleset from the
original report (base chain committed, then `nft add rule ... 'jump
{ accept; }'`, then `ip link del br0`):

  6.12.111  no slab-use-after-free in nf_tables_netdev_event
  6.6.157   no slab-use-after-free in nf_tables_netdev_event
  6.1.188   no slab-use-after-free in nf_tables_netdev_event

The same ruleset hits that KASAN bug on unpatched 6.6.144.

Thanks,
Yu Junzhe

[PATCH 6.12 1/2] netfilter: nf_tables: Tolerate chains with no remaining hooks

[Yu Junzhe]

From: Phil Sutter <phil@nwl.cc>

commit fc0133428e7ad65aa6b7c8e65ccfe86e469e4512 upstream.

Do not drop a netdev-family chain if the last interface it is registered
for vanishes. Users dumping and storing the ruleset upon shutdown to
restore it upon next boot may otherwise lose the chain and all contained
rules. They will still lose the list of devices, a later patch will fix
that. For now, this aligns the event handler's behaviour with that for
flowtables.
The controversal situation at netns exit should be no problem here:
event handler will unregister the hooks, core nftables cleanup code will
drop the chain itself.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Yu Junzhe <junzheyu1@gmail.com>
---
 include/net/netfilter/nf_tables.h |  2 --
 net/netfilter/nf_tables_api.c     | 41 -------------------------------
 net/netfilter/nft_chain_filter.c  | 29 ++++++----------------
 3 files changed, 7 insertions(+), 65 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index f4b59915a..923b2c0d6 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1238,8 +1238,6 @@ static inline bool nft_is_base_chain(const struct nft_chain *chain)
 	return chain->flags & NFT_CHAIN_BASE;
 }
 
-int __nft_release_basechain(struct nft_ctx *ctx);
-
 unsigned int nft_do_chain(struct nft_pktinfo *pkt, void *priv);
 
 static inline bool nft_use_inc(u32 *use)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 2613ebfdc..af9df95dd 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -11362,47 +11362,6 @@ int nft_data_dump(struct sk_buff *skb, int attr, const struct nft_data *data,
 }
 EXPORT_SYMBOL_GPL(nft_data_dump);
 
-static void __nft_release_basechain_now(struct nft_ctx *ctx)
-{
-	struct nft_rule *rule, *nr;
-
-	list_for_each_entry_safe(rule, nr, &ctx->chain->rules, list) {
-		list_del(&rule->list);
-		nf_tables_rule_release(ctx, rule);
-	}
-	nf_tables_chain_destroy(ctx->chain);
-}
-
-int __nft_release_basechain(struct nft_ctx *ctx)
-{
-	struct nft_rule *rule;
-
-	if (WARN_ON_ONCE(!nft_is_base_chain(ctx->chain)))
-		return 0;
-
-	nf_tables_unregister_hook(ctx->net, ctx->chain->table, ctx->chain);
-	list_for_each_entry(rule, &ctx->chain->rules, list)
-		nft_use_dec(&ctx->chain->use);
-
-	nft_chain_del(ctx->chain);
-	nft_use_dec(&ctx->table->use);
-
-	if (!maybe_get_net(ctx->net)) {
-		__nft_release_basechain_now(ctx);
-		return 0;
-	}
-
-	/* wait for ruleset dumps to complete.  Owning chain is no longer in
-	 * lists, so new dumps can't find any of these rules anymore.
-	 */
-	synchronize_rcu();
-
-	__nft_release_basechain_now(ctx);
-	put_net(ctx->net);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(__nft_release_basechain);
-
 static void __nft_release_hook(struct net *net, struct nft_table *table)
 {
 	struct nft_flowtable *flowtable;
diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
index 7010541fc..543f258b7 100644
--- a/net/netfilter/nft_chain_filter.c
+++ b/net/netfilter/nft_chain_filter.c
@@ -322,34 +322,19 @@ static void nft_netdev_event(unsigned long event, struct net_device *dev,
 			     struct nft_ctx *ctx)
 {
 	struct nft_base_chain *basechain = nft_base_chain(ctx->chain);
-	struct nft_hook *hook, *found = NULL;
-	int n = 0;
+	struct nft_hook *hook;
 
 	list_for_each_entry(hook, &basechain->hook_list, list) {
-		if (hook->ops.dev == dev)
-			found = hook;
-
-		n++;
-	}
-	if (!found)
-		return;
+		if (hook->ops.dev != dev)
+			continue;
 
-	if (n > 1) {
 		if (!(ctx->chain->table->flags & NFT_TABLE_F_DORMANT))
-			nf_unregister_net_hook(ctx->net, &found->ops);
+			nf_unregister_net_hook(ctx->net, &hook->ops);
 
-		list_del_rcu(&found->list);
-		kfree_rcu(found, rcu);
-		return;
+		list_del_rcu(&hook->list);
+		kfree_rcu(hook, rcu);
+		break;
 	}
-
-	/* UNREGISTER events are also happening on netns exit.
-	 *
-	 * Although nf_tables core releases all tables/chains, only this event
-	 * handler provides guarantee that hook->ops.dev is still accessible,
-	 * so we cannot skip exiting net namespaces.
-	 */
-	__nft_release_basechain(ctx);
 }
 
 static int nf_tables_netdev_event(struct notifier_block *this,
-- 
2.53.0

[PATCH 6.12 2/2] netfilter: nf_tables: Simplify chain netdev notifier

[Yu Junzhe]

From: Phil Sutter <phil@nwl.cc>

commit 375f222800bc001bb9cbd2baa1daec006430aeba upstream.

With conditional chain deletion gone, callback code simplifies: Instead
of filling an nft_ctx object, just pass basechain to the per-chain
function. Also plain list_for_each_entry() is safe now.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Yu Junzhe <junzheyu1@gmail.com>
---
 net/netfilter/nft_chain_filter.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
index 543f258b7..19a553550 100644
--- a/net/netfilter/nft_chain_filter.c
+++ b/net/netfilter/nft_chain_filter.c
@@ -319,17 +319,16 @@ static const struct nft_chain_type nft_chain_filter_netdev = {
 };
 
 static void nft_netdev_event(unsigned long event, struct net_device *dev,
-			     struct nft_ctx *ctx)
+			     struct nft_base_chain *basechain)
 {
-	struct nft_base_chain *basechain = nft_base_chain(ctx->chain);
 	struct nft_hook *hook;
 
 	list_for_each_entry(hook, &basechain->hook_list, list) {
 		if (hook->ops.dev != dev)
 			continue;
 
-		if (!(ctx->chain->table->flags & NFT_TABLE_F_DORMANT))
-			nf_unregister_net_hook(ctx->net, &hook->ops);
+		if (!(basechain->chain.table->flags & NFT_TABLE_F_DORMANT))
+			nf_unregister_net_hook(dev_net(dev), &hook->ops);
 
 		list_del_rcu(&hook->list);
 		kfree_rcu(hook, rcu);
@@ -343,25 +342,20 @@ static int nf_tables_netdev_event(struct notifier_block *this,
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
 	struct nft_base_chain *basechain;
 	struct nftables_pernet *nft_net;
-	struct nft_chain *chain, *nr;
+	struct nft_chain *chain;
 	struct nft_table *table;
-	struct nft_ctx ctx = {
-		.net	= dev_net(dev),
-	};
 
 	if (event != NETDEV_UNREGISTER)
 		return NOTIFY_DONE;
 
-	nft_net = nft_pernet(ctx.net);
+	nft_net = nft_pernet(dev_net(dev));
 	mutex_lock(&nft_net->commit_mutex);
 	list_for_each_entry(table, &nft_net->tables, list) {
 		if (table->family != NFPROTO_NETDEV &&
 		    table->family != NFPROTO_INET)
 			continue;
 
-		ctx.family = table->family;
-		ctx.table = table;
-		list_for_each_entry_safe(chain, nr, &table->chains, list) {
+		list_for_each_entry(chain, &table->chains, list) {
 			if (!nft_is_base_chain(chain))
 				continue;
 
@@ -370,8 +364,7 @@ static int nf_tables_netdev_event(struct notifier_block *this,
 			    basechain->ops.hooknum != NF_INET_INGRESS)
 				continue;
 
-			ctx.chain = chain;
-			nft_netdev_event(event, dev, &ctx);
+			nft_netdev_event(event, dev, basechain);
 		}
 	}
 	mutex_unlock(&nft_net->commit_mutex);
-- 
2.53.0

[PATCH 6.6 6.1 1/2] netfilter: nf_tables: Tolerate chains with no remaining hooks

[Yu Junzhe]

From: Phil Sutter <phil@nwl.cc>

commit fc0133428e7ad65aa6b7c8e65ccfe86e469e4512 upstream.

Do not drop a netdev-family chain if the last interface it is registered
for vanishes. Users dumping and storing the ruleset upon shutdown to
restore it upon next boot may otherwise lose the chain and all contained
rules. They will still lose the list of devices, a later patch will fix
that. For now, this aligns the event handler's behaviour with that for
flowtables.
The controversal situation at netns exit should be no problem here:
event handler will unregister the hooks, core nftables cleanup code will
drop the chain itself.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Yu Junzhe <junzheyu1@gmail.com>
---
 include/net/netfilter/nf_tables.h |  2 --
 net/netfilter/nf_tables_api.c     | 41 -------------------------------
 net/netfilter/nft_chain_filter.c  | 29 ++++++----------------
 3 files changed, 7 insertions(+), 65 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 4056d2272..515031702 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1233,8 +1233,6 @@ static inline bool nft_is_base_chain(const struct nft_chain *chain)
 	return chain->flags & NFT_CHAIN_BASE;
 }
 
-int __nft_release_basechain(struct nft_ctx *ctx);
-
 unsigned int nft_do_chain(struct nft_pktinfo *pkt, void *priv);
 
 static inline bool nft_use_inc(u32 *use)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index a9053667e..81d67a4ae 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -11267,47 +11267,6 @@ int nft_data_dump(struct sk_buff *skb, int attr, const struct nft_data *data,
 }
 EXPORT_SYMBOL_GPL(nft_data_dump);
 
-static void __nft_release_basechain_now(struct nft_ctx *ctx)
-{
-	struct nft_rule *rule, *nr;
-
-	list_for_each_entry_safe(rule, nr, &ctx->chain->rules, list) {
-		list_del(&rule->list);
-		nf_tables_rule_release(ctx, rule);
-	}
-	nf_tables_chain_destroy(ctx->chain);
-}
-
-int __nft_release_basechain(struct nft_ctx *ctx)
-{
-	struct nft_rule *rule;
-
-	if (WARN_ON_ONCE(!nft_is_base_chain(ctx->chain)))
-		return 0;
-
-	nf_tables_unregister_hook(ctx->net, ctx->chain->table, ctx->chain);
-	list_for_each_entry(rule, &ctx->chain->rules, list)
-		nft_use_dec(&ctx->chain->use);
-
-	nft_chain_del(ctx->chain);
-	nft_use_dec(&ctx->table->use);
-
-	if (!maybe_get_net(ctx->net)) {
-		__nft_release_basechain_now(ctx);
-		return 0;
-	}
-
-	/* wait for ruleset dumps to complete.  Owning chain is no longer in
-	 * lists, so new dumps can't find any of these rules anymore.
-	 */
-	synchronize_rcu();
-
-	__nft_release_basechain_now(ctx);
-	put_net(ctx->net);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(__nft_release_basechain);
-
 static void __nft_release_hook(struct net *net, struct nft_table *table)
 {
 	struct nft_flowtable *flowtable;
diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
index d170758a1..e48de5a2b 100644
--- a/net/netfilter/nft_chain_filter.c
+++ b/net/netfilter/nft_chain_filter.c
@@ -322,37 +322,22 @@ static void nft_netdev_event(unsigned long event, struct net_device *dev,
 			     struct nft_ctx *ctx)
 {
 	struct nft_base_chain *basechain = nft_base_chain(ctx->chain);
-	struct nft_hook *hook, *found = NULL;
-	int n = 0;
+	struct nft_hook *hook;
 
 	if (event != NETDEV_UNREGISTER)
 		return;
 
 	list_for_each_entry(hook, &basechain->hook_list, list) {
-		if (hook->ops.dev == dev)
-			found = hook;
-
-		n++;
-	}
-	if (!found)
-		return;
+		if (hook->ops.dev != dev)
+			continue;
 
-	if (n > 1) {
 		if (!(ctx->chain->table->flags & NFT_TABLE_F_DORMANT))
-			nf_unregister_net_hook(ctx->net, &found->ops);
+			nf_unregister_net_hook(ctx->net, &hook->ops);
 
-		list_del_rcu(&found->list);
-		kfree_rcu(found, rcu);
-		return;
+		list_del_rcu(&hook->list);
+		kfree_rcu(hook, rcu);
+		break;
 	}
-
-	/* UNREGISTER events are also happening on netns exit.
-	 *
-	 * Although nf_tables core releases all tables/chains, only this event
-	 * handler provides guarantee that hook->ops.dev is still accessible,
-	 * so we cannot skip exiting net namespaces.
-	 */
-	__nft_release_basechain(ctx);
 }
 
 static int nf_tables_netdev_event(struct notifier_block *this,
-- 
2.53.0

Re: [PATCH 6.6 6.1 1/2] netfilter: nf_tables: Tolerate chains with no remaining hooks

[Sasha Levin]

> commit fc0133428e7ad65aa6b7c8e65ccfe86e469e4512 upstream.
>
> Do not drop a netdev-family chain if the last interface it is registered
> for vanishes.

Queued the 6.12 and 6.6 series, thanks.

I'm not taking this for 6.1 though. It leaves a netdev chain with no
devices behind, and 6.1 can't handle that state because it lacks

  207296f1a03b ("netfilter: nf_tables: allow to create netdev chain without device")
  b9703ed44ffb ("netfilter: nf_tables: support for adding new devices to an existing netdev chain")

On 6.1 a non-flushing "nft -f" that re-adds the chain once the device is
back fails with EEXIST, and a ruleset dump that contains the orphaned
chain can't be restored.

For 6.1, could you either do a minimal fix for the nf_tables_netdev_event()
walker that Pablo or Florian are happy with, or get their OK to carry this
behaviour change there?

-- 
Thanks,
Sasha

[PATCH 6.1] netfilter: nf_tables: fix UAF in nf_tables_netdev_event walker

[Ma Xinmeng]

This is a 6.1-only fix. Mainline already fixed this bug in fc0133428e7a
("netfilter: nf_tables: Tolerate chains with no remaining hooks"), which
6.1 cannot take because it lacks 207296f1a03b ("netfilter: nf_tables:
allow to create netdev chain without device") and b9703ed44ffb
("netfilter: nf_tables: support for adding new devices to an existing
netdev chain"). So 6.1 keeps dropping the chain on the last
NETDEV_UNREGISTER, and this patch only makes the walker safe.

nf_tables_netdev_event() walks table->chains with
list_for_each_entry_safe(). On the last NETDEV_UNREGISTER for a base
chain, nft_netdev_event() calls __nft_release_basechain(), which removes
that base chain (nft_chain_del()) and destroys its rules. A JUMP/GOTO rule
targeting an NFT_CHAIN_BINDING chain deactivates and frees that successor
(nft_immediate_destroy() -> nf_tables_chain_destroy()), so the walker's
saved "nr" iterator can point at freed memory, triggering a
slab-use-after-free.

Fix it by restarting the table->chains walk whenever nft_netdev_event()
released a chain. __nft_release_basechain() removes the base chain from
the list before returning, so a restart cannot revisit the freed chain and
the walk terminates.

Signed-off-by: Ma Xinmeng <1564938642@qq.com>
---
 net/netfilter/nft_chain_filter.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
index d170758..ca6450a 100644
--- a/net/netfilter/nft_chain_filter.c
+++ b/net/netfilter/nft_chain_filter.c
@@ -318,7 +318,7 @@ static const struct nft_chain_type nft_chain_filter_netdev = {
 	},
 };
 
-static void nft_netdev_event(unsigned long event, struct net_device *dev,
+static bool nft_netdev_event(unsigned long event, struct net_device *dev,
 			     struct nft_ctx *ctx)
 {
 	struct nft_base_chain *basechain = nft_base_chain(ctx->chain);
@@ -326,7 +326,7 @@ static void nft_netdev_event(unsigned long event, struct net_device *dev,
 	int n = 0;
 
 	if (event != NETDEV_UNREGISTER)
-		return;
+		return false;
 
 	list_for_each_entry(hook, &basechain->hook_list, list) {
 		if (hook->ops.dev == dev)
@@ -335,7 +335,7 @@ static void nft_netdev_event(unsigned long event, struct net_device *dev,
 		n++;
 	}
 	if (!found)
-		return;
+		return false;
 
 	if (n > 1) {
 		if (!(ctx->chain->table->flags & NFT_TABLE_F_DORMANT))
@@ -343,7 +343,7 @@ static void nft_netdev_event(unsigned long event, struct net_device *dev,
 
 		list_del_rcu(&found->list);
 		kfree_rcu(found, rcu);
-		return;
+		return false;
 	}
 
 	/* UNREGISTER events are also happening on netns exit.
@@ -353,6 +353,8 @@ static void nft_netdev_event(unsigned long event, struct net_device *dev,
 	 * so we cannot skip exiting net namespaces.
 	 */
 	__nft_release_basechain(ctx);
+
+	return true;
 }
 
 static int nf_tables_netdev_event(struct notifier_block *this,
@@ -380,6 +382,7 @@ static int nf_tables_netdev_event(struct notifier_block *this,
 
 		ctx.family = table->family;
 		ctx.table = table;
+restart:
 		list_for_each_entry_safe(chain, nr, &table->chains, list) {
 			if (!nft_is_base_chain(chain))
 				continue;
@@ -390,7 +393,8 @@ static int nf_tables_netdev_event(struct notifier_block *this,
 				continue;
 
 			ctx.chain = chain;
-			nft_netdev_event(event, dev, &ctx);
+			if (nft_netdev_event(event, dev, &ctx))
+				goto restart;
 		}
 	}
 	mutex_unlock(&nft_net->commit_mutex);
-- 
2.49.0.windows.1

Re: [PATCH 6.1] netfilter: nf_tables: fix UAF in nf_tables_netdev_event walker

[Pablo Neira Ayuso]

On Thu, Oct 01, 2026 at 11:51:30AM +0800, Ma Xinmeng wrote:
> This is a 6.1-only fix. Mainline already fixed this bug in fc0133428e7a
> ("netfilter: nf_tables: Tolerate chains with no remaining hooks"), which
> 6.1 cannot take because it lacks 207296f1a03b ("netfilter: nf_tables:
> allow to create netdev chain without device") and b9703ed44ffb
> ("netfilter: nf_tables: support for adding new devices to an existing
> netdev chain").

Then, why not add those patches you refer to as -stable dependencies?
-stable trees will become hard to maintain if they start deviating too
much from upstream.

> So 6.1 keeps dropping the chain on the last
> NETDEV_UNREGISTER, and this patch only makes the walker safe.
> 
> nf_tables_netdev_event() walks table->chains with
> list_for_each_entry_safe(). On the last NETDEV_UNREGISTER for a base
> chain, nft_netdev_event() calls __nft_release_basechain(), which removes
> that base chain (nft_chain_del()) and destroys its rules. A JUMP/GOTO rule
> targeting an NFT_CHAIN_BINDING chain deactivates and frees that successor
> (nft_immediate_destroy() -> nf_tables_chain_destroy()), so the walker's
> saved "nr" iterator can point at freed memory, triggering a
> slab-use-after-free.
> 
> Fix it by restarting the table->chains walk whenever nft_netdev_event()
> released a chain. __nft_release_basechain() removes the base chain from
> the list before returning, so a restart cannot revisit the freed chain and
> the walk terminates.
> 
> Signed-off-by: Ma Xinmeng <1564938642@qq.com>
> ---
>  net/netfilter/nft_chain_filter.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
> index d170758..ca6450a 100644
> --- a/net/netfilter/nft_chain_filter.c
> +++ b/net/netfilter/nft_chain_filter.c
> @@ -318,7 +318,7 @@ static const struct nft_chain_type nft_chain_filter_netdev = {
>  	},
>  };
>  
> -static void nft_netdev_event(unsigned long event, struct net_device *dev,
> +static bool nft_netdev_event(unsigned long event, struct net_device *dev,
>  			     struct nft_ctx *ctx)
>  {
>  	struct nft_base_chain *basechain = nft_base_chain(ctx->chain);
> @@ -326,7 +326,7 @@ static void nft_netdev_event(unsigned long event, struct net_device *dev,
>  	int n = 0;
>  
>  	if (event != NETDEV_UNREGISTER)
> -		return;
> +		return false;
>  
>  	list_for_each_entry(hook, &basechain->hook_list, list) {
>  		if (hook->ops.dev == dev)
> @@ -335,7 +335,7 @@ static void nft_netdev_event(unsigned long event, struct net_device *dev,
>  		n++;
>  	}
>  	if (!found)
> -		return;
> +		return false;
>  
>  	if (n > 1) {
>  		if (!(ctx->chain->table->flags & NFT_TABLE_F_DORMANT))
> @@ -343,7 +343,7 @@ static void nft_netdev_event(unsigned long event, struct net_device *dev,
>  
>  		list_del_rcu(&found->list);
>  		kfree_rcu(found, rcu);
> -		return;
> +		return false;
>  	}
>  
>  	/* UNREGISTER events are also happening on netns exit.
> @@ -353,6 +353,8 @@ static void nft_netdev_event(unsigned long event, struct net_device *dev,
>  	 * so we cannot skip exiting net namespaces.
>  	 */
>  	__nft_release_basechain(ctx);
> +
> +	return true;
>  }
>  
>  static int nf_tables_netdev_event(struct notifier_block *this,
> @@ -380,6 +382,7 @@ static int nf_tables_netdev_event(struct notifier_block *this,
>  
>  		ctx.family = table->family;
>  		ctx.table = table;
> +restart:
>  		list_for_each_entry_safe(chain, nr, &table->chains, list) {
>  			if (!nft_is_base_chain(chain))
>  				continue;
> @@ -390,7 +393,8 @@ static int nf_tables_netdev_event(struct notifier_block *this,
>  				continue;
>  
>  			ctx.chain = chain;
> -			nft_netdev_event(event, dev, &ctx);
> +			if (nft_netdev_event(event, dev, &ctx))
> +				goto restart;
>  		}
>  	}
>  	mutex_unlock(&nft_net->commit_mutex);
> -- 
> 2.49.0.windows.1
> 

[PATCH 6.1 1/2] netfilter: nf_tables: allow to create netdev chain without device

[Ma Xinmeng]

From: Pablo Neira Ayuso <pablo@netfilter.org>

Relax netdev chain creation to allow for loading the ruleset, then
adding/deleting devices at a later stage. Hardware offload does not
support for this feature yet.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Ma Xinmeng <1564938642@qq.com>

This is a 6.1 backport of upstream commit 207296f1a03b; the hunk for
nft_delchain_hook() is dropped since that function is not present in 6.1.
---
 net/netfilter/nf_tables_api.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 96b0638c2..18aa388d5 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2180,7 +2180,7 @@ struct nft_chain_hook {
 
 static int nft_chain_parse_netdev(struct net *net,
 				  struct nlattr *tb[],
-				  struct list_head *hook_list)
+				  struct list_head *hook_list, u32 flags)
 {
 	struct nft_hook *hook;
 	int err;
@@ -2197,19 +2197,20 @@ static int nft_chain_parse_netdev(struct net *net,
 		if (err < 0)
 			return err;
 
-		if (list_empty(hook_list))
-			return -EINVAL;
-	} else {
-		return -EINVAL;
 	}
 
+	if (flags & NFT_CHAIN_HW_OFFLOAD &&
+	    list_empty(hook_list))
+		return -EINVAL;
+
 	return 0;
 }
 
 static int nft_chain_parse_hook(struct net *net,
 				const struct nlattr * const nla[],
 				struct nft_chain_hook *hook, u8 family,
-				struct netlink_ext_ack *extack, bool autoload)
+				struct netlink_ext_ack *extack, bool autoload,
+				u32 flags)
 {
 	struct nftables_pernet *nft_net = nft_pernet(net);
 	struct nlattr *ha[NFTA_HOOK_MAX + 1];
@@ -2261,7 +2262,7 @@ static int nft_chain_parse_hook(struct net *net,
 
 	INIT_LIST_HEAD(&hook->list);
 	if (nft_base_chain_netdev(family, hook->num)) {
-		err = nft_chain_parse_netdev(net, ha, &hook->list);
+		err = nft_chain_parse_netdev(net, ha, &hook->list, flags);
 		if (err < 0) {
 			module_put(type->owner);
 			return err;
@@ -2409,7 +2410,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
 			return -EOPNOTSUPP;
 
 		err = nft_chain_parse_hook(net, nla, &hook, family, extack,
-					   true);
+					   true, flags);
 		if (err < 0)
 			return err;
 
@@ -2574,7 +2575,7 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 			return -EEXIST;
 		}
 		err = nft_chain_parse_hook(ctx->net, nla, &hook, ctx->family,
-					   extack, false);
+					   extack, false, flags);
 		if (err < 0)
 			return err;
 
@@ -2795,6 +2796,7 @@ static int nf_tables_newchain(struct sk_buff *skb, const struct nfnl_info *info,
 	return nf_tables_addchain(&ctx, family, genmask, policy, flags, extack);
 }
 
+
 static int nf_tables_delchain(struct sk_buff *skb, const struct nfnl_info *info,
 			      const struct nlattr * const nla[])
 {
-- 
2.43.0

Re: [PATCH 6.1 1/2] netfilter: nf_tables: allow to create netdev chain without device

[Sasha Levin]

> This is a 6.1 backport of upstream commit 207296f1a03b; the hunk for
> nft_delchain_hook() is dropped since that function is not present in 6.1.

This series only brings in the prerequisites. The UAF fix itself isn't
in it:

  fc0133428e7a ("netfilter: nf_tables: Tolerate chains with no remaining hooks")
  375f222800bc ("netfilter: nf_tables: Simplify chain netdev notifier")

2/2 also adds b9703ed44ffb ("netfilter: nf_tables: support for adding
new devices to an existing netdev chain") to 6.1 without the later
fixes for it, none of which 6.1 has:

  043d2acf5722 ("netfilter: nf_tables: drop module reference after updating chain")
  7eaf837a4eb5 ("netfilter: nf_tables: Fix a memory leak in nf_tables_updchain")
  1e1fb6f00f52 ("netfilter: nf_tables: reject table flag and netdev basechain updates")
  216e7bf7402c ("netfilter: nf_tables: skip netdev hook unregistration if table is dormant")
  688c15017d5c ("netfilter: nf_tables: don't unregister hook when table is dormant")
  4ffcf5ca81c3 ("netfilter: nf_tables: use rcu chain hook list iterator from netlink dump path")

It also needs the netdev chain parts of a6134e62dba2 ("netfilter:
nf_tables: join hook list via splice_list_rcu() in commit phase") and
cf5fb87fcdaa ("netfilter: nf_tables: reject duplicate device on
updates"); 6.1 only has their flowtable parts. Without 043d2acf5722,
every base chain update that carries a hook leaks a reference on the
chain type module.

Could you send a single 6.1 series in upstream order: the two
prerequisites, the fixes above, then fc0133428e7a and 375f222800bc?
Please start each patch with its "commit <sha> upstream." line and add
a cover letter. An ack from Pablo or Florian would help too.

5.15 and 5.10 have the same UAF. Are you planning to cover them as well?

-- 
Thanks,
Sasha

Re: [PATCH 6.1 1/2] netfilter: nf_tables: allow to create netdev chain without device

[Pablo Neira Ayuso]

Hi,

On Fri, Oct 02, 2026 at 05:10:34PM -0400, Sasha Levin wrote:
> > This is a 6.1 backport of upstream commit 207296f1a03b; the hunk for
> > nft_delchain_hook() is dropped since that function is not present in 6.1.
> 
> This series only brings in the prerequisites. The UAF fix itself isn't
> in it:
> 
>   fc0133428e7a ("netfilter: nf_tables: Tolerate chains with no remaining hooks")
>   375f222800bc ("netfilter: nf_tables: Simplify chain netdev notifier")
> 
> 2/2 also adds b9703ed44ffb ("netfilter: nf_tables: support for adding
> new devices to an existing netdev chain") to 6.1 without the later
> fixes for it, none of which 6.1 has:
> 
>   043d2acf5722 ("netfilter: nf_tables: drop module reference after updating chain")
>   7eaf837a4eb5 ("netfilter: nf_tables: Fix a memory leak in nf_tables_updchain")
>   1e1fb6f00f52 ("netfilter: nf_tables: reject table flag and netdev basechain updates")
>   216e7bf7402c ("netfilter: nf_tables: skip netdev hook unregistration if table is dormant")
>   688c15017d5c ("netfilter: nf_tables: don't unregister hook when table is dormant")
>   4ffcf5ca81c3 ("netfilter: nf_tables: use rcu chain hook list iterator from netlink dump path")
> 
> It also needs the netdev chain parts of a6134e62dba2 ("netfilter:
> nf_tables: join hook list via splice_list_rcu() in commit phase") and
> cf5fb87fcdaa ("netfilter: nf_tables: reject duplicate device on
> updates"); 6.1 only has their flowtable parts. Without 043d2acf5722,
> every base chain update that carries a hook leaks a reference on the
> chain type module.
> 
> Could you send a single 6.1 series in upstream order: the two
> prerequisites, the fixes above, then fc0133428e7a and 375f222800bc?
> Please start each patch with its "commit <sha> upstream." line and add
> a cover letter. An ack from Pablo or Florian would help too.
> 
> 5.15 and 5.10 have the same UAF. Are you planning to cover them as well?

Yes, I am collecting backport patches and preparing a batch for you.

[PATCH 6.1 2/2] netfilter: nf_tables: support for adding new devices to an existing netdev chain

[Ma Xinmeng]

From: Pablo Neira Ayuso <pablo@netfilter.org>

This patch allows users to add devices to an existing netdev chain.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Ma Xinmeng <1564938642@qq.com>

This is a 6.1 backport of upstream commit b9703ed44ffb, adapted to 6.1's
nft_chain_parse_hook() and nf_tables_chain_destroy() signatures.
---
 include/net/netfilter/nf_tables.h |   6 +
 net/netfilter/nf_tables_api.c     | 213 +++++++++++++++++++-----------
 2 files changed, 140 insertions(+), 79 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 7e63281f2..e79d5f294 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1625,6 +1625,8 @@ struct nft_trans_chain {
 	u8				policy;
 	bool				bound;
 	u32				chain_id;
+	struct nft_base_chain		*basechain;
+	struct list_head		hook_list;
 };
 
 #define nft_trans_chain(trans)	\
@@ -1641,6 +1643,10 @@ struct nft_trans_chain {
 	(((struct nft_trans_chain *)trans->data)->bound)
 #define nft_trans_chain_id(trans)	\
 	(((struct nft_trans_chain *)trans->data)->chain_id)
+#define nft_trans_basechain(trans)	\
+	(((struct nft_trans_chain *)trans->data)->basechain)
+#define nft_trans_chain_hooks(trans)	\
+	(((struct nft_trans_chain *)trans->data)->hook_list)
 
 struct nft_trans_table {
 	bool				update;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 18aa388d5..df26de2eb 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1750,7 +1750,8 @@ static int nft_dump_stats(struct sk_buff *skb, struct nft_stats __percpu *stats)
 }
 
 static int nft_dump_basechain_hook(struct sk_buff *skb, int family,
-				   const struct nft_base_chain *basechain)
+				   const struct nft_base_chain *basechain,
+				   const struct list_head *hook_list)
 {
 	const struct nf_hook_ops *ops = &basechain->ops;
 	struct nft_hook *hook, *first = NULL;
@@ -1767,7 +1768,11 @@ static int nft_dump_basechain_hook(struct sk_buff *skb, int family,
 
 	if (nft_base_chain_netdev(family, ops->hooknum)) {
 		nest_devs = nla_nest_start_noflag(skb, NFTA_HOOK_DEVS);
-		list_for_each_entry(hook, &basechain->hook_list, list) {
+
+		if (!hook_list)
+			hook_list = &basechain->hook_list;
+
+		list_for_each_entry(hook, hook_list, list) {
 			if (!first)
 				first = hook;
 
@@ -1792,7 +1797,8 @@ static int nft_dump_basechain_hook(struct sk_buff *skb, int family,
 static int nf_tables_fill_chain_info(struct sk_buff *skb, struct net *net,
 				     u32 portid, u32 seq, int event, u32 flags,
 				     int family, const struct nft_table *table,
-				     const struct nft_chain *chain)
+				     const struct nft_chain *chain,
+				     const struct list_head *hook_list)
 {
 	struct nlmsghdr *nlh;
 
@@ -1814,7 +1820,7 @@ static int nf_tables_fill_chain_info(struct sk_buff *skb, struct net *net,
 		const struct nft_base_chain *basechain = nft_base_chain(chain);
 		struct nft_stats __percpu *stats;
 
-		if (nft_dump_basechain_hook(skb, family, basechain))
+		if (nft_dump_basechain_hook(skb, family, basechain, hook_list))
 			goto nla_put_failure;
 
 		if (nla_put_be32(skb, NFTA_CHAIN_POLICY,
@@ -1849,7 +1855,8 @@ static int nf_tables_fill_chain_info(struct sk_buff *skb, struct net *net,
 	return -1;
 }
 
-static void nf_tables_chain_notify(const struct nft_ctx *ctx, int event)
+static void nf_tables_chain_notify(const struct nft_ctx *ctx, int event,
+				   const struct list_head *hook_list)
 {
 	struct nftables_pernet *nft_net;
 	struct sk_buff *skb;
@@ -1869,7 +1876,7 @@ static void nf_tables_chain_notify(const struct nft_ctx *ctx, int event)
 
 	err = nf_tables_fill_chain_info(skb, ctx->net, ctx->portid, ctx->seq,
 					event, flags, ctx->family, ctx->table,
-					ctx->chain);
+					ctx->chain, hook_list);
 	if (err < 0) {
 		kfree_skb(skb);
 		goto err;
@@ -1915,7 +1922,7 @@ static int nf_tables_dump_chains(struct sk_buff *skb,
 						      NFT_MSG_NEWCHAIN,
 						      NLM_F_MULTI,
 						      table->family, table,
-						      chain) < 0)
+						      chain, NULL) < 0)
 				goto done;
 
 			nl_dump_check_consistent(cb, nlmsg_hdr(skb));
@@ -1969,7 +1976,7 @@ static int nf_tables_getchain(struct sk_buff *skb, const struct nfnl_info *info,
 
 	err = nf_tables_fill_chain_info(skb2, net, NETLINK_CB(skb).portid,
 					info->nlh->nlmsg_seq, NFT_MSG_NEWCHAIN,
-					0, family, table, chain);
+					0, family, table, chain, NULL);
 	if (err < 0)
 		goto err_fill_chain_info;
 
@@ -2207,6 +2214,7 @@ static int nft_chain_parse_netdev(struct net *net,
 }
 
 static int nft_chain_parse_hook(struct net *net,
+				struct nft_base_chain *basechain,
 				const struct nlattr * const nla[],
 				struct nft_chain_hook *hook, u8 family,
 				struct netlink_ext_ack *extack, bool autoload,
@@ -2226,31 +2234,46 @@ static int nft_chain_parse_hook(struct net *net,
 	if (err < 0)
 		return err;
 
-	if (ha[NFTA_HOOK_HOOKNUM] == NULL ||
-	    ha[NFTA_HOOK_PRIORITY] == NULL)
-		return -EINVAL;
+	if (!basechain) {
+		if (!ha[NFTA_HOOK_HOOKNUM] ||
+		    !ha[NFTA_HOOK_PRIORITY])
+			return -EINVAL;
 
-	hook->num = ntohl(nla_get_be32(ha[NFTA_HOOK_HOOKNUM]));
-	hook->priority = ntohl(nla_get_be32(ha[NFTA_HOOK_PRIORITY]));
+		hook->num = ntohl(nla_get_be32(ha[NFTA_HOOK_HOOKNUM]));
+		hook->priority = ntohl(nla_get_be32(ha[NFTA_HOOK_PRIORITY]));
 
-	type = __nft_chain_type_get(family, NFT_CHAIN_T_DEFAULT);
-	if (!type)
-		return -EOPNOTSUPP;
+		type = __nft_chain_type_get(family, NFT_CHAIN_T_DEFAULT);
+		if (!type)
+			return -EOPNOTSUPP;
 
-	if (nla[NFTA_CHAIN_TYPE]) {
-		type = nf_tables_chain_type_lookup(net, nla[NFTA_CHAIN_TYPE],
-						   family, autoload);
-		if (IS_ERR(type)) {
-			NL_SET_BAD_ATTR(extack, nla[NFTA_CHAIN_TYPE]);
-			return PTR_ERR(type);
+		if (nla[NFTA_CHAIN_TYPE]) {
+			type = nf_tables_chain_type_lookup(net, nla[NFTA_CHAIN_TYPE],
+							   family, autoload);
+			if (IS_ERR(type)) {
+				NL_SET_BAD_ATTR(extack, nla[NFTA_CHAIN_TYPE]);
+				return PTR_ERR(type);
+			}
 		}
-	}
-	if (hook->num >= NFT_MAX_HOOKS || !(type->hook_mask & (1 << hook->num)))
-		return -EOPNOTSUPP;
+		if (hook->num >= NFT_MAX_HOOKS || !(type->hook_mask & (1 << hook->num)))
+			return -EOPNOTSUPP;
 
-	if (type->type == NFT_CHAIN_T_NAT &&
-	    hook->priority <= NF_IP_PRI_CONNTRACK)
-		return -EOPNOTSUPP;
+		if (type->type == NFT_CHAIN_T_NAT &&
+		    hook->priority <= NF_IP_PRI_CONNTRACK)
+			return -EOPNOTSUPP;
+	} else {
+		if (ha[NFTA_HOOK_HOOKNUM]) {
+			hook->num = ntohl(nla_get_be32(ha[NFTA_HOOK_HOOKNUM]));
+			if (hook->num != basechain->ops.hooknum)
+				return -EOPNOTSUPP;
+		}
+		if (ha[NFTA_HOOK_PRIORITY]) {
+			hook->priority = ntohl(nla_get_be32(ha[NFTA_HOOK_PRIORITY]));
+			if (hook->priority != basechain->ops.priority)
+				return -EOPNOTSUPP;
+		}
+
+		type = basechain->type;
+	}
 
 	if (!try_module_get(type->owner)) {
 		if (nla[NFTA_CHAIN_TYPE])
@@ -2348,12 +2371,8 @@ static int nft_basechain_init(struct nft_base_chain *basechain, u8 family,
 		list_splice_init(&hook->list, &basechain->hook_list);
 		list_for_each_entry(h, &basechain->hook_list, list)
 			nft_basechain_hook_init(&h->ops, family, hook, chain);
-
-		basechain->ops.hooknum	= hook->num;
-		basechain->ops.priority	= hook->priority;
-	} else {
-		nft_basechain_hook_init(&basechain->ops, family, hook, chain);
 	}
+	nft_basechain_hook_init(&basechain->ops, family, hook, chain);
 
 	chain->flags |= NFT_CHAIN_BASE | flags;
 	basechain->policy = NF_ACCEPT;
@@ -2401,7 +2420,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
 
 	if (nla[NFTA_CHAIN_HOOK]) {
 		struct nft_stats __percpu *stats = NULL;
-		struct nft_chain_hook hook;
+		struct nft_chain_hook hook = {};
 
 		if (table->flags & __NFT_TABLE_F_UPDATE)
 			return -EINVAL;
@@ -2409,7 +2428,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
 		if (flags & NFT_CHAIN_BINDING)
 			return -EOPNOTSUPP;
 
-		err = nft_chain_parse_hook(net, nla, &hook, family, extack,
+		err = nft_chain_parse_hook(net, NULL, nla, &hook, family, extack,
 					   true, flags);
 		if (err < 0)
 			return err;
@@ -2533,65 +2552,57 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
 	return err;
 }
 
-static bool nft_hook_list_equal(struct list_head *hook_list1,
-				struct list_head *hook_list2)
-{
-	struct nft_hook *hook;
-	int n = 0, m = 0;
-
-	n = 0;
-	list_for_each_entry(hook, hook_list2, list) {
-		if (!nft_hook_list_find(hook_list1, hook))
-			return false;
-
-		n++;
-	}
-	list_for_each_entry(hook, hook_list1, list)
-		m++;
-
-	return n == m;
-}
-
 static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 			      u32 flags, const struct nlattr *attr,
 			      struct netlink_ext_ack *extack)
 {
 	const struct nlattr * const *nla = ctx->nla;
+	struct nft_base_chain *basechain = NULL;
 	struct nft_table *table = ctx->table;
 	struct nft_chain *chain = ctx->chain;
-	struct nft_base_chain *basechain;
+	struct nft_chain_hook hook = {};
 	struct nft_stats *stats = NULL;
-	struct nft_chain_hook hook;
+	struct nft_hook *h, *next;
 	struct nf_hook_ops *ops;
 	struct nft_trans *trans;
+	bool unregister = false;
 	int err;
 
 	if (chain->flags ^ flags)
 		return -EOPNOTSUPP;
 
+	INIT_LIST_HEAD(&hook.list);
+
 	if (nla[NFTA_CHAIN_HOOK]) {
 		if (!nft_is_base_chain(chain)) {
 			NL_SET_BAD_ATTR(extack, attr);
 			return -EEXIST;
 		}
-		err = nft_chain_parse_hook(ctx->net, nla, &hook, ctx->family,
-					   extack, false, flags);
+
+		basechain = nft_base_chain(chain);
+		err = nft_chain_parse_hook(ctx->net, basechain, nla, &hook,
+					   ctx->family, extack, false, flags);
 		if (err < 0)
 			return err;
 
-		basechain = nft_base_chain(chain);
 		if (basechain->type != hook.type) {
 			nft_chain_release_hook(&hook);
 			NL_SET_BAD_ATTR(extack, attr);
 			return -EEXIST;
 		}
 
-		if (nft_base_chain_netdev(ctx->family, hook.num)) {
-			if (!nft_hook_list_equal(&basechain->hook_list,
-						 &hook.list)) {
-				nft_chain_release_hook(&hook);
-				NL_SET_BAD_ATTR(extack, attr);
-				return -EEXIST;
+		if (nft_base_chain_netdev(ctx->family, basechain->ops.hooknum)) {
+			list_for_each_entry_safe(h, next, &hook.list, list) {
+				h->ops.pf	= basechain->ops.pf;
+				h->ops.hooknum	= basechain->ops.hooknum;
+				h->ops.priority	= basechain->ops.priority;
+				h->ops.priv	= basechain->ops.priv;
+				h->ops.hook	= basechain->ops.hook;
+
+				if (nft_hook_list_find(&basechain->hook_list, h)) {
+					list_del(&h->list);
+					kfree(h);
+				}
 			}
 		} else {
 			ops = &basechain->ops;
@@ -2602,7 +2613,6 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 				return -EEXIST;
 			}
 		}
-		nft_chain_release_hook(&hook);
 	}
 
 	if (nla[NFTA_CHAIN_HANDLE] &&
@@ -2613,24 +2623,43 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 					  nla[NFTA_CHAIN_NAME], genmask);
 		if (!IS_ERR(chain2)) {
 			NL_SET_BAD_ATTR(extack, nla[NFTA_CHAIN_NAME]);
-			return -EEXIST;
+			err = -EEXIST;
+			goto err_hooks;
 		}
 	}
 
 	if (nla[NFTA_CHAIN_COUNTERS]) {
-		if (!nft_is_base_chain(chain))
-			return -EOPNOTSUPP;
+		if (!nft_is_base_chain(chain)) {
+			err = -EOPNOTSUPP;
+			goto err_hooks;
+		}
 
 		stats = nft_stats_alloc(nla[NFTA_CHAIN_COUNTERS]);
-		if (IS_ERR(stats))
-			return PTR_ERR(stats);
+		if (IS_ERR(stats)) {
+			err = PTR_ERR(stats);
+			goto err_hooks;
+		}
 	}
 
+	if (!(table->flags & NFT_TABLE_F_DORMANT) &&
+	    nft_is_base_chain(chain) &&
+	    !list_empty(&hook.list)) {
+		basechain = nft_base_chain(chain);
+		ops = &basechain->ops;
+
+		if (nft_base_chain_netdev(table->family, basechain->ops.hooknum)) {
+			err = nft_netdev_register_hooks(ctx->net, &hook.list);
+			if (err < 0)
+				goto err_hooks;
+		}
+	}
+
+	unregister = true;
 	err = -ENOMEM;
 	trans = nft_trans_alloc(ctx, NFT_MSG_NEWCHAIN,
 				sizeof(struct nft_trans_chain));
 	if (trans == NULL)
-		goto err;
+		goto err_trans;
 
 	nft_trans_chain_stats(trans) = stats;
 	nft_trans_chain_update(trans) = true;
@@ -2649,7 +2678,7 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 		err = -ENOMEM;
 		name = nla_strdup(nla[NFTA_CHAIN_NAME], GFP_KERNEL_ACCOUNT);
 		if (!name)
-			goto err;
+			goto err_trans;
 
 		err = -EEXIST;
 		list_for_each_entry(tmp, &nft_net->commit_list, list) {
@@ -2660,18 +2689,35 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 			    strcmp(name, nft_trans_chain_name(tmp)) == 0) {
 				NL_SET_BAD_ATTR(extack, nla[NFTA_CHAIN_NAME]);
 				kfree(name);
-				goto err;
+				goto err_trans;
 			}
 		}
 
 		nft_trans_chain_name(trans) = name;
 	}
+
+	nft_trans_basechain(trans) = basechain;
+	INIT_LIST_HEAD(&nft_trans_chain_hooks(trans));
+	list_splice(&hook.list, &nft_trans_chain_hooks(trans));
+
 	nft_trans_commit_list_add_tail(ctx->net, trans);
 
 	return 0;
-err:
+
+err_trans:
 	free_percpu(stats);
 	kfree(trans);
+err_hooks:
+	if (nla[NFTA_CHAIN_HOOK]) {
+		list_for_each_entry_safe(h, next, &hook.list, list) {
+			if (unregister)
+				nf_unregister_net_hook(ctx->net, &h->ops);
+			list_del(&h->list);
+			kfree_rcu(h, rcu);
+		}
+		module_put(hook.type->owner);
+	}
+
 	return err;
 }
 
@@ -9876,18 +9922,21 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 		case NFT_MSG_NEWCHAIN:
 			if (nft_trans_chain_update(trans)) {
 				nft_chain_commit_update(trans);
-				nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN);
+				nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN,
+						       &nft_trans_chain_hooks(trans));
+				list_splice(&nft_trans_chain_hooks(trans),
+					    &nft_trans_basechain(trans)->hook_list);
 				/* trans destroyed after rcu grace period */
 			} else {
 				nft_chain_commit_drop_policy(trans);
 				nft_clear(net, trans->ctx.chain);
-				nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN);
+				nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN, NULL);
 				nft_trans_destroy(trans);
 			}
 			break;
 		case NFT_MSG_DELCHAIN:
 			nft_chain_del(trans->ctx.chain);
-			nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN);
+			nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN, NULL);
 			nf_tables_unregister_hook(trans->ctx.net,
 						  trans->ctx.table,
 						  trans->ctx.chain);
@@ -10066,7 +10115,10 @@ static void nf_tables_abort_release(struct nft_trans *trans)
 		nf_tables_table_destroy(&trans->ctx);
 		break;
 	case NFT_MSG_NEWCHAIN:
-		nf_tables_chain_destroy(nft_trans_chain(trans));
+		if (nft_trans_chain_update(trans))
+			nft_hooks_destroy(&nft_trans_chain_hooks(trans));
+		else
+			nf_tables_chain_destroy(nft_trans_chain(trans));
 		break;
 	case NFT_MSG_NEWRULE:
 		nf_tables_rule_destroy(&trans->ctx, nft_trans_rule(trans));
@@ -10144,6 +10196,9 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 			break;
 		case NFT_MSG_NEWCHAIN:
 			if (nft_trans_chain_update(trans)) {
+				nft_netdev_unregister_hooks(net,
+							    &nft_trans_chain_hooks(trans),
+							    true);
 				free_percpu(nft_trans_chain_stats(trans));
 				kfree(nft_trans_chain_name(trans));
 				nft_trans_destroy(trans);
-- 
2.43.0

[PATCH 6.6 6.1 2/2] netfilter: nf_tables: Simplify chain netdev notifier

[Yu Junzhe]

From: Phil Sutter <phil@nwl.cc>

commit 375f222800bc001bb9cbd2baa1daec006430aeba upstream.

With conditional chain deletion gone, callback code simplifies: Instead
of filling an nft_ctx object, just pass basechain to the per-chain
function. Also plain list_for_each_entry() is safe now.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Yu Junzhe <junzheyu1@gmail.com>
---
 net/netfilter/nft_chain_filter.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
index e48de5a2b..867ff860c 100644
--- a/net/netfilter/nft_chain_filter.c
+++ b/net/netfilter/nft_chain_filter.c
@@ -319,9 +319,8 @@ static const struct nft_chain_type nft_chain_filter_netdev = {
 };
 
 static void nft_netdev_event(unsigned long event, struct net_device *dev,
-			     struct nft_ctx *ctx)
+			     struct nft_base_chain *basechain)
 {
-	struct nft_base_chain *basechain = nft_base_chain(ctx->chain);
 	struct nft_hook *hook;
 
 	if (event != NETDEV_UNREGISTER)
@@ -331,8 +330,8 @@ static void nft_netdev_event(unsigned long event, struct net_device *dev,
 		if (hook->ops.dev != dev)
 			continue;
 
-		if (!(ctx->chain->table->flags & NFT_TABLE_F_DORMANT))
-			nf_unregister_net_hook(ctx->net, &hook->ops);
+		if (!(basechain->chain.table->flags & NFT_TABLE_F_DORMANT))
+			nf_unregister_net_hook(dev_net(dev), &hook->ops);
 
 		list_del_rcu(&hook->list);
 		kfree_rcu(hook, rcu);
@@ -346,26 +345,21 @@ static int nf_tables_netdev_event(struct notifier_block *this,
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
 	struct nft_base_chain *basechain;
 	struct nftables_pernet *nft_net;
-	struct nft_chain *chain, *nr;
+	struct nft_chain *chain;
 	struct nft_table *table;
-	struct nft_ctx ctx = {
-		.net	= dev_net(dev),
-	};
 
 	if (event != NETDEV_UNREGISTER &&
 	    event != NETDEV_CHANGENAME)
 		return NOTIFY_DONE;
 
-	nft_net = nft_pernet(ctx.net);
+	nft_net = nft_pernet(dev_net(dev));
 	mutex_lock(&nft_net->commit_mutex);
 	list_for_each_entry(table, &nft_net->tables, list) {
 		if (table->family != NFPROTO_NETDEV &&
 		    table->family != NFPROTO_INET)
 			continue;
 
-		ctx.family = table->family;
-		ctx.table = table;
-		list_for_each_entry_safe(chain, nr, &table->chains, list) {
+		list_for_each_entry(chain, &table->chains, list) {
 			if (!nft_is_base_chain(chain))
 				continue;
 
@@ -374,8 +368,7 @@ static int nf_tables_netdev_event(struct notifier_block *this,
 			    basechain->ops.hooknum != NF_INET_INGRESS)
 				continue;
 
-			ctx.chain = chain;
-			nft_netdev_event(event, dev, &ctx);
+			nft_netdev_event(event, dev, basechain);
 		}
 	}
 	mutex_unlock(&nft_net->commit_mutex);
-- 
2.53.0