Re: [PATCH net] psample: zero the netlink attribute padding in PSAMPLE_ATTR_DATA

[Jakub Kicinski <kuba@kernel.org>]

On Sat, 13 Jun 2026 17:02:39 -0700 Xiang Mei wrote:
> > >       if (data_len) {
> > > -             int nla_len = nla_total_size(data_len);
> > >               struct nlattr *nla;
> > >
> > > -             nla = skb_put(nl_skb, nla_len);
> > > -             nla->nla_type = PSAMPLE_ATTR_DATA;
> > > -             nla->nla_len = nla_attr_size(data_len);
> > > +             nla = nla_reserve(nl_skb, PSAMPLE_ATTR_DATA, data_len);
> > > +             if (!nla)
> > > +                     goto error;
> > >
> > >               if (skb_copy_bits(skb, 0, nla_data(nla), data_len))
> > >                       goto error;
> > >
> > > Let me know if the new patch makes sense.  
> >
> > I assumed the author intentionally was avoiding the memset for
> > the memory we will override with data. Otherwise the whole dance
> > could be avoided and nla_put() would have been the answer.  
> 
> The reason nla_put() isn't used here is that the payload source is the
> sampled skb, which can be nonlinear, so the data has to be gathered with
> skb_copy_bits() rather than a flat memcpy().

That too.

> There is no nla_put() variant that takes an skb source, hence the
> reserve-then-copy.
> 
> But that doesn't require open-coding the attribute: nla_reserve() only
> memsets the alignment padding (nla_padlen), never the data region, so
> nla_reserve() + skb_copy_bits() writes every byte exactly once with no
> redundant memset over the payload. The bug was that the open-coded
> version dropped that padding-zero step.

I find it hard to believe that nla_reserve() was not considered.
It's a widely used function in the networking stack.
Please move on.