Re: [PATCH net-next v8 01/24] netlink: add NLA_POLICY_MAX_LEN macro

From: Jakub Kicinski
Date: Fri Oct 04 2024 - 09:39:06 EST


On Fri, 04 Oct 2024 13:58:04 +0100 Donald Hunter wrote:
> > @@ -466,6 +466,8 @@ class TypeBinary(Type):
> > def _attr_policy(self, policy):
> > if 'exact-len' in self.checks:
> > mem = 'NLA_POLICY_EXACT_LEN(' + str(self.get_limit('exact-len')) + ')'
> > + elif 'max-len' in self.checks:
> > + mem = 'NLA_POLICY_MAX_LEN(' + str(self.get_limit('max-len')) + ')'
>
> This takes precedence over min-length. What if both are set? The logic
> should probably check and use NLA_POLICY_RANGE

Or we could check if len(self.checks) <= 1 early and throw our hands up
if there is more, for now?

> > else:
> > mem = '{ '
> > if len(self.checks) == 1 and 'min-len' in self.checks:
>
> Perhaps this should use NLA_POLICY_MIN_LEN ? In fact the current code
> looks broken to me because the NLA_BINARY len check in validate_nla() is
> a max length check, right?
>
> https://elixir.bootlin.com/linux/v6.11.1/source/lib/nlattr.c#L499
>
> The alternative is you emit an explicit initializer that includes the
> correct NLA_VALIDATE_* type and sets type, min and/or max.

Yeah, this code leads to endless confusion. We use NLA_UNSPEC (0)
if min-len is set (IOW we don't set .type to NLA_BINARY). NLA_UNSPEC
has different semantics for len.

Agreed that we should probably clean this up, but no bug AFAICT.