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

On 8/26/26 12:46 PM, Phil Sutter wrote:
> On Mon, Aug 24, 2026 at 11:28:05AM +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 <[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 | 27 ++++++++++++++++++++++++---
>>   1 file changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>> index c112ecc4fca3..3ad9a5e88c5b 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;
>>   }
>> @@ -2457,6 +2457,27 @@ static struct nft_hook *nft_hook_list_find(struct list_head *hook_list,
>>   	return NULL;
>>   }
>>   
>> +static struct nft_hook *nft_hook_list_find_strict(struct list_head *hook_list,
>> +						  const struct nft_hook *this)
>> +{
>> +	struct nft_hook *hook;
>> +
>> +	list_for_each_entry(hook, hook_list, list) {
>> +		if (hook_is_prefix(hook) != hook_is_prefix(this))
>> +			continue;
> 
> I don't think this is necessary: The first ifnamelen chars of both
> hooks' ifname either contain the NUL-char or not. If both ifnamelen
> values are identical, the following strncmp will only match if either
> both are prefixes or both are not.
> 
> (I realized my initial approach using max() won't work when comparing
> "eth0\0" (non-prefix) to "eth0" (prefix) as the latter is followed by
> NUL-chars as well.)
> 
>> +		if (hook->ifnamelen != this->ifnamelen)
>> +			continue;
> 
> Given the above, this could become 'if (strict && hook-ifnamelen !=...)'
> in nft_hook_list_find with extra parameter, up to you.
> 

Hm, that is true. I am sending a v3 following this method as it is simpler.

Pablo, feel free to pick up either this v2 or the v3 I am sending. Both 
are fine to me.

Thank you Phil and Pablo!