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

On Wed, Aug 26, 2026 at 12:25:09PM +0200, Fernando Fernandez Mancera wrote:
> 
> 
> 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]/

Oh I see, thanks for the pointer. Let's meet over there. :)

Cheers, Phil