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

Phil Sutter <[email protected]>
Newsgroups gmane.comp.security.firewalls.netfilter.devel
Message-ID <[email protected]>
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.

Thanks, Phil
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.