Hi,
On 04/10/2024 15:38, Jakub Kicinski wrote:
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?
We already perform the same check in the 'else' branch below.
It'd be about moving it at the beginning of the function and bail out if
true, right?
Should I modify this patch and move the check above?
Cheers,
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.
--
Antonio Quartulli
OpenVPN Inc.