linux-kernel - Re: [PATCH net-next v8 01/24] netlink: add NLA_POLICY_MAX

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <m2msjkf2jn.fsf@gmail.com>
Date: Fri, 04 Oct 2024 13:58:04 +0100
From: Donald Hunter <donald.hunter@...il.com>
To: Antonio Quartulli <antonio@...nvpn.net>
Cc: Eric Dumazet <edumazet@...gle.com>,  Jakub Kicinski <kuba@...nel.org>,
  Paolo Abeni <pabeni@...hat.com>,  Shuah Khan <shuah@...nel.org>,
  netdev@...r.kernel.org,  linux-kernel@...r.kernel.org,
  linux-kselftest@...r.kernel.org,  sd@...asysnail.net,
  ryazanov.s.a@...il.com
Subject: Re: [PATCH net-next v8 01/24] netlink: add NLA_POLICY_MAX_LEN macro

Antonio Quartulli <antonio@...nvpn.net> writes:

> Similarly to NLA_POLICY_MIN_LEN, NLA_POLICY_MAX_LEN defines a policy
> with a maximum length value.
>
> The netlink generator for YAML specs has been extended accordingly.
>
> Cc: donald.hunter@...il.com
> Signed-off-by: Antonio Quartulli <antonio@...nvpn.net>
> ---
>  include/net/netlink.h      | 1 +
>  tools/net/ynl/ynl-gen-c.py | 2 ++
>  2 files changed, 3 insertions(+)
>
> diff --git a/include/net/netlink.h b/include/net/netlink.h
> index db6af207287c839408c58cb28b82408e0548eaca..2dc671c977ff3297975269d236264907009703d3 100644
> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -469,6 +469,7 @@ struct nla_policy {
>  	.max = _len						\
>  }
>  #define NLA_POLICY_MIN_LEN(_len)	NLA_POLICY_MIN(NLA_BINARY, _len)
> +#define NLA_POLICY_MAX_LEN(_len)	NLA_POLICY_MAX(NLA_BINARY, _len)
>  
>  /**
>   * struct nl_info - netlink source information
> diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
> index 717530bc9c52e7cfa897814870b4583c88618a27..3ccbb301be87f80bbcf03da63d60f58c4fedc1c8 100755
> --- a/tools/net/ynl/ynl-gen-c.py
> +++ b/tools/net/ynl/ynl-gen-c.py
> @@ -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

>          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.