Hi Pablo,
On Fri, Jul 25, 2025 at 05:12:42PM +0200, Pablo Neira Ayuso wrote:
> 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@xxxxxx>
> > ---
> > 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.
Ah, it's not just about the name. The initial version using
NFTA_DEVICE_NAME for both, distinction of wildcards from regular
names came from missing '\0' terminator. With distinct attribute types,
this is not needed anymore. I guess it's more user (space) friendly to
include the NUL-char in wildcards as well, right?
> > 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?
Attribute parser in libnftnl will abort if it finds an attribute with
type other than NFTA_DEVICE_NAME nested in NFTA_HOOK_DEVS (or the
flowtable equivalent). So old userspace will refuse to parse the data,
but not crash at least.
> 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.
ACK, it is definitely much more sane than before!
> 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.
We could introduce a "NFTA_DEVICE_NAME_NEW" which may hold wildcards or
a regular name (thereby keeping the NUL-char distinction mentioned
above) and at some point drop NFTA_DEVICE_NAME. Basically a merge
strategy to upgrade NFTA_DEVICE_NAME to support also wildcards, but I'm
not sure how long this transition period will take. At least it would
never crash old user space, but "merely" become incompatible to it at
some point.
Cheers, Phil
