mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] net: psp: do not inherit the Rx association on clone
@ 2026-08-29 16:56 Norbert Szetei
  2026-08-29 20:40 ` Daniel Zahka
  0 siblings, 1 reply; 2+ messages in thread
From: Norbert Szetei @ 2026-08-29 16:56 UTC (permalink / raw)
  To: netdev
  Cc: Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
	David S. Miller, Jakub Kicinski, Simon Horman, Daniel Zahka,
	linux-kernel

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@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
---
Reproducer available on request.

 net/core/sock.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/core/sock.c b/net/core/sock.c
index 1ad41904db25..fa60b7494c58 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2494,6 +2494,9 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority,
 #ifdef CONFIG_BPF_SYSCALL
 	RCU_INIT_POINTER(newsk->sk_bpf_storage, NULL);
 #endif
+#if IS_ENABLED(CONFIG_INET_PSP)
+	RCU_INIT_POINTER(newsk->psp_assoc, NULL);
+#endif
 
 	/* SANITY */
 	if (likely(newsk->sk_net_refcnt)) {
-- 
2.55.0


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

* Re: [PATCH net] net: psp: do not inherit the Rx association on clone
  2026-08-29 16:56 [PATCH net] net: psp: do not inherit the Rx association on clone Norbert Szetei
@ 2026-08-29 20:40 ` Daniel Zahka
  0 siblings, 0 replies; 2+ messages in thread
From: Daniel Zahka @ 2026-08-29 20:40 UTC (permalink / raw)
  To: Norbert Szetei, netdev
  Cc: Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
	David S. Miller, Jakub Kicinski, Simon Horman, Daniel Zahka,
	linux-kernel

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@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Norbert Szetei <norbert@doyensec.com>

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.

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

Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>

> ---
> 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`

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

end of thread, other threads:[~2026-08-29 20:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-29 16:56 [PATCH net] net: psp: do not inherit the Rx association on clone Norbert Szetei
2026-08-29 20:40 ` Daniel Zahka

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

all inboxes | Powered by JetHome®