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

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 <[email protected]>
>> 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 <[email protected]>
>> ---
>>   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/[email protected]/

Thanks, Fernando.