Re: [nf-next RFC] netfilter: nf_tables: Introduce NFTA_DEVICE_WILDCARD

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

Hi Phil,

On Fri, Jul 25, 2025 at 12:00:31AM +0200, Phil Sutter wrote:
> On netlink receive side, this attribute is just another name for
> NFTA_DEVICE_NAME and handled equally. It enables user space to detect
> lack of wildcard interface spec support as older kernels will reject it.
> 
> On netlink send side, it is used for wildcard interface specs to avoid
> confusing or even crashing old user space with non NUL-terminated
> strings in attributes which are expected to be NUL-terminated.

This looks good to me.

> Fixes: 6d07a289504a ("netfilter: nf_tables: Support wildcard netdev hook specs")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> While this works, I wonder if it should be named NFTA_DEVICE_PREFIX
> instead and contain NUL-terminated strings just like NFTA_DEVICE_NAME.
> Kernel-internally I would continue using strncmp() and hook->ifnamelen,
> but handling in user space might be simpler.

Pick the name you like.

> A downside of this approach is that we mix NFTA_DEVICE_NAME and
> NFTA_DEVICE_WILDCARD attributes in NFTA_FLOWTABLE_HOOK_DEVS and
> NFTA_HOOK_DEVS nested attributes, even though old user space will reject
> the whole thing and not just take the known attributes and ignore the
> rest.

Old userspace is just ignoring the unknown attribute?

I think upside is good enough to follow this approach: new userspace
version with old kernel bails out with EINVAL, so it is easy to see
that feature is unsupported.

As for netlink attributes coming from the kernel, we can just review
the existing userspace parsing side and see what we can do better in
that regard.

> ---
>  include/uapi/linux/netfilter/nf_tables.h |  1 +
>  net/netfilter/nf_tables_api.c            | 16 +++++++++++++---
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> index 2beb30be2c5f..ed85b61afcd8 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -1788,6 +1788,7 @@ enum nft_synproxy_attributes {
>  enum nft_devices_attributes {
>  	NFTA_DEVICE_UNSPEC,
>  	NFTA_DEVICE_NAME,
> +	NFTA_DEVICE_WILDCARD,
>  	__NFTA_DEVICE_MAX
>  };
>  #define NFTA_DEVICE_MAX		(__NFTA_DEVICE_MAX - 1)
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 04795af6e586..f65b4fba5225 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -1959,6 +1959,14 @@ static int nft_dump_stats(struct sk_buff *skb, struct nft_stats __percpu *stats)
>  	return -ENOSPC;
>  }
>  
> +static int hook_device_attr(struct nft_hook *hook)
> +{
> +	if (hook->ifname[hook->ifnamelen - 1] == '\0')
> +		return NFTA_DEVICE_NAME;
> +
> +	return NFTA_DEVICE_WILDCARD;
> +}
> +
>  static int nft_dump_basechain_hook(struct sk_buff *skb,
>  				   const struct net *net, int family,
>  				   const struct nft_base_chain *basechain,
> @@ -1990,7 +1998,7 @@ static int nft_dump_basechain_hook(struct sk_buff *skb,
>  			if (!first)
>  				first = hook;
>  
> -			if (nla_put(skb, NFTA_DEVICE_NAME,
> +			if (nla_put(skb, hook_device_attr(hook),
>  				    hook->ifnamelen, hook->ifname))
>  				goto nla_put_failure;
>  			n++;
> @@ -1998,6 +2006,7 @@ static int nft_dump_basechain_hook(struct sk_buff *skb,
>  		nla_nest_end(skb, nest_devs);
>  
>  		if (n == 1 &&
> +		    hook_device_attr(first) != NFTA_DEVICE_WILDCARD &&
>  		    nla_put(skb, NFTA_HOOK_DEV,
>  			    first->ifnamelen, first->ifname))
>  			goto nla_put_failure;
> @@ -2376,7 +2385,8 @@ static int nf_tables_parse_netdev_hooks(struct net *net,
>  	int rem, n = 0, err;
>  
>  	nla_for_each_nested(tmp, attr, rem) {
> -		if (nla_type(tmp) != NFTA_DEVICE_NAME) {
> +		if (nla_type(tmp) != NFTA_DEVICE_NAME &&
> +		    nla_type(tmp) != NFTA_DEVICE_WILDCARD) {
>  			err = -EINVAL;
>  			goto err_hook;
>  		}
> @@ -9427,7 +9437,7 @@ static int nf_tables_fill_flowtable_info(struct sk_buff *skb, struct net *net,
>  
>  	list_for_each_entry_rcu(hook, hook_list, list,
>  				lockdep_commit_lock_is_held(net)) {
> -		if (nla_put(skb, NFTA_DEVICE_NAME,
> +		if (nla_put(skb, hook_device_attr(hook),
>  			    hook->ifnamelen, hook->ifname))
>  			goto nla_put_failure;
>  	}
> -- 
> 2.49.0
>