* Re: [PATCH net] tools: ynl: fix string attribute length to include null terminator
2025-10-24 13:24 [PATCH net] tools: ynl: fix string attribute length to include null terminator Petr Oros
@ 2025-10-24 13:38 ` Ivan Vecera
2025-10-25 0:03 ` Jakub Kicinski
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Ivan Vecera @ 2025-10-24 13:38 UTC (permalink / raw)
To: Petr Oros, Donald Hunter, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, Jacob Keller,
Asbjørn Sloth Tønnesen, open list:NETWORKING [GENERAL],
open list
Cc: mschmidt
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@redhat.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@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] tools: ynl: fix string attribute length to include null terminator
2025-10-24 13:24 [PATCH net] tools: ynl: fix string attribute length to include null terminator Petr Oros
2025-10-24 13:38 ` Ivan Vecera
@ 2025-10-25 0:03 ` Jakub Kicinski
[not found] ` <CAPR2-9=UeTLfqWbfX+NcLef0BQ_xbKq7MJgt4YsjMi==FWZD-Q@mail.gmail.com>
2025-10-27 9:08 ` Ivan Vecera
2025-10-28 0:00 ` patchwork-bot+netdevbpf
3 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2025-10-25 0:03 UTC (permalink / raw)
To: Petr Oros
Cc: Donald Hunter, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Jacob Keller, Asbjørn Sloth Tønnesen,
netdev, ivecera, mschmidt, Zahari Doychev
On Fri, 24 Oct 2025 15:24:38 +0200 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@redhat.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);
> }
looks familiar...
Link: https://lore.kernel.org/20251018151737.365485-3-zahari.doychev@linux.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] tools: ynl: fix string attribute length to include null terminator
2025-10-24 13:24 [PATCH net] tools: ynl: fix string attribute length to include null terminator Petr Oros
2025-10-24 13:38 ` Ivan Vecera
2025-10-25 0:03 ` Jakub Kicinski
@ 2025-10-27 9:08 ` Ivan Vecera
2025-10-28 0:00 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: Ivan Vecera @ 2025-10-27 9:08 UTC (permalink / raw)
To: Petr Oros, Donald Hunter, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, Jacob Keller,
Asbjørn Sloth Tønnesen, open list:NETWORKING [GENERAL],
open list
Cc: mschmidt
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@redhat.com>
Reviewed-by: Ivan Vecera <ivecera@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] tools: ynl: fix string attribute length to include null terminator
2025-10-24 13:24 [PATCH net] tools: ynl: fix string attribute length to include null terminator Petr Oros
` (2 preceding siblings ...)
2025-10-27 9:08 ` Ivan Vecera
@ 2025-10-28 0:00 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-10-28 0:00 UTC (permalink / raw)
To: Petr Oros
Cc: donald.hunter, kuba, davem, edumazet, pabeni, horms,
jacob.e.keller, ast, netdev, linux-kernel, ivecera, mschmidt
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Fri, 24 Oct 2025 15:24:38 +0200 you 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)
>
> [...]
Here is the summary with links:
- [net] tools: ynl: fix string attribute length to include null terminator
https://git.kernel.org/netdev/net/c/65f9c4c58889
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread