Re: [PATCH 2/4] tools: ynl: zero-initialize struct ynl_sock memory

Zahari Doychev
2025-10-21T17:37:48Z

On Mon, Oct 20, 2025 at 04:16:39PM -0700, Jakub Kicinski wrote:
> On Sat, 18 Oct 2025 17:17:35 +0200 Zahari Doychev wrote:
> > The memory belonging to tx_buf and rx_buf in ynl_sock is not
> > initialized after allocation. This commit ensures the entire
> > allocated memory is set to zero.
> >
> > When asan is enabled, uninitialized bytes may contain poison values.
> > This can cause failures e.g. when doing ynl_attr_put_str then poisoned
> > bytes appear after the null terminator. As a result, tc filter addition
> > may fail.
>
> We add strings with the null-terminating char, AFAICT.
> Do you mean that the poison value appears in the padding?
>

Yes, correct. The function nla_strcmp(...) does not match in this case as
the poison value appears in the padding after the null byte.

> > Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
> > ---
> > tools/net/ynl/lib/ynl.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/net/ynl/lib/ynl.c b/tools/net/ynl/lib/ynl.c
> > index 2bcd781111d7..16a4815d6a49 100644
> > --- a/tools/net/ynl/lib/ynl.c
> > +++ b/tools/net/ynl/lib/ynl.c
> > @@ -744,7 +744,7 @@ ynl_sock_create(const struct ynl_family *yf, struct ynl_error *yse)
> > ys = malloc(sizeof(*ys) + 2 * YNL_SOCKET_BUFFER_SIZE);
> > if (!ys)
> > return NULL;
> > - memset(ys, 0, sizeof(*ys));
> > + memset(ys, 0, sizeof(*ys) + 2 * YNL_SOCKET_BUFFER_SIZE);
>
> This is just clearing the buffer initially, it can be used for multiple
> requests. This change is no good as is.

I see. Should then the ynl_attr_put_str be changed to zero the padding
bytes or it is better to make sure the buffers are cleared for each
request?

Thanks