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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux