RE: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up() on empty publication list

Subject: Re: [PATCH net v8 1/3] tipc: fix NULL deref in tipc_named_node_up()
on empty publication list

Hi Tung,

I tested this on v7.1-rc5 with the two-node setup (UDP bearers, node-id
addressing), as an unprivileged user and as root.
The original NULL deref is fixed, and the sashiko findings look addressed.

But I found one path the patch misses: tipc_node_cleanup(). It removes a stale
node with tipc_node_delete_from_list() directly, without the wake/cancel you
added in tipc_node_delete(). If a deferred worker is still parked when the stale
timer fires, the node is removed from the list while the worker is sleeping in
wait_var_event(). Since the node is off the list, the wake in tipc_node_delete()
can no longer reach it. The worker stays in D state and keeps its node
reference, so the node struct leaks as well.

This state is reachable from userspace without any kernel change:
filling local_publ_count to TIPC_MAX_PUBL with node-scope binds makes
tipc_net_finalize()'s publish fail, and the work_rescheduled retry then keeps
finalized at 0, so the link-up defers the worker for as long as the table stays full.
(The retry also也重试 busy-loops, printing "tipc: Bind failed, max limit 65535
reached" each round.) Bringing the link down makes the peer node go stale
NODE_CLEANUP_AFTER (300s) later, with the worker still parked.
The hung task watchdog flags it:

    [  370.440401] INFO: task kworker/0:0:9 blocked for more than 245 seconds.
    [  370.441580] Workqueue: events tipc_node_dist_bulk
    [  370.442803] Call Trace:
    [  370.443151]  __schedule+0x18bf/0x4680
    [  370.444293]  tipc_named_dist_cluster_scope+0x1a0/0x220
    [  370.446076]  tipc_node_dist_bulk+0x6d/0x1b0
    [  370.446218]  process_one_work+0x845/0x1a60 ```

With a test-only marker printk in tipc_node_cleanup() I can see the stale timer
remove both nodes at ~368s and ~375s, i.e. while the workers above are still
parked. Nothing wakes them on that path; in this run they only exit because
killing the reproducer closes the bind sockets, so the next retry's publish
succeeds. With a persistent failure source (e.g. memory
pressure) the worker would stay parked across the netns teardown as well.

[  247.560544] INFO: task kworker/0:0:9 blocked for more than 122 seconds.
    [  247.568089] INFO: task kworker/0:2:67 blocked for more than 122
seconds.

    [  370.440401] INFO: task kworker/0:0:9 blocked for more than 245 seconds.
    [  370.441580] Workqueue: events tipc_node_dist_bulk
    [  370.442803] Call Trace:
    [  370.443151]  __schedule+0x18bf/0x4680
    [  370.444293]  tipc_named_dist_cluster_scope+0x1a0/0x220     ←
wait_var_event 睡眠点
    [  370.446076]  tipc_node_dist_bulk+0x6d/0x1b0
    [  370.446218]  process_one_work+0x845/0x1a60

The Configs and reproducer ：

CONFIG_TIPC=y,
CONFIG_TIPC_MEDIA_UDP=y, CONFIG_USER_NS=y, CONFIG_NET_NS=y,
CONFIG_KASAN=y (crash on the unpatched kernel),
CONFIG_DETECT_HUNG_TASK=y ```

#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sched.h>
#include <fcntl.h>
#include <signal.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <net/if.h>
#include <linux/netlink.h>
#include <linux/rtnetlink.h>
#include <linux/genetlink.h>
#include <linux/if_link.h>
#include <linux/veth.h>
#include <linux/tipc.h>

#ifndef AF_TIPC
#define AF_TIPC 30
#endif
#ifndef SOCK_RDM
#define SOCK_RDM 4
#endif

#ifndef VETH_INFO_PEER
#define VETH_INFO_PEER 1
#endif

#define TIPC_GENL_V2_NAME "TIPCv2"

enum {
       TIPC_NL_UNSPEC,
       TIPC_NL_LEGACY,
       TIPC_NL_BEARER_DISABLE,
       TIPC_NL_BEARER_ENABLE,
       TIPC_NL_BEARER_GET,
       TIPC_NL_BEARER_SET,
       TIPC_NL_SOCK_GET,
       TIPC_NL_PUBL_GET,
       TIPC_NL_LINK_GET,
       TIPC_NL_LINK_SET,
       TIPC_NL_LINK_RESET_STATS,
       TIPC_NL_MEDIA_GET,
       TIPC_NL_MEDIA_SET,
       TIPC_NL_NODE_GET,
       TIPC_NL_NET_GET,
       TIPC_NL_NET_SET,
};

enum {
       TIPC_NLA_UNSPEC,
       TIPC_NLA_BEARER,
       TIPC_NLA_SOCK,
       TIPC_NLA_PUBL,
       TIPC_NLA_LINK,
       TIPC_NLA_MEDIA,
       TIPC_NLA_NODE,
       TIPC_NLA_NET,
};

enum {
       TIPC_NLA_BEARER_UNSPEC,
       TIPC_NLA_BEARER_NAME,
       TIPC_NLA_BEARER_PROP,
       TIPC_NLA_BEARER_DOMAIN,
       TIPC_NLA_BEARER_UDP_OPTS,
};

enum {
       TIPC_NLA_UDP_UNSPEC,
       TIPC_NLA_UDP_LOCAL,
       TIPC_NLA_UDP_REMOTE,
       TIPC_NLA_UDP_MULTI_REMOTEIP,
};

enum {
       TIPC_NLA_NET_UNSPEC,
       TIPC_NLA_NET_ID,
       TIPC_NLA_NET_ADDR,
       TIPC_NLA_NET_NODEID,
       TIPC_NLA_NET_NODEID_W1,
};

#define TIPC_UDP_PORT 6118
#define NLA_ALIGNTO 4
#define MY_NLA_ALIGN(len) (((len) + NLA_ALIGNTO - 1) & ~(NLA_ALIGNTO -
1))

