Re: [PATCH nf] 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]> |
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