[PATCH nf v3] netfilter: nf_tables: fix device name and prefix match in hook lookup

[PATCH nf v3] netfilter: nf_tables: fix device name and prefix match in hook lookup

[Fernando Fernandez Mancera]

Currently, a netdev chain or flowtable hooked to a device prefix can be
unintentionally deleted by a control-plane request targeting an exact
device name or even a shorter one due to the usage of min() to calculate
the length to match.

Fix this by making sure an exact device match never matches a prefix and
that both the target and the candidate have the same length during
delete operation. The add and update paths retain the existing overlap
matching to prevent a single device from matching multiple hooks.

Reported-by: Wei Fang <void0red@gmail.com>
Closes: https://lore.kernel.org/netfilter-devel/CANE+tVrDeNCHQVmsqkV2ozeBqyE3GtRDMhZgsg1bhw10yGNTRQ@mail.gmail.com/
Fixes: 6d07a289504a ("netfilter: nf_tables: Support wildcard netdev hook specs")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
 net/netfilter/nf_tables_api.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c112ecc4fca3..9ef12feec1ac 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1978,7 +1978,7 @@ static int nft_dump_stats(struct sk_buff *skb, struct nft_stats __percpu *stats)
 	return -ENOSPC;
 }
 
-static bool hook_is_prefix(struct nft_hook *hook)
+static bool hook_is_prefix(const struct nft_hook *hook)
 {
 	return strlen(hook->ifname) >= hook->ifnamelen;
 }
@@ -2440,11 +2440,14 @@ static struct nft_hook *nft_netdev_hook_alloc(struct net *net,
 }
 
 static struct nft_hook *nft_hook_list_find(struct list_head *hook_list,
-					   const struct nft_hook *this)
+					   const struct nft_hook *this,
+					   bool strict)
 {
 	struct nft_hook *hook;
 
 	list_for_each_entry(hook, hook_list, list) {
+		if (strict && hook->ifnamelen != this->ifnamelen)
+			continue;
 		if (!strncmp(hook->ifname, this->ifname,
 			     min(hook->ifnamelen, this->ifnamelen))) {
 			if (hook->flags & NFT_HOOK_REMOVE)
@@ -2486,7 +2489,7 @@ static int nf_tables_parse_netdev_hooks(struct net *net,
 			err = PTR_ERR(hook);
 			goto err_hook;
 		}
-		if (nft_hook_list_find(hook_list, hook)) {
+		if (nft_hook_list_find(hook_list, hook, false)) {
 			NL_SET_BAD_ATTR(extack, tmp);
 			nft_netdev_hook_free(hook);
 			err = -EEXIST;
@@ -2943,7 +2946,7 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 					ops->hook	= basechain->ops.hook;
 				}
 
-				if (nft_hook_list_find(&basechain->hook_list, h)) {
+				if (nft_hook_list_find(&basechain->hook_list, h, false)) {
 					list_del(&h->list);
 					nft_netdev_hook_free(h);
 					continue;
@@ -2956,7 +2959,8 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 					    !nft_trans_chain_update(trans))
 						continue;
 
-					if (nft_hook_list_find(&nft_trans_chain_hooks(trans), h)) {
+					if (nft_hook_list_find(&nft_trans_chain_hooks(trans),
+							       h, false)) {
 						nft_chain_release_hook(&hook);
 						return -EEXIST;
 					}
@@ -3257,7 +3261,7 @@ static int nft_delchain_hook(struct nft_ctx *ctx,
 		return err;
 
 	list_for_each_entry(this, &chain_hook.list, list) {
-		hook = nft_hook_list_find(&basechain->hook_list, this);
+		hook = nft_hook_list_find(&basechain->hook_list, this, true);
 		if (!hook) {
 			err = -ENOENT;
 			goto err_chain_del_hook;
@@ -9073,7 +9077,7 @@ static int nft_register_flowtable_net_hooks(struct net *net,
 			if (!nft_is_active_next(net, ft))
 				continue;
 
-			if (nft_hook_list_find(&ft->hook_list, hook)) {
+			if (nft_hook_list_find(&ft->hook_list, hook, false)) {
 				err = -EEXIST;
 				goto err_unregister_net_hooks;
 			}
@@ -9150,7 +9154,7 @@ static int nft_flowtable_update(struct nft_ctx *ctx, const struct nlmsghdr *nlh,
 		return err;
 
 	list_for_each_entry_safe(hook, next, &flowtable_hook.list, list) {
-		if (nft_hook_list_find(&flowtable->hook_list, hook)) {
+		if (nft_hook_list_find(&flowtable->hook_list, hook, false)) {
 			list_del(&hook->list);
 			nft_netdev_hook_free(hook);
 			continue;
@@ -9163,7 +9167,7 @@ static int nft_flowtable_update(struct nft_ctx *ctx, const struct nlmsghdr *nlh,
 			    !nft_trans_flowtable_update(trans))
 				continue;
 
-			if (nft_hook_list_find(&nft_trans_flowtable_hooks(trans), hook)) {
+			if (nft_hook_list_find(&nft_trans_flowtable_hooks(trans), hook, false)) {
 				err = -EEXIST;
 				goto err_flowtable_update_hook;
 			}
@@ -9383,7 +9387,7 @@ static int nft_delflowtable_hook(struct nft_ctx *ctx,
 		return err;
 
 	list_for_each_entry(this, &flowtable_hook.list, list) {
-		hook = nft_hook_list_find(&flowtable->hook_list, this);
+		hook = nft_hook_list_find(&flowtable->hook_list, this, true);
 		if (!hook) {
 			err = -ENOENT;
 			goto err_flowtable_del_hook;
-- 
2.55.0

Re: [PATCH nf v3] netfilter: nf_tables: fix device name and prefix match in hook lookup

[Phil Sutter]

On Wed, Aug 26, 2026 at 03:32:08PM +0200, Fernando Fernandez Mancera wrote:
> Currently, a netdev chain or flowtable hooked to a device prefix can be
> unintentionally deleted by a control-plane request targeting an exact
> device name or even a shorter one due to the usage of min() to calculate
> the length to match.
> 
> Fix this by making sure an exact device match never matches a prefix and
> that both the target and the candidate have the same length during
> delete operation. The add and update paths retain the existing overlap
> matching to prevent a single device from matching multiple hooks.
> 
> Reported-by: Wei Fang <void0red@gmail.com>
> Closes: https://lore.kernel.org/netfilter-devel/CANE+tVrDeNCHQVmsqkV2ozeBqyE3GtRDMhZgsg1bhw10yGNTRQ@mail.gmail.com/
> Fixes: 6d07a289504a ("netfilter: nf_tables: Support wildcard netdev hook specs")
> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
> ---
>  net/netfilter/nf_tables_api.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index c112ecc4fca3..9ef12feec1ac 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -1978,7 +1978,7 @@ static int nft_dump_stats(struct sk_buff *skb, struct nft_stats __percpu *stats)
>  	return -ENOSPC;
>  }
>  
> -static bool hook_is_prefix(struct nft_hook *hook)
> +static bool hook_is_prefix(const struct nft_hook *hook)

This is an unrelated change now.

>  {
>  	return strlen(hook->ifname) >= hook->ifnamelen;
>  }
> @@ -2440,11 +2440,14 @@ static struct nft_hook *nft_netdev_hook_alloc(struct net *net,
>  }
>  
>  static struct nft_hook *nft_hook_list_find(struct list_head *hook_list,
> -					   const struct nft_hook *this)
> +					   const struct nft_hook *this,
> +					   bool strict)
>  {
>  	struct nft_hook *hook;
>  
>  	list_for_each_entry(hook, hook_list, list) {
> +		if (strict && hook->ifnamelen != this->ifnamelen)
> +			continue;
>  		if (!strncmp(hook->ifname, this->ifname,
>  			     min(hook->ifnamelen, this->ifnamelen))) {
>  			if (hook->flags & NFT_HOOK_REMOVE)
> @@ -2486,7 +2489,7 @@ static int nf_tables_parse_netdev_hooks(struct net *net,
>  			err = PTR_ERR(hook);
>  			goto err_hook;
>  		}
> -		if (nft_hook_list_find(hook_list, hook)) {
> +		if (nft_hook_list_find(hook_list, hook, false)) {
>  			NL_SET_BAD_ATTR(extack, tmp);
>  			nft_netdev_hook_free(hook);
>  			err = -EEXIST;
> @@ -2943,7 +2946,7 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
>  					ops->hook	= basechain->ops.hook;
>  				}
>  
> -				if (nft_hook_list_find(&basechain->hook_list, h)) {
> +				if (nft_hook_list_find(&basechain->hook_list, h, false)) {

I think this should be a strict match. It fixes a problem you didn't
intend to fix (I feel like an LLM-reviewer now), when updating a chain
with a partially matching wildcard:

| add table netdev t
| add chain netdev t c '{ type filter hook ingress priority 0; devices = { eth* }; }'
| add chain netdev t c '{ type filter hook ingress priority 0; devices = { et* }; }'

The last command should return EEXIST instead of being accepted and
treated as a NOP.

>  					list_del(&h->list);
>  					nft_netdev_hook_free(h);
>  					continue;
> @@ -2956,7 +2959,8 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
>  					    !nft_trans_chain_update(trans))
>  						continue;
>  
> -					if (nft_hook_list_find(&nft_trans_chain_hooks(trans), h)) {
> +					if (nft_hook_list_find(&nft_trans_chain_hooks(trans),
> +							       h, false)) {
>  						nft_chain_release_hook(&hook);
>  						return -EEXIST;
>  					}
> @@ -3257,7 +3261,7 @@ static int nft_delchain_hook(struct nft_ctx *ctx,
>  		return err;
>  
>  	list_for_each_entry(this, &chain_hook.list, list) {
> -		hook = nft_hook_list_find(&basechain->hook_list, this);
> +		hook = nft_hook_list_find(&basechain->hook_list, this, true);
>  		if (!hook) {
>  			err = -ENOENT;
>  			goto err_chain_del_hook;
> @@ -9073,7 +9077,7 @@ static int nft_register_flowtable_net_hooks(struct net *net,
>  			if (!nft_is_active_next(net, ft))
>  				continue;
>  
> -			if (nft_hook_list_find(&ft->hook_list, hook)) {
> +			if (nft_hook_list_find(&ft->hook_list, hook, false)) {
>  				err = -EEXIST;
>  				goto err_unregister_net_hooks;
>  			}
> @@ -9150,7 +9154,7 @@ static int nft_flowtable_update(struct nft_ctx *ctx, const struct nlmsghdr *nlh,
>  		return err;
>  
>  	list_for_each_entry_safe(hook, next, &flowtable_hook.list, list) {
> -		if (nft_hook_list_find(&flowtable->hook_list, hook)) {
> +		if (nft_hook_list_find(&flowtable->hook_list, hook, false)) {

Same here.

I'll write test cases for nftables shell test suite to cover these.

Thanks, Phil

>  			list_del(&hook->list);
>  			nft_netdev_hook_free(hook);
>  			continue;
> @@ -9163,7 +9167,7 @@ static int nft_flowtable_update(struct nft_ctx *ctx, const struct nlmsghdr *nlh,
>  			    !nft_trans_flowtable_update(trans))
>  				continue;
>  
> -			if (nft_hook_list_find(&nft_trans_flowtable_hooks(trans), hook)) {
> +			if (nft_hook_list_find(&nft_trans_flowtable_hooks(trans), hook, false)) {
>  				err = -EEXIST;
>  				goto err_flowtable_update_hook;
>  			}
> @@ -9383,7 +9387,7 @@ static int nft_delflowtable_hook(struct nft_ctx *ctx,
>  		return err;
>  
>  	list_for_each_entry(this, &flowtable_hook.list, list) {
> -		hook = nft_hook_list_find(&flowtable->hook_list, this);
> +		hook = nft_hook_list_find(&flowtable->hook_list, this, true);
>  		if (!hook) {
>  			err = -ENOENT;
>  			goto err_flowtable_del_hook;
> -- 
> 2.55.0
> 
> 

Re: [PATCH nf v3] netfilter: nf_tables: fix device name and prefix match in hook lookup

[Fernando Fernandez Mancera]

On 8/26/26 6:22 PM, Phil Sutter wrote:
> On Wed, Aug 26, 2026 at 03:32:08PM +0200, Fernando Fernandez Mancera wrote:
>> Currently, a netdev chain or flowtable hooked to a device prefix can be
>> unintentionally deleted by a control-plane request targeting an exact
>> device name or even a shorter one due to the usage of min() to calculate
>> the length to match.
>>
>> Fix this by making sure an exact device match never matches a prefix and
>> that both the target and the candidate have the same length during
>> delete operation. The add and update paths retain the existing overlap
>> matching to prevent a single device from matching multiple hooks.
>>
>> Reported-by: Wei Fang <void0red@gmail.com>
>> Closes: https://lore.kernel.org/netfilter-devel/CANE+tVrDeNCHQVmsqkV2ozeBqyE3GtRDMhZgsg1bhw10yGNTRQ@mail.gmail.com/
>> Fixes: 6d07a289504a ("netfilter: nf_tables: Support wildcard netdev hook specs")
>> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
>> ---
>>   net/netfilter/nf_tables_api.c | 24 ++++++++++++++----------
>>   1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>> index c112ecc4fca3..9ef12feec1ac 100644
>> --- a/net/netfilter/nf_tables_api.c
>> +++ b/net/netfilter/nf_tables_api.c
>> @@ -1978,7 +1978,7 @@ static int nft_dump_stats(struct sk_buff *skb, struct nft_stats __percpu *stats)
>>   	return -ENOSPC;
>>   }
>>   
>> -static bool hook_is_prefix(struct nft_hook *hook)
>> +static bool hook_is_prefix(const struct nft_hook *hook)
> 
> This is an unrelated change now.
> 

Oops, yes sorry.

>>   {
>>   	return strlen(hook->ifname) >= hook->ifnamelen;
>>   }
>> @@ -2440,11 +2440,14 @@ static struct nft_hook *nft_netdev_hook_alloc(struct net *net,
>>   }
>>   
>>   static struct nft_hook *nft_hook_list_find(struct list_head *hook_list,
>> -					   const struct nft_hook *this)
>> +					   const struct nft_hook *this,
>> +					   bool strict)
>>   {
>>   	struct nft_hook *hook;
>>   
>>   	list_for_each_entry(hook, hook_list, list) {
>> +		if (strict && hook->ifnamelen != this->ifnamelen)
>> +			continue;
>>   		if (!strncmp(hook->ifname, this->ifname,
>>   			     min(hook->ifnamelen, this->ifnamelen))) {
>>   			if (hook->flags & NFT_HOOK_REMOVE)
>> @@ -2486,7 +2489,7 @@ static int nf_tables_parse_netdev_hooks(struct net *net,
>>   			err = PTR_ERR(hook);
>>   			goto err_hook;
>>   		}
>> -		if (nft_hook_list_find(hook_list, hook)) {
>> +		if (nft_hook_list_find(hook_list, hook, false)) {
>>   			NL_SET_BAD_ATTR(extack, tmp);
>>   			nft_netdev_hook_free(hook);
>>   			err = -EEXIST;
>> @@ -2943,7 +2946,7 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
>>   					ops->hook	= basechain->ops.hook;
>>   				}
>>   
>> -				if (nft_hook_list_find(&basechain->hook_list, h)) {
>> +				if (nft_hook_list_find(&basechain->hook_list, h, false)) {
> 
> I think this should be a strict match. It fixes a problem you didn't
> intend to fix (I feel like an LLM-reviewer now), when updating a chain
> with a partially matching wildcard:
> 
> | add table netdev t
> | add chain netdev t c '{ type filter hook ingress priority 0; devices = { eth* }; }'
> | add chain netdev t c '{ type filter hook ingress priority 0; devices = { et* }; }'
> 
> The last command should return EEXIST instead of being accepted and
> treated as a NOP.

Hm. I am not so sure about it. Wouldn't that allow for overlapping 
devices with prefixes?

Given this is not straightforward and we need to think a bit about it, 
Pablo leave this out of the next PR to netdev. I am going to do a bit 
more testing.

> 
>>   					list_del(&h->list);
>>   					nft_netdev_hook_free(h);
>>   					continue;
>> @@ -2956,7 +2959,8 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
>>   					    !nft_trans_chain_update(trans))
>>   						continue;
>>   
>> -					if (nft_hook_list_find(&nft_trans_chain_hooks(trans), h)) {
>> +					if (nft_hook_list_find(&nft_trans_chain_hooks(trans),
>> +							       h, false)) {
>>   						nft_chain_release_hook(&hook);
>>   						return -EEXIST;
>>   					}
>> @@ -3257,7 +3261,7 @@ static int nft_delchain_hook(struct nft_ctx *ctx,
>>   		return err;
>>   
>>   	list_for_each_entry(this, &chain_hook.list, list) {
>> -		hook = nft_hook_list_find(&basechain->hook_list, this);
>> +		hook = nft_hook_list_find(&basechain->hook_list, this, true);
>>   		if (!hook) {
>>   			err = -ENOENT;
>>   			goto err_chain_del_hook;
>> @@ -9073,7 +9077,7 @@ static int nft_register_flowtable_net_hooks(struct net *net,
>>   			if (!nft_is_active_next(net, ft))
>>   				continue;
>>   
>> -			if (nft_hook_list_find(&ft->hook_list, hook)) {
>> +			if (nft_hook_list_find(&ft->hook_list, hook, false)) {
>>   				err = -EEXIST;
>>   				goto err_unregister_net_hooks;
>>   			}
>> @@ -9150,7 +9154,7 @@ static int nft_flowtable_update(struct nft_ctx *ctx, const struct nlmsghdr *nlh,
>>   		return err;
>>   
>>   	list_for_each_entry_safe(hook, next, &flowtable_hook.list, list) {
>> -		if (nft_hook_list_find(&flowtable->hook_list, hook)) {
>> +		if (nft_hook_list_find(&flowtable->hook_list, hook, false)) {
> 
> Same here.
> 
> I'll write test cases for nftables shell test suite to cover these.
> 

Thank you Phil for these reviews!

> Thanks, Phil>
>>   			list_del(&hook->list);
>>   			nft_netdev_hook_free(hook);
>>   			continue;
>> @@ -9163,7 +9167,7 @@ static int nft_flowtable_update(struct nft_ctx *ctx, const struct nlmsghdr *nlh,
>>   			    !nft_trans_flowtable_update(trans))
>>   				continue;
>>   
>> -			if (nft_hook_list_find(&nft_trans_flowtable_hooks(trans), hook)) {
>> +			if (nft_hook_list_find(&nft_trans_flowtable_hooks(trans), hook, false)) {
>>   				err = -EEXIST;
>>   				goto err_flowtable_update_hook;
>>   			}
>> @@ -9383,7 +9387,7 @@ static int nft_delflowtable_hook(struct nft_ctx *ctx,
>>   		return err;
>>   
>>   	list_for_each_entry(this, &flowtable_hook.list, list) {
>> -		hook = nft_hook_list_find(&flowtable->hook_list, this);
>> +		hook = nft_hook_list_find(&flowtable->hook_list, this, true);
>>   		if (!hook) {
>>   			err = -ENOENT;
>>   			goto err_flowtable_del_hook;
>> -- 
>> 2.55.0
>>
>>

Re: [PATCH nf v3] netfilter: nf_tables: fix device name and prefix match in hook lookup

[Phil Sutter]

On Wed, Aug 26, 2026 at 06:47:57PM +0200, Fernando Fernandez Mancera wrote:
> On 8/26/26 6:22 PM, Phil Sutter wrote:
> > On Wed, Aug 26, 2026 at 03:32:08PM +0200, Fernando Fernandez Mancera wrote:
> >> Currently, a netdev chain or flowtable hooked to a device prefix can be
> >> unintentionally deleted by a control-plane request targeting an exact
> >> device name or even a shorter one due to the usage of min() to calculate
> >> the length to match.
> >>
> >> Fix this by making sure an exact device match never matches a prefix and
> >> that both the target and the candidate have the same length during
> >> delete operation. The add and update paths retain the existing overlap
> >> matching to prevent a single device from matching multiple hooks.
> >>
> >> Reported-by: Wei Fang <void0red@gmail.com>
> >> Closes: https://lore.kernel.org/netfilter-devel/CANE+tVrDeNCHQVmsqkV2ozeBqyE3GtRDMhZgsg1bhw10yGNTRQ@mail.gmail.com/
> >> Fixes: 6d07a289504a ("netfilter: nf_tables: Support wildcard netdev hook specs")
> >> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
> >> ---
> >>   net/netfilter/nf_tables_api.c | 24 ++++++++++++++----------
> >>   1 file changed, 14 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> >> index c112ecc4fca3..9ef12feec1ac 100644
> >> --- a/net/netfilter/nf_tables_api.c
> >> +++ b/net/netfilter/nf_tables_api.c
> >> @@ -1978,7 +1978,7 @@ static int nft_dump_stats(struct sk_buff *skb, struct nft_stats __percpu *stats)
> >>   	return -ENOSPC;
> >>   }
> >>   
> >> -static bool hook_is_prefix(struct nft_hook *hook)
> >> +static bool hook_is_prefix(const struct nft_hook *hook)
> > 
> > This is an unrelated change now.
> > 
> 
> Oops, yes sorry.
> 
> >>   {
> >>   	return strlen(hook->ifname) >= hook->ifnamelen;
> >>   }
> >> @@ -2440,11 +2440,14 @@ static struct nft_hook *nft_netdev_hook_alloc(struct net *net,
> >>   }
> >>   
> >>   static struct nft_hook *nft_hook_list_find(struct list_head *hook_list,
> >> -					   const struct nft_hook *this)
> >> +					   const struct nft_hook *this,
> >> +					   bool strict)
> >>   {
> >>   	struct nft_hook *hook;
> >>   
> >>   	list_for_each_entry(hook, hook_list, list) {
> >> +		if (strict && hook->ifnamelen != this->ifnamelen)
> >> +			continue;
> >>   		if (!strncmp(hook->ifname, this->ifname,
> >>   			     min(hook->ifnamelen, this->ifnamelen))) {
> >>   			if (hook->flags & NFT_HOOK_REMOVE)
> >> @@ -2486,7 +2489,7 @@ static int nf_tables_parse_netdev_hooks(struct net *net,
> >>   			err = PTR_ERR(hook);
> >>   			goto err_hook;
> >>   		}
> >> -		if (nft_hook_list_find(hook_list, hook)) {
> >> +		if (nft_hook_list_find(hook_list, hook, false)) {
> >>   			NL_SET_BAD_ATTR(extack, tmp);
> >>   			nft_netdev_hook_free(hook);
> >>   			err = -EEXIST;
> >> @@ -2943,7 +2946,7 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
> >>   					ops->hook	= basechain->ops.hook;
> >>   				}
> >>   
> >> -				if (nft_hook_list_find(&basechain->hook_list, h)) {
> >> +				if (nft_hook_list_find(&basechain->hook_list, h, false)) {
> > 
> > I think this should be a strict match. It fixes a problem you didn't
> > intend to fix (I feel like an LLM-reviewer now), when updating a chain
> > with a partially matching wildcard:
> > 
> > | add table netdev t
> > | add chain netdev t c '{ type filter hook ingress priority 0; devices = { eth* }; }'
> > | add chain netdev t c '{ type filter hook ingress priority 0; devices = { et* }; }'
> > 
> > The last command should return EEXIST instead of being accepted and
> > treated as a NOP.
> 
> Hm. I am not so sure about it. Wouldn't that allow for overlapping 
> devices with prefixes?

You're right! I tested my concern with a flowtable, which behaves as I
expected (so the strict lookup suggested below works as it should). With
chains however, there is no duplicate hook check as for flowtables in
nft_register_flowtable_net_hooks.

I think the policy was to allow multiple netdev chains on the same
interface but not multiple flowtables. And IIRC, it wasn't clear whether
this restriction is really needed (e.g. for hw offloading) or kept "just
in case".

Either way, the duplicate hook check for flowtables just coincidentally
happens to do what we need, namely reject non-strict matching hooks in
the same flowtable/chain. This deserves a distinct lookup loop for both
flowtables and chains IMO.

I'll look into that tomorrow and once I have something I'll also submit
the test cases I wrote.

> Given this is not straightforward and we need to think a bit about it, 
> Pablo leave this out of the next PR to netdev. I am going to do a bit 
> more testing.

Sorry for putting obstacles in your way. :(

Cheers, Phil

Re: [PATCH nf v3] netfilter: nf_tables: fix device name and prefix match in hook lookup

[Fernando Fernandez Mancera]

On 8/26/26 9:06 PM, Phil Sutter wrote:
> On Wed, Aug 26, 2026 at 06:47:57PM +0200, Fernando Fernandez Mancera wrote:
>> On 8/26/26 6:22 PM, Phil Sutter wrote:
>>> On Wed, Aug 26, 2026 at 03:32:08PM +0200, Fernando Fernandez Mancera wrote:
>>>> Currently, a netdev chain or flowtable hooked to a device prefix can be
>>>> unintentionally deleted by a control-plane request targeting an exact
>>>> device name or even a shorter one due to the usage of min() to calculate
>>>> the length to match.
>>>>
>>>> Fix this by making sure an exact device match never matches a prefix and
>>>> that both the target and the candidate have the same length during
>>>> delete operation. The add and update paths retain the existing overlap
>>>> matching to prevent a single device from matching multiple hooks.
>>>>
>>>> Reported-by: Wei Fang <void0red@gmail.com>
>>>> Closes: https://lore.kernel.org/netfilter-devel/CANE+tVrDeNCHQVmsqkV2ozeBqyE3GtRDMhZgsg1bhw10yGNTRQ@mail.gmail.com/
>>>> Fixes: 6d07a289504a ("netfilter: nf_tables: Support wildcard netdev hook specs")
>>>> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
>>>> ---
>>>>    net/netfilter/nf_tables_api.c | 24 ++++++++++++++----------
>>>>    1 file changed, 14 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>>>> index c112ecc4fca3..9ef12feec1ac 100644
>>>> --- a/net/netfilter/nf_tables_api.c
>>>> +++ b/net/netfilter/nf_tables_api.c
>>>> @@ -1978,7 +1978,7 @@ static int nft_dump_stats(struct sk_buff *skb, struct nft_stats __percpu *stats)
>>>>    	return -ENOSPC;
>>>>    }
>>>>    
>>>> -static bool hook_is_prefix(struct nft_hook *hook)
>>>> +static bool hook_is_prefix(const struct nft_hook *hook)
>>>
>>> This is an unrelated change now.
>>>
>>
>> Oops, yes sorry.
>>
>>>>    {
>>>>    	return strlen(hook->ifname) >= hook->ifnamelen;
>>>>    }
>>>> @@ -2440,11 +2440,14 @@ static struct nft_hook *nft_netdev_hook_alloc(struct net *net,
>>>>    }
>>>>    
>>>>    static struct nft_hook *nft_hook_list_find(struct list_head *hook_list,
>>>> -					   const struct nft_hook *this)
>>>> +					   const struct nft_hook *this,
>>>> +					   bool strict)
>>>>    {
>>>>    	struct nft_hook *hook;
>>>>    
>>>>    	list_for_each_entry(hook, hook_list, list) {
>>>> +		if (strict && hook->ifnamelen != this->ifnamelen)
>>>> +			continue;
>>>>    		if (!strncmp(hook->ifname, this->ifname,
>>>>    			     min(hook->ifnamelen, this->ifnamelen))) {
>>>>    			if (hook->flags & NFT_HOOK_REMOVE)
>>>> @@ -2486,7 +2489,7 @@ static int nf_tables_parse_netdev_hooks(struct net *net,
>>>>    			err = PTR_ERR(hook);
>>>>    			goto err_hook;
>>>>    		}
>>>> -		if (nft_hook_list_find(hook_list, hook)) {
>>>> +		if (nft_hook_list_find(hook_list, hook, false)) {
>>>>    			NL_SET_BAD_ATTR(extack, tmp);
>>>>    			nft_netdev_hook_free(hook);
>>>>    			err = -EEXIST;
>>>> @@ -2943,7 +2946,7 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
>>>>    					ops->hook	= basechain->ops.hook;
>>>>    				}
>>>>    
>>>> -				if (nft_hook_list_find(&basechain->hook_list, h)) {
>>>> +				if (nft_hook_list_find(&basechain->hook_list, h, false)) {
>>>
>>> I think this should be a strict match. It fixes a problem you didn't
>>> intend to fix (I feel like an LLM-reviewer now), when updating a chain
>>> with a partially matching wildcard:
>>>
>>> | add table netdev t
>>> | add chain netdev t c '{ type filter hook ingress priority 0; devices = { eth* }; }'
>>> | add chain netdev t c '{ type filter hook ingress priority 0; devices = { et* }; }'
>>>
>>> The last command should return EEXIST instead of being accepted and
>>> treated as a NOP.
>>
>> Hm. I am not so sure about it. Wouldn't that allow for overlapping
>> devices with prefixes?
> 
> You're right! I tested my concern with a flowtable, which behaves as I
> expected (so the strict lookup suggested below works as it should). With
> chains however, there is no duplicate hook check as for flowtables in
> nft_register_flowtable_net_hooks.
> 
> I think the policy was to allow multiple netdev chains on the same
> interface but not multiple flowtables. And IIRC, it wasn't clear whether
> this restriction is really needed (e.g. for hw offloading) or kept "just
> in case".
> 
> Either way, the duplicate hook check for flowtables just coincidentally
> happens to do what we need, namely reject non-strict matching hooks in
> the same flowtable/chain. This deserves a distinct lookup loop for both
> flowtables and chains IMO.
> 

I agree. I am sending a v4 of this patch removing the unrelated change 
and let's address the other problem in a follow-up to nf-next. We should 
likely unify this behavior between flowtables and chains.

> I'll look into that tomorrow and once I have something I'll also submit
> the test cases I wrote.
> 
>> Given this is not straightforward and we need to think a bit about it,
>> Pablo leave this out of the next PR to netdev. I am going to do a bit
>> more testing.
> 
> Sorry for putting obstacles in your way. :(

No need to apologize, you found interesting issues in the patch :)

Thanks,
Fernando.

> 
> Cheers, Phil
>