static int tipc_family;

static int nl_open(int protocol)
{
       int fd = socket(AF_NETLINK, SOCK_RAW, protocol);
       if (fd < 0) { perror("socket(NETLINK)"); exit(1); }
       struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
       if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
               perror("bind(NETLINK)"); exit(1);
       }
       return fd;
}

static void *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);
       *off += MY_NLA_ALIGN(a->nla_len);
       return a;
}

static struct nlattr *nla_nest_start(char *buf, int *off, int type) {
       struct nlattr *a = (struct nlattr *)(buf + *off);
       a->nla_type = type | NLA_F_NESTED;
       *off += NLA_HDRLEN;
       return a;
}

static void nla_nest_end(char *buf, int *off, struct nlattr *start) {
       start->nla_len = (buf + *off) - (char *)start; }

static int genl_resolve_family(int fd, const char *name) {
       char buf[1024];
       memset(buf, 0, sizeof(buf));
       struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
       struct genlmsghdrgenlmsghdr *gh = (struct genlmsghdrgenlmsghdr
*)NLMSG_DATA(nlh);
       int off = NLMSG_HDRLEN + GENL_HDRLEN;

       nlh->nlmsg_type = GENL_ID_CTRL;
       nlh->nlmsg_flags = NLM_F_REQUEST;
       nlh->nlmsg_seq = 1;
       gh->cmd = CTRL_CMD_GETFAMILY;
       gh->version = 1;
       nla_put(buf, &off, CTRL_ATTR_FAMILY_NAME, name, strlen(name) + 1);
       nlh->nlmsg_len = off;

       if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send genl resolve");
exit(1); }

       char rbuf[4096];
       int n = recv(fd, rbuf, sizeof(rbuf), 0);
       if (n < 0) { perror("recv genl resolve"); exit(1); }

       struct nlmsghdr *rh = (struct nlmsghdr *)rbuf;
       if (rh->nlmsg_type == NLMSG_ERROR)
               return -1;
       struct nlattr *a = (struct nlattr *)((char *)NLMSG_DATA(rh) +
GENL_HDRLEN);
       int alen阿伦 = rh->nlmsg_len - NLMSG_HDRLEN - GENL_HDRLEN;
       while (alen > 0) {
               if (a->nla_type == CTRL_ATTR_FAMILY_ID)
                       return *(uint16_t *)((char *)a + NLA_HDRLEN);
               int step = MY_NLA_ALIGN(a->nla_len);
               alen -= step;
               a = (struct nlattr *)((char *)a + step);
       }
       return -1;
}

static int nl_recv_ack(int fd, const char *what) {
       char rbuf[8192];
       int n = recv(fd, rbuf, sizeof(rbuf), 0);
       if (n < 0) { fprintf(stderr, "recv ack (%s): %s\n", what, strerror(errno));
return -1; }
       struct nlmsghdr *rh = (struct nlmsghdr *)rbuf;
       if (rh->nlmsg_type == NLMSG_ERROR) {
               struct nlmsgerr *e = NLMSG_DATA(rh);
               if (e->error != 0)
                       fprintf(stderr, "[%s] netlink error: %d (%s)\n", what, e->error,
strerror(-e->error));
               return e->error;
       }
       return 0;
}

