Re: [PATCH net-next v3 01/10] handshake: Add tags to "done" downcall

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] handshake: HANDSHAKE_A_DONE_TAG uses NLA_STRING instead of
  NLA_NUL_STRING
--

commit 686d8603f3072dc2bd0a821dcea3a0351fd8b262
Author: Chuck Lever <cel@kernel.org>

handshake: Add tags to "done" downcall

This commit adds the HANDSHAKE_A_DONE_TAG netlink attribute to the
handshake "done" downcall so userspace agents can return session tags to
the kernel.

> diff --git a/Documentation/netlink/specs/handshake.yaml b/Documentation/netlink/specs/handshake.yaml
> index 0498ada2dd97a..17521b159e893 100644
> --- a/Documentation/netlink/specs/handshake.yaml
> +++ b/Documentation/netlink/specs/handshake.yaml
[ ... ]
> @@ -96,6 +100,16 @@ attribute-sets:
>          name: remote-auth
>          type: s32
>          multi-attr: true
> +      -
> +        name: tag
> +        type: string
> +        doc: >-
> +          A tag the handshake agent assigns to this handshake. The
> +          payload may omit the trailing NUL. An embedded NUL is
> +          rejected.
> +        checks:
> +          max-len: session-tag-max-len
> +        multi-attr: true
[ ... ]
> diff --git a/net/handshake/genl.c b/net/handshake/genl.c
> index 26e3efec64ca7..7150b32a54c45 100644
> --- a/net/handshake/genl.c
> +++ b/net/handshake/genl.c
> @@ -18,10 +18,11 @@ static const struct nla_policy handshake_accept_nl_policy[HANDSHAKE_A_ACCEPT_HAN
>  };
>  
>  /* HANDSHAKE_CMD_DONE - do */
> -static const struct nla_policy handshake_done_nl_policy[HANDSHAKE_A_DONE_REMOTE_AUTH + 1] = {
> +static const struct nla_policy handshake_done_nl_policy[HANDSHAKE_A_DONE_TAG + 1] = {
>  	[HANDSHAKE_A_DONE_STATUS] = NLA_POLICY_MAX(NLA_U32, MAX_ERRNO),
>  	[HANDSHAKE_A_DONE_SOCKFD] = { .type = NLA_S32, },
>  	[HANDSHAKE_A_DONE_REMOTE_AUTH] = { .type = NLA_S32, },
> +	[HANDSHAKE_A_DONE_TAG] = { .type = NLA_STRING, .len = HANDSHAKE_SESSION_TAG_MAX_LEN, },
>  };

[Severity: Medium]
Should HANDSHAKE_A_DONE_TAG require NUL termination with NLA_NUL_STRING
instead of NLA_STRING?

In non-legacy genetlink specifications, string attributes are required to
be NUL-terminated.  Here, tools/net/ynl/pyynl/ynl_gen_c.py emitted
NLA_STRING only because TypeMultiAttr inherits Type.attr_policy() and
delegates _attr_policy() instead of attr_policy(), bypassing
TypeString.attr_policy():

tools/net/ynl/pyynl/ynl_gen_c.py:TypeMultiAttr._attr_policy() {
    def _attr_policy(self, policy):
        return self.base_type._attr_policy(policy)
}

Documenting in handshake.yaml that the payload may omit the trailing NUL
bakes this generator quirk into the uAPI: if TypeMultiAttr is later fixed
to emit NLA_NUL_STRING for multi-attr strings, regenerating the policy
with tools/net/ynl/ynl-regen.sh will break userspace agents that omit the
trailing NUL.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-tls-session-tags-v3-0-91271964cdb0@kernel.org?part=1