Linux Netfilter development
 help / color / mirror / Atom feed
* [PATCH nf] netfilter: nf_tables: fix device name and prefix match in hook lookup
@ 2026-08-18 10:18 Fernando Fernandez Mancera
  2026-08-26 10:22 ` Phil Sutter
  0 siblings, 1 reply; 4+ messages in thread
From: Fernando Fernandez Mancera @ 2026-08-18 10:18 UTC (permalink / raw)
  To: netfilter-devel
  Cc: pablo, fw, phil, coreteam, Fernando Fernandez Mancera, Wei Fang

Currently, a netdev chain or flowtable hooked to a device prefix can be
unintentionally deleted or updated 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.

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 | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index af357f6c5070..e2b18c4722fc 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1973,7 +1973,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,8 +2440,11 @@ static struct nft_hook *nft_hook_list_find(struct list_head *hook_list,
 	struct nft_hook *hook;
 
 	list_for_each_entry(hook, hook_list, list) {
-		if (!strncmp(hook->ifname, this->ifname,
-			     min(hook->ifnamelen, this->ifnamelen))) {
+		if (hook_is_prefix(hook) != hook_is_prefix(this))
+			continue;
+		if (hook->ifnamelen != this->ifnamelen)
+			continue;
+		if (!strncmp(hook->ifname, this->ifname, hook->ifnamelen)) {
 			if (hook->flags & NFT_HOOK_REMOVE)
 				continue;
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH nf] netfilter: nf_tables: fix device name and prefix match in hook lookup
  2026-08-18 10:18 [PATCH nf] netfilter: nf_tables: fix device name and prefix match in hook lookup Fernando Fernandez Mancera
@ 2026-08-26 10:22 ` Phil Sutter
  2026-08-26 10:25   ` Fernando Fernandez Mancera
  0 siblings, 1 reply; 4+ messages in thread
From: Phil Sutter @ 2026-08-26 10:22 UTC (permalink / raw)
  To: Fernando Fernandez Mancera; +Cc: netfilter-devel, pablo, fw, coreteam, Wei Fang

Hi Fernando,

On Tue, Aug 18, 2026 at 12:18:25PM +0200, Fernando Fernandez Mancera wrote:
> Currently, a netdev chain or flowtable hooked to a device prefix can be
> unintentionally deleted or updated 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.
> 
> 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 | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index af357f6c5070..e2b18c4722fc 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -1973,7 +1973,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,8 +2440,11 @@ static struct nft_hook *nft_hook_list_find(struct list_head *hook_list,
>  	struct nft_hook *hook;
>  
>  	list_for_each_entry(hook, hook_list, list) {
> -		if (!strncmp(hook->ifname, this->ifname,
> -			     min(hook->ifnamelen, this->ifnamelen))) {
> +		if (hook_is_prefix(hook) != hook_is_prefix(this))
> +			continue;
> +		if (hook->ifnamelen != this->ifnamelen)
> +			continue;
> +		if (!strncmp(hook->ifname, this->ifname, hook->ifnamelen)) {
>  			if (hook->flags & NFT_HOOK_REMOVE)
>  				continue;

I think this fix introduces another issue, when adding another prefix to
a flowtable/chain which partially matches an existing prefix. E.g.,
adding "eth*" with "eth0*" present must be rejected since we otherwise
end up trying to register a new interface "eth007" to two hooks at the
same time.

Maybe we need a "bool exact" flag which turns the min() intoa a max()?
(untested ;)

Cheers, Phil

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH nf] netfilter: nf_tables: fix device name and prefix match in hook lookup
  2026-08-26 10:22 ` Phil Sutter
@ 2026-08-26 10:25   ` Fernando Fernandez Mancera
  2026-08-26 10:32     ` Phil Sutter
  0 siblings, 1 reply; 4+ messages in thread
From: Fernando Fernandez Mancera @ 2026-08-26 10:25 UTC (permalink / raw)
  To: Phil Sutter; +Cc: netfilter-devel, pablo, fw, coreteam, Wei Fang