static void *rta_put(char *buf, int *off, int type, const void *data, int len) {
       struct rtattr *a = (struct rtattr *)(buf + *off);
       a->rta_type = type;
       a->rta_len = RTA_LENGTH(len);
       if (len) memcpy(RTA_DATA(a), data, len);
       *off += RTA_ALIGN(a->rta_len);
       return a;
}

static struct rtattr *rta_nest(char *buf, int *off, int type) {
       struct rtattr *a = (struct rtattr *)(buf + *off);
       a->rta_type = type | NLA_F_NESTED;
       *off += RTA_LENGTH(0);
       return a;
}

static void rta_nest_end(char *buf, int *off, struct rtattr *start) {
       start->rta_len = (buf + *off) - (char *)start; }

static void create_veth(int fd, const char *n0, const char *n1, int peer_nsfd) {
       char buf[2048];
       memset(buf, 0, sizeof(buf));
       struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
       struct ifinfomsg *ifi = NLMSG_DATA(nlh);
       int off = NLMSG_HDRLEN + sizeof(*ifi);

       nlh->nlmsg_type = RTM_NEWLINK;
       nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE |
NLM_F_EXCL;
       nlh->nlmsg_seq = 10;
       ifi->ifi_family = AF_UNSPEC;

       rta_put(buf, &off, IFLA_IFNAME, n0, strlen(n0) + 1);
       struct rtattr *linfo = rta_nest(buf, &off, IFLA_LINKINFO);
       rta_put(buf, &off, IFLA_INFO_KIND, "veth", 5);
       struct rtattr *idata = rta_nest(buf, &off, IFLA_INFO_DATA);
       struct rtattr *peer = rta_nest(buf, &off, VETH_INFO_PEER);
       struct ifinfomsg *pifi = (struct ifinfomsg *)(buf + off);
       memset(pifi, 0, sizeof(*pifi));
       off += sizeof(*pifi);
       rta_put(buf, &off, IFLA_IFNAME, n1, strlen(n1) + 1);
       rta_put(buf, &off, IFLA_NET_NS_FD, &peer_nsfd, sizeof(peer_nsfd));
       rta_nest_end(buf, &off, peer);
       rta_nest_end(buf, &off, idata);
       rta_nest_end(buf, &off, linfo);
       nlh->nlmsg_len = off;

       if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send veth"); exit(1); }
       int e = nl_recv_ack(fd, "create_veth");
       if (e && e != -EEXIST) { fprintf(stderr, "veth create failed:
%d\n", e); exit(1); }
}

static void set_if_up(int fd, const char *name) {
       char buf[512];
       memset(buf, 0, sizeof(buf));
       struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
       struct ifinfomsg *ifi = NLMSG_DATA(nlh);
       int off = NLMSG_HDRLEN + sizeof(*ifi);
       nlh->nlmsg_type = RTM_NEWLINK;
       nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
       nlh->nlmsg_seq = 20;
       ifi->ifi_family = AF_UNSPEC;
       ifi->ifi_index = if_nametoindex(name);
       ifi->ifi_flags = IFF_UP;
       ifi->ifi_change = IFF_UP;
       nlh->nlmsg_len = off;
       if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send up"); exit(1); }
       nl_recv_ack(fd, "set_if_up");
}

static void add_ipv4(int fd, const char *name, const char *ip, int prefix) {
       char buf[512];
       memset(buf, 0, sizeof(buf));
       struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
       struct ifaddrmsg *ifa = NLMSG_DATA(nlh);
       int off = NLMSG_HDRLEN + sizeof(*ifa);
       nlh->nlmsg_type = RTM_NEWADDR;
       nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE |
NLM_F_REPLACE;
       nlh->nlmsg_seq = 30;
       ifa->ifa_family = AF_INET;
       ifa->ifa_prefixlen = prefix;
       ifa->ifa_scope = 0;
       ifa->ifa_index = if_nametoindex(name);

       struct in_addr a;
       inet_pton(AF_INET, ip, &a);
       rta_put(buf, &off, IFA_LOCAL, &a, 4);
       rta_put(buf, &off, IFA_ADDRESS, &a, 4);
       nlh->nlmsg_len = off;
       if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send addr"); exit(1); }
       nl_recv_ack(fd, "add_ipv4");
}

static void tipc_set_node_id(int fd, uint64_t w0, uint64_t w1) {
       char buf[1024];
       memset(buf, 0, sizeof(buf));
       struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
       struct genlmsghdrgenlmsghdr *gh = (struct genlmsghdrgenlmsghdr
*)NLMSG_DATA(nlh);
       int off = NLMSG_HDRLEN + GENL_HDRLEN;
       nlh->nlmsg_type = tipc_family;
       nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
       nlh->nlmsg_seq = 40;
       gh->cmd = TIPC_NL_NET_SET;
       gh->version = 1;

       struct nlattr *net = nla_nest_start(buf, &off, TIPC_NLA_NET);
       nla_put(buf, &off, TIPC_NLA_NET_NODEID, &w0, sizeof(w0));
       nla_put(buf, &off, TIPC_NLA_NET_NODEID_W1, &w1, sizeof(w1));
       nla_nest_end(buf, &off, net);
       nlh->nlmsg_len = off;

       if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send net_set"); exit(1); }
       int e = nl_recv_ack(fd, "tipc_set_node_id");
       if (e) fprintf(stderr, "net_set(nodeid) returned %d\n", e); }

static void put_sockaddr_storage_v4(char *buf, int *off, int type, const char *ip,
int port) {
       struct sockaddr_storage ss;
       memset(&ss, 0, sizeof(ss));
       struct sockaddr_in *sin = (struct sockaddr_in *)&ss;
       sin->sin_family = AF_INET;
       sin->sin_port = htons(port);
       inet_pton(AF_INET, ip, &sin->sin_addr);
       nla_put(buf, off, type, &ss, sizeof(ss)); }

static void tipc_udp_bearer(int fd, int cmd, const char *bname,
                           const char *local_ip, const char *remote_ip) {
       char buf[2048];
       memset(buf, 0, sizeof(buf));
       struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
       struct genlmsghdrgenlmsghdr *gh = (struct genlmsghdrgenlmsghdr
*)NLMSG_DATA(nlh);
       int off = NLMSG_HDRLEN + GENL_HDRLEN;
       nlh->nlmsg_type = tipc_family;
       nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
       nlh->nlmsg_seq = 50;
       gh->cmd = cmd;
       gh->version = 1;

       struct nlattr *bearer = nla_nest_start(buf, &off, TIPC_NLA_BEARER);
       nla_put(buf, &off, TIPC_NLA_BEARER_NAME, bname名称,
strlen(bname名称) + 1);
       if (cmd == TIPC_NL_BEARER_ENABLE) {
               struct nlattr *udp = nla_nest_start(buf, &off,
TIPC_NLA_BEARER_UDP_OPTS);
               put_sockaddr_storage_v4(buf, &off, TIPC_NLA_UDP_LOCAL, local_ip,
TIPC_UDP_PORT);
               put_sockaddr_storage_v4(buf, &off, TIPC_NLA_UDP_REMOTE,
remote_ip, TIPC_UDP_PORT);
               nla_nest_end(buf, &off, udp);
       }
       nla_nest_end(buf, &off, bearer);
       nlh->nlmsg_len = off;

       if (send(fd, buf, nlh->nlmsg_len, 0) < 0) { perror("send bearer"); exit(1); }
       int e = nl_recv_ack(fd, "tipc_udp_bearer");
       if (e) fprintf(stderr, "bearer cmd %d (%s) returned %d\n", cmd,
bname名称, e); }

static int write_file(const char *path, const char *val) {
       int fd = open(path, O_WRONLY);
       if (fd < 0) return -1;
       int r = write(fd, val, strlen(val));
       close(fd);
       return r;
}

static int bindfill_node_scope(int target) {
       int fd = socket(AF_TIPC, SOCK_RDM, 0);
       if (fd < 0) { perror("socket(AF_TIPC)"); exit(1); }
       int ok = 0, fail = 0;
       for (int i = 0; i < target + 1000; i++) {
               struct sockaddr_tipc sa;
               memset(&sa, 0, sizeof(sa));
               sa.family = AF_TIPC;
               sa.addrtype = TIPC_SERVICE_ADDR;
               sa.scope = TIPC_NODE_SCOPE;
               sa.addr.nameseq.type = 100000 + i;
               sa.addr.nameseq.lower = 100000 + i;
               sa.addr.nameseq.upper = 100000 + i;
               if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) == 0) {
                       ok++;
                       fail = 0;
               } else if (++fail > 100) {
                       break;
               }
       }
       fprintf(stderr, "[*] bound %d node-scope publications (cap reached)\n",
ok);
       if (ok < 65000) { fprintf(stderr, "bindfill shortfall\n"); exit(1); }
       return fd;
}

static void child_main(int sync_rd, int sync_wr) {
       char c;
       write(sync_wr, "R", 1);
       if (read(sync_rd, &c, 1) != 1) _exit(1);

       int rfd = nl_open(NETLINK_ROUTE);
       set_if_up(rfd, "lo");
       set_if_up(rfd, "veth1");
       add_ipv4(rfd, "veth1", "10.0.0.2", 24);

       int gfd = nl_open(NETLINK_GENERIC);
       tipc_family = genl_resolve_family(gfd, TIPC_GENL_V2_NAME);
       if (tipc_family < 0) _exit(1);

       int bfd = bindfill_node_scope(65535);
       tipc_set_node_id(gfd, 0x2222222222222222ULL,
0x2222222222222222ULL);
       tipc_udp_bearer(gfd, TIPC_NL_BEARER_ENABLE, "udp:b1", "10.0.0.2",
"10.0.0.1");
       write(sync_wr, "C", 1);

       sleep(35);
       tipc_udp_bearer(gfd, TIPC_NL_BEARER_DISABLE, "udp:b1", NULL, NULL);
       fprintf(stderr, "[child] b1 disabled, keeping it down\n");
       (void)bfd;
       for (;;) pause();
}

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

       uid_t uid = getuid();
       gid_t gid = getgid();

       if (unshare(CLONE_NEWUSER | CLONE_NEWNET) == 0) {
               write_file("/proc/self/setgroups", "deny");
               char m[64];
               snprintf(m, sizeof(m), "0 %d 1", uid);
               write_file("/proc/self/uid_map", m);
               snprintf(m, sizeof(m), "0 %d 1", gid);
               write_file("/proc/self/gid_map", m);
       } else if (unshare(CLONE_NEWNET) < 0) {
               perror("unshare");
               return 1;
       }

       int p2c[2], c2p[2];
       if (pipe(p2c) < 0 || pipe(c2p) < 0) { perror("pipe"); return 1; }

       pid_t pid = fork();
       if (pid == 0) {
               close(p2c[1]);
               close(c2p[0]);
               if (unshare(CLONE_NEWNET) < 0) _exit(1);
               child_main(p2c[0], c2p[1]);
               _exit(0);
       }
       close(p2c[0]);
       close(c2p[1]);

       char c;
       if (read(c2p[0], &c, 1) != 1) { fprintf(stderr, "child not ready\n"); return 1; }

       char nspath[64];
       snprintf(nspath, sizeof(nspath), "/proc/%d/ns/net", pid);
       int nsfd = open(nspath, O_RDONLY);
       if (nsfd < 0) { perror("open ns"); return 1; }

       int rfd = nl_open(NETLINK_ROUTE);
       create_veth(rfd, "veth0", "veth1", nsfd);
       close(nsfd);
       set_if_up(rfd, "lo");
       set_if_up(rfd, "veth0");
       add_ipv4(rfd, "veth0", "10.0.0.1", 24);
       write(p2c[1], "G", 1);

       int gfd = nl_open(NETLINK_GENERIC);
       tipc_family = genl_resolve_family(gfd, TIPC_GENL_V2_NAME);
       if (tipc_family < 0) return 1;
       int bfd = bindfill_node_scope(65535);
       tipc_set_node_id(gfd, 0x1111111111111111ULL,
0x1111111111111111ULL);

       if (read(c2p[0], &c, 1) != 1)
               fprintf(stderr, "child config sync missed\n");
       tipc_udp_bearer(gfd, TIPC_NL_BEARER_ENABLE, "udp:b0", "10.0.0.1",
"10.0.0.2");

       fprintf(stderr, "[*] nodes configured; waiting for stale cleanup\n");
       (void)bfd;
       sleep(400);
       kill(pid, SIGKILL);
       waitpid(pid, NULL, 0);
       fprintf(stderr, "[*] done\n");
       return 0;
}

Do you think tipc_node_cleanup() should go through the same wake logic as
tipc_node_delete()? Happy to test the next version.

Thanks for your test report and reproducer. It will look into the reproducer to fix outstanding issues.

Thanks,
Weiming