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

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi Fernando,

On Wed, Aug 26, 2026 at 12:55:32PM +0200, Fernando Fernandez Mancera wrote:
> 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 <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 | 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.

I can wait for a few hours before the next PR if you'd like to explore
the v3.

Thanks.