On 8/26/26 12:22 PM, Phil Sutter wrote:
> Hi Fernando,
> 
> On Tue, Aug 18, 2026 at 12:18:25PM +0200, Fernando Fernandez Mancera wrote:
>> Currently, a netdev chain or flowtable hooked to a device prefix can be
>> unintentionally deleted or updated 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.
>>
>> 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 | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>> index af357f6c5070..e2b18c4722fc 100644
>> --- a/net/netfilter/nf_tables_api.c
>> +++ b/net/netfilter/nf_tables_api.c
>> @@ -1973,7 +1973,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,8 +2440,11 @@ static struct nft_hook *nft_hook_list_find(struct list_head *hook_list,
>>   	struct nft_hook *hook;
>>   
>>   	list_for_each_entry(hook, hook_list, list) {
>> -		if (!strncmp(hook->ifname, this->ifname,
>> -			     min(hook->ifnamelen, this->ifnamelen))) {
>> +		if (hook_is_prefix(hook) != hook_is_prefix(this))
>> +			continue;
>> +		if (hook->ifnamelen != this->ifnamelen)
>> +			continue;
>> +		if (!strncmp(hook->ifname, this->ifname, hook->ifnamelen)) {
>>   			if (hook->flags & NFT_HOOK_REMOVE)
>>   				continue;
> 
> I think this fix introduces another issue, when adding another prefix to
> a flowtable/chain which partially matches an existing prefix. E.g.,
> adding "eth*" with "eth0*" present must be rejected since we otherwise
> end up trying to register a new interface "eth007" to two hooks at the
> same time.
> 
> Maybe we need a "bool exact" flag which turns the min() intoa a max()?
> (untested ;)
> 
> Cheers, Phil

Hi Phil, you are right here. I sent a v2 2 days ago changing the 
behavior of delete operation only. What do you think?

https://lore.kernel.org/netfilter-devel/20260824092805.4940-1-fmancera@suse.de/

Thanks, Fernando.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH nf] netfilter: nf_tables: fix device name and prefix match in hook lookup
  2026-08-26 10:25   ` Fernando Fernandez Mancera
@ 2026-08-26 10:32     ` Phil Sutter
  0 siblings, 0 replies; 4+ messages in thread
From: Phil Sutter @ 2026-08-26 10:32 UTC (permalink / raw)
  To: Fernando Fernandez Mancera; +Cc: netfilter-devel, pablo, fw, coreteam, Wei Fang

On Wed, Aug 26, 2026 at 12:25:09PM +0200, Fernando Fernandez Mancera wrote:
> 
> 
> On 8/26/26 12:22 PM, Phil Sutter wrote:
> > Hi Fernando,
> > 
> > On Tue, Aug 18, 2026 at 12:18:25PM +0200, Fernando Fernandez Mancera wrote:
> >> Currently, a netdev chain or flowtable hooked to a device prefix can be
> >> unintentionally deleted or updated 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.
> >>
> >> 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 | 9 ++++++---
> >>   1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> >> index af357f6c5070..e2b18c4722fc 100644
> >> --- a/net/netfilter/nf_tables_api.c
> >> +++ b/net/netfilter/nf_tables_api.c
> >> @@ -1973,7 +1973,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,8 +2440,11 @@ static struct nft_hook *nft_hook_list_find(struct list_head *hook_list,
> >>   	struct nft_hook *hook;
> >>   
> >>   	list_for_each_entry(hook, hook_list, list) {
> >> -		if (!strncmp(hook->ifname, this->ifname,
> >> -			     min(hook->ifnamelen, this->ifnamelen))) {
> >> +		if (hook_is_prefix(hook) != hook_is_prefix(this))
> >> +			continue;
> >> +		if (hook->ifnamelen != this->ifnamelen)
> >> +			continue;
> >> +		if (!strncmp(hook->ifname, this->ifname, hook->ifnamelen)) {
> >>   			if (hook->flags & NFT_HOOK_REMOVE)
> >>   				continue;
> > 
> > I think this fix introduces another issue, when adding another prefix to
> > a flowtable/chain which partially matches an existing prefix. E.g.,
> > adding "eth*" with "eth0*" present must be rejected since we otherwise
> > end up trying to register a new interface "eth007" to two hooks at the
> > same time.
> > 
> > Maybe we need a "bool exact" flag which turns the min() intoa a max()?
> > (untested ;)
> > 
> > Cheers, Phil
> 
> Hi Phil, you are right here. I sent a v2 2 days ago changing the 
> behavior of delete operation only. What do you think?
> 
> https://lore.kernel.org/netfilter-devel/20260824092805.4940-1-fmancera@suse.de/

Oh I see, thanks for the pointer. Let's meet over there. :)

Cheers, Phil

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-26 10:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-18 10:18 [PATCH nf] netfilter: nf_tables: fix device name and prefix match in hook lookup Fernando Fernandez Mancera
2026-08-26 10:22 ` Phil Sutter
2026-08-26 10:25   ` Fernando Fernandez Mancera
2026-08-26 10:32     ` Phil Sutter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox