Netdev List
 help / color / mirror / Atom feed
* [PATCH v2] atm: fix use-after-free in sigd_put_skb()
@ 2026-06-09 16:21 Weiming Shi
  2026-06-11 16:26 ` Weiming Shi
  0 siblings, 1 reply; 2+ messages in thread
From: Weiming Shi @ 2026-06-09 16:21 UTC (permalink / raw)
  To: Chas Williams, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Simon Horman, linux-atm-general, netdev, linux-kernel, Xiang Mei,
	Weiming Shi

sigd_put_skb() delivers a signalling message to the daemon socket named
by the global @sigd pointer, ending in a call to sk_data_ready(). It
reads @sigd with no synchronisation, so it can race with a close of the
daemon socket: sigd_close() clears @sigd and the socket is then torn
down and freed.

Holding a reference on the socket is not enough to make this safe. The
daemon fd close runs __sock_release(), which frees the struct socket --
and the wait queue that sk->sk_wq points at -- via iput() once
->release() has returned. sk_data_ready() (sock_def_readable()) then
dereferences a freed sk_wq:

 KASAN: null-ptr-deref in range [0x0000000000000188-0x000000000000018f]
 RIP: 0010:sigd_put_skb (net/atm/signaling.c:65)
  sigd_enq2 (net/atm/signaling.c:228)
  sigd_enq (net/atm/signaling.c:237)
  svc_bind (net/atm/svc.c:135)
  __sys_bind
  __x64_sys_bind
  do_syscall_64

Fix it on both sides. sigd_close() now calls sock_orphan(), which under
sk_callback_lock sets SOCK_DEAD and clears sk_wq before the socket is
freed. sigd_put_skb() latches @sigd with READ_ONCE(), pins the socket
with find_get_vcc(), and then takes sk_callback_lock; it delivers only
while the socket is still the signalling daemon and not yet SOCK_DEAD,
otherwise it drops the skb. sk_callback_lock is used rather than
lock_sock() because sigd_put_skb() can be reached from vcc_sendmsg() ->
sigd_send(), which already holds lock_sock() on the daemon socket.

The READ_ONCE(sigd) != vcc check also rejects a reused VCC: a caller
preempted between latching @sigd and validating it in find_get_vcc()
could otherwise match a freed-and-reallocated, live (non-SOCK_DEAD) VCC.
This needs a concurrent (privileged) daemon teardown, so it is hardening.

Triggering the use-after-free requires CAP_NET_ADMIN and CAP_SYS_RAWIO
to attach the daemon.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Xiang Mei <xmei5@asu.edu>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
v2:
 - Also bail when READ_ONCE(sigd) != vcc, rejecting a reused VCC that a
   preempted caller could match in find_get_vcc() (hardening, no stable).

 net/atm/signaling.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/net/atm/signaling.c b/net/atm/signaling.c
index 358fbe5e4d1d0..346fb969166e8 100644
--- a/net/atm/signaling.c
+++ b/net/atm/signaling.c
@@ -54,14 +54,32 @@ static struct atm_vcc *find_get_vcc(struct atm_vcc *vcc)
 
 static void sigd_put_skb(struct sk_buff *skb)
 {
-	if (!sigd) {
+	struct atm_vcc *vcc;
+	struct sock *sk;
+
+	vcc = find_get_vcc(READ_ONCE(sigd));
+	if (!vcc) {
 		pr_debug("atmsvc: no signaling daemon\n");
 		kfree_skb(skb);
 		return;
 	}
-	atm_force_charge(sigd, skb->truesize);
-	skb_queue_tail(&sk_atm(sigd)->sk_receive_queue, skb);
-	sk_atm(sigd)->sk_data_ready(sk_atm(sigd));
+	sk = sk_atm(vcc);
+
+	/* Pairs with sock_orphan() in sigd_close(). */
+	read_lock_bh(&sk->sk_callback_lock);
+	/* Bail if torn down, or if the slot was reused by another VCC. */
+	if (sock_flag(sk, SOCK_DEAD) || READ_ONCE(sigd) != vcc) {
+		read_unlock_bh(&sk->sk_callback_lock);
+		sock_put(sk);
+		kfree_skb(skb);
+		return;
+	}
+	atm_force_charge(vcc, skb->truesize);
+	skb_queue_tail(&sk->sk_receive_queue, skb);
+	sk->sk_data_ready(sk);
+	read_unlock_bh(&sk->sk_callback_lock);
+
+	sock_put(sk);
 }
 
 static void modify_qos(struct atm_vcc *vcc, struct atmsvc_msg *msg)
@@ -257,6 +275,9 @@ static void sigd_close(struct atm_vcc *vcc)
 		pr_err("closing with requests pending\n");
 	skb_queue_purge(&sk_atm(vcc)->sk_receive_queue);
 
+	/* Make a concurrent sigd_put_skb() observe SOCK_DEAD and bail. */
+	sock_orphan(sk_atm(vcc));
+
 	read_lock(&vcc_sklist_lock);
 	for (i = 0; i < VCC_HTABLE_SIZE; ++i) {
 		struct hlist_head *head = &vcc_hash[i];
-- 
2.43.0


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

* Re: [PATCH v2] atm: fix use-after-free in sigd_put_skb()
  2026-06-09 16:21 [PATCH v2] atm: fix use-after-free in sigd_put_skb() Weiming Shi
@ 2026-06-11 16:26 ` Weiming Shi
  0 siblings, 0 replies; 2+ messages in thread
From: Weiming Shi @ 2026-06-11 16:26 UTC (permalink / raw)
  To: Weiming Shi, Chas Williams, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, linux-atm-general, netdev, linux-kernel, Xiang Mei

On Wed Jun 10, 2026 at 12:21 AM CST, Weiming Shi wrote:
> sigd_put_skb() delivers a signalling message to the daemon socket named
> by the global @sigd pointer, ending in a call to sk_data_ready(). It
> reads @sigd with no synchronisation, so it can race with a close of the
> daemon socket: sigd_close() clears @sigd and the socket is then torn
> down and freed.
>
> Holding a reference on the socket is not enough to make this safe. The
> daemon fd close runs __sock_release(), which frees the struct socket --
> and the wait queue that sk->sk_wq points at -- via iput() once
> ->release() has returned. sk_data_ready() (sock_def_readable()) then
> dereferences a freed sk_wq:
>
>  KASAN: null-ptr-deref in range [0x0000000000000188-0x000000000000018f]
>  RIP: 0010:sigd_put_skb (net/atm/signaling.c:65)
>   sigd_enq2 (net/atm/signaling.c:228)
>   sigd_enq (net/atm/signaling.c:237)
>   svc_bind (net/atm/svc.c:135)
>   __sys_bind
>   __x64_sys_bind
>   do_syscall_64
>
> Fix it on both sides. sigd_close() now calls sock_orphan(), which under
> sk_callback_lock sets SOCK_DEAD and clears sk_wq before the socket is
> freed. sigd_put_skb() latches @sigd with READ_ONCE(), pins the socket
> with find_get_vcc(), and then takes sk_callback_lock; it delivers only
> while the socket is still the signalling daemon and not yet SOCK_DEAD,
> otherwise it drops the skb. sk_callback_lock is used rather than
> lock_sock() because sigd_put_skb() can be reached from vcc_sendmsg() ->
> sigd_send(), which already holds lock_sock() on the daemon socket.
>
> The READ_ONCE(sigd) != vcc check also rejects a reused VCC: a caller
> preempted between latching @sigd and validating it in find_get_vcc()
> could otherwise match a freed-and-reallocated, live (non-SOCK_DEAD) VCC.
> This needs a concurrent (privileged) daemon teardown, so it is hardening.
>
> Triggering the use-after-free requires CAP_NET_ADMIN and CAP_SYS_RAWIO
> to attach the daemon.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
> v2:
>  - Also bail when READ_ONCE(sigd) != vcc, rejecting a reused VCC that a
>    preempted caller could match in find_get_vcc() (hardening, no stable).
>
>  net/atm/signaling.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/net/atm/signaling.c b/net/atm/signaling.c
> index 358fbe5e4d1d0..346fb969166e8 100644
> --- a/net/atm/signaling.c
> +++ b/net/atm/signaling.c
> @@ -54,14 +54,32 @@ static struct atm_vcc *find_get_vcc(struct atm_vcc *vcc)
>  
>  static void sigd_put_skb(struct sk_buff *skb)
>  {
> -	if (!sigd) {
> +	struct atm_vcc *vcc;
> +	struct sock *sk;
> +
> +	vcc = find_get_vcc(READ_ONCE(sigd));
> +	if (!vcc) {
>  		pr_debug("atmsvc: no signaling daemon\n");
>  		kfree_skb(skb);
>  		return;
>  	}
> -	atm_force_charge(sigd, skb->truesize);
> -	skb_queue_tail(&sk_atm(sigd)->sk_receive_queue, skb);
> -	sk_atm(sigd)->sk_data_ready(sk_atm(sigd));
> +	sk = sk_atm(vcc);
> +
> +	/* Pairs with sock_orphan() in sigd_close(). */
> +	read_lock_bh(&sk->sk_callback_lock);
> +	/* Bail if torn down, or if the slot was reused by another VCC. */
> +	if (sock_flag(sk, SOCK_DEAD) || READ_ONCE(sigd) != vcc) {
> +		read_unlock_bh(&sk->sk_callback_lock);
> +		sock_put(sk);
> +		kfree_skb(skb);
> +		return;
> +	}
> +	atm_force_charge(vcc, skb->truesize);
> +	skb_queue_tail(&sk->sk_receive_queue, skb);
> +	sk->sk_data_ready(sk);
> +	read_unlock_bh(&sk->sk_callback_lock);
> +
> +	sock_put(sk);
>  }
>  
>  static void modify_qos(struct atm_vcc *vcc, struct atmsvc_msg *msg)
> @@ -257,6 +275,9 @@ static void sigd_close(struct atm_vcc *vcc)
>  		pr_err("closing with requests pending\n");
>  	skb_queue_purge(&sk_atm(vcc)->sk_receive_queue);
>  
> +	/* Make a concurrent sigd_put_skb() observe SOCK_DEAD and bail. */
> +	sock_orphan(sk_atm(vcc));
> +
>  	read_lock(&vcc_sklist_lock);
>  	for (i = 0; i < VCC_HTABLE_SIZE; ++i) {
>  		struct hlist_head *head = &vcc_hash[i];


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

end of thread, other threads:[~2026-06-11 16:27 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-09 16:21 [PATCH v2] atm: fix use-after-free in sigd_put_skb() Weiming Shi
2026-06-11 16:26 ` Weiming Shi

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