Re: [nft PATCH v5 1/3] mnl: Support simple wildcards in netdev hooks

[Phil Sutter <phil@nwl.cc>]

On Fri, Sep 05, 2025 at 01:20:49PM +0200, Pablo Neira Ayuso wrote:
> On Fri, Sep 05, 2025 at 12:27:28AM +0200, Phil Sutter wrote:
> > On Thu, Sep 04, 2025 at 05:16:19PM +0200, Pablo Neira Ayuso wrote:
> > > Hi Phil,
> > > 
> > > NFTA_DEVICE_PREFIX is now available in net.git, let's pick up on this.
> > > 
> > > On Fri, Aug 01, 2025 at 12:29:43AM +0200, Phil Sutter wrote:
> > > > When building NFTA_{FLOWTABLE_,}HOOK_DEVS attributes, detect trailing
> > > > asterisks in interface names and transmit the leading part in a
> > > > NFTA_DEVICE_PREFIX attribute.
> > > > 
> > > > Deserialization (i.e., appending asterisk to interface prefixes returned
> > > > in NFTA_DEVICE_PREFIX atributes happens in libnftnl.
> > > > 
> > > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > > ---
> > > > Changes since v4:
> > > > - Introduce and use NFTA_DEVICE_PREFIX which contains a NUL-terminated
> > > >   string as well but signals the kernel to interpret it as a prefix to
> > > >   match interfaces on.
> > > > - Do not send wildcards in NFTA_HOOK_DEV: On one hand, the kernel can't
> > > >   detect them anymore since they are NUL-terminated as well. On the
> > > >   other, it would defeat the purpose of having NFTA_DEVICE_PREFIX, which
> > > >   is to not crash old user space.
> > > > 
> > > > Changes since v3:
> > > > - Use uint16_t for 'attr' parameter and size_t for 'len' variable
> > > > - Use mnl_nft_ prefix for the helper function
> > > > 
> > > > Changes since v2:
> > > > - Introduce mnl_attr_put_ifname() to perform the conditional
> > > >   mnl_attr_put() parameter adjustment
> > > > - Sanity-check array index in above function to avoid out-of-bounds
> > > >   access
> > > > ---
> > > >  include/linux/netfilter/nf_tables.h |  2 ++
> > > >  src/mnl.c                           | 26 +++++++++++++++++++++++---
> > > >  2 files changed, 25 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
> > > > index f57963e89fd16..b38d4780ae8c8 100644
> > > > --- a/include/linux/netfilter/nf_tables.h
> > > > +++ b/include/linux/netfilter/nf_tables.h
> > > > @@ -1774,10 +1774,12 @@ enum nft_synproxy_attributes {
> > > >   * enum nft_device_attributes - nf_tables device netlink attributes
> > > >   *
> > > >   * @NFTA_DEVICE_NAME: name of this device (NLA_STRING)
> > > > + * @NFTA_DEVICE_PREFIX: device name prefix, a simple wildcard (NLA_STRING)
> > > >   */
> > > >  enum nft_devices_attributes {
> > > >  	NFTA_DEVICE_UNSPEC,
> > > >  	NFTA_DEVICE_NAME,
> > > > +	NFTA_DEVICE_PREFIX,
> > > >  	__NFTA_DEVICE_MAX
> > > >  };
> > > >  #define NFTA_DEVICE_MAX		(__NFTA_DEVICE_MAX - 1)
> > > > diff --git a/src/mnl.c b/src/mnl.c
> > > > index 43229f2498e55..b532b8ff00c1e 100644
> > > > --- a/src/mnl.c
> > > > +++ b/src/mnl.c
> > > > @@ -795,6 +795,26 @@ static void nft_dev_array_free(const struct nft_dev *dev_array)
> > > >  	free_const(dev_array);
> > > >  }
> > > >  
> > > > +static bool is_wildcard_str(const char *str)
> > > > +{
> > > > +	size_t len = strlen(str);
> > > > +
> > > > +	if (len < 1 || str[len - 1] != '*')
> > > > +		return false;
> > > > +	if (len < 2 || str[len - 2] != '\\')
> > > > +		return true;
> > > > +	/* XXX: ignore backslash escaping for now */
> > > 
> > > Is this comment here still valid?
> > 
> > Yes, sadly. The above covers for eth* and eth\* but not for eth\\* since
> > a proper solution didn't quickly come to mind which avoids playing
> > whack-a-mole. (E.g., does eth\\\\\\* escape the wildcard or not?)
> > 
> > Guess I could just count the number of backslashes immediately preceding
> > the asterisk and return true if the sum is odd?
> 
> Thanks for explaining the comment, this supports eth* and eth\* which
> is sufficient at this stage.
> 
> For this series:
> 
> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>

Series applied, thanks!