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

[Donald Hunter]

Antonio Quartulli <antonio@xxxxxxxxxxx> 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@xxxxxxxxx
> Signed-off-by: Antonio Quartulli <antonio@xxxxxxxxxxx>
> ---
>  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.