netdev - Re: [PATCH net] tools: ynl: fix string attribute length to include null terminator

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: <56ba4cd5-32d5-452b-9312-f6b778a44ef3@redhat.com>
Date: Fri, 24 Oct 2025 15:38:20 +0200
From: Ivan Vecera <ivecera@...hat.com>
To: Petr Oros <poros@...hat.com>, Donald Hunter <donald.hunter@...il.com>,
 Jakub Kicinski <kuba@...nel.org>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>, Jacob Keller <jacob.e.keller@...el.com>,
 Asbjørn Sloth Tønnesen <ast@...erby.net>,
 "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>,
 open list <linux-kernel@...r.kernel.org>
Cc: mschmidt@...hat.com
Subject: Re: [PATCH net] tools: ynl: fix string attribute length to include
 null terminator



On 10/24/25 3:24 PM, Petr Oros wrote:
> The ynl_attr_put_str() function was not including the null terminator
> in the attribute length calculation. This caused kernel to reject
> CTRL_CMD_GETFAMILY requests with EINVAL:
> "Attribute failed policy validation".
> 
> For a 4-character family name like "dpll":
> - Sent: nla_len=8 (4 byte header + 4 byte string without null)
> - Expected: nla_len=9 (4 byte header + 5 byte string with null)
> 
> The bug was introduced in commit 15d2540e0d62 ("tools: ynl: check for
> overflow of constructed messages") when refactoring from stpcpy() to
> strlen(). The original code correctly included the null terminator:
> 
>    end = stpcpy(ynl_attr_data(attr), str);
>    attr->nla_len = NLA_HDRLEN + NLA_ALIGN(end -
>                                  (char *)ynl_attr_data(attr));
> 
> Since stpcpy() returns a pointer past the null terminator, the length
> included it. The refactored version using strlen() omitted the +1.
> 
> The fix also removes NLA_ALIGN() from nla_len calculation, since
> nla_len should contain actual attribute length, not aligned length.
> Alignment is only for calculating next attribute position. This makes
> the code consistent with ynl_attr_put().
> 
> CTRL_ATTR_FAMILY_NAME uses NLA_NUL_STRING policy which requires
> null terminator. Kernel validates with memchr() and rejects if not
> found.
> 
> Fixes: 15d2540e0d62 ("tools: ynl: check for overflow of constructed messages")
> Signed-off-by: Petr Oros <poros@...hat.com>
> ---
>   tools/net/ynl/lib/ynl-priv.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/net/ynl/lib/ynl-priv.h b/tools/net/ynl/lib/ynl-priv.h
> index 29481989ea7662..ced7dce44efb43 100644
> --- a/tools/net/ynl/lib/ynl-priv.h
> +++ b/tools/net/ynl/lib/ynl-priv.h
> @@ -313,7 +313,7 @@ ynl_attr_put_str(struct nlmsghdr *nlh, unsigned int attr_type, const char *str)
>   	struct nlattr *attr;
>   	size_t len;
>   
> -	len = strlen(str);
> +	len = strlen(str) + 1;
>   	if (__ynl_attr_put_overflow(nlh, len))
>   		return;
>   
> @@ -321,7 +321,7 @@ ynl_attr_put_str(struct nlmsghdr *nlh, unsigned int attr_type, const char *str)
>   	attr->nla_type = attr_type;
>   
>   	strcpy((char *)ynl_attr_data(attr), str);
> -	attr->nla_len = NLA_HDRLEN + NLA_ALIGN(len);
> +	attr->nla_len = NLA_HDRLEN + len;
>   
>   	nlh->nlmsg_len += NLMSG_ALIGN(attr->nla_len);
>   }

Tested-by: Ivan Vecera <ivecera@...hat.com>