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

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

[Petr Oros]

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);
 }
-- 
2.51.0

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

[Ivan Vecera]

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>

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

[Jakub Kicinski]

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

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

[Zahari Doychev]

On Sat, Oct 25, 2025 at 08:43:27PM +0200, Petr Oros wrote:
> Yeah, that must be it.
> 
> Regards,
> -Petr

This fixes my problem as well.

thanks

> 
> Dne so 25. 10. 2025 2:03 uživatel Jakub Kicinski <kuba@kernel.org> napsal:
> 
> > 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
> >
> >

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

[Ivan Vecera]

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>

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

[patchwork-bot+netdevbpf]

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