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

[Pablo Neira Ayuso <pablo@netfilter.org>]

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
>