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

On 8/24/26 2:21 PM, Pablo Neira Ayuso wrote:
> Hi Fernando,
> 
> 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.
> 
> netdev maintainers prefer follow ups for LLM reported issues.
> 
> Please, provide an incremental patch for your v1 that we can take in
> the next PR round.
> 

Oh really? Even in this situation? Will do once the PR is accepted. The 
patch won't be much different of course.

Thanks!

> Thanks!
> 
>> 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;
>> +		if (hook->ifnamelen != this->ifnamelen)
>> +			continue;
>> +		if (!strncmp(hook->ifname, this->ifname, hook->ifnamelen)) {
>> +			if (hook->flags & NFT_HOOK_REMOVE)
>> +				continue;
>> +
>> +			return hook;
>> +		}
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>>   static int nf_tables_parse_netdev_hooks(struct net *net,
>>   					const struct nlattr *attr,
>>   					struct list_head *hook_list,
>> @@ -3257,7 +3278,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_strict(&basechain->hook_list, this);
>>   		if (!hook) {
>>   			err = -ENOENT;
>>   			goto err_chain_del_hook;
>> @@ -9383,7 +9404,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_strict(&flowtable->hook_list, this);
>>   		if (!hook) {
>>   			err = -ENOENT;
>>   			goto err_flowtable_del_hook;
>> -- 
>> 2.55.0
>>