Netdev List
 help / color / mirror / Atom feed
* [PATCH net] net/x25: fix use-after-free in x25_kill_by_neigh()
@ 2026-07-13 10:47 David Lee
  2026-07-14  6:42 ` Martin Schiller
  0 siblings, 1 reply; 2+ messages in thread
From: David Lee @ 2026-07-13 10:47 UTC (permalink / raw)
  To: Martin Schiller
  Cc: David Lee, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman,
	Dominik 'Disconnect3d' Czarnota, linux-x25, netdev,
	linux-kernel, stable

x25_kill_by_neigh() walks the global X.25 socket list looking for sockets
attached to a terminating neighbour. x25_list_lock protects list membership
while the lookup is in progress, but it does not pin a socket's lifetime
after the lock is dropped.

The function currently drops x25_list_lock before calling lock_sock(s). A
concurrent close can run x25_release(), remove the same socket from
x25_list, and drop the last socket reference in that window. The neighbour
teardown path can then lock or inspect a freed struct sock/struct x25_sock.

Take sock_hold(s) while x25_list_lock still proves that the list entry is
live, then drop the temporary reference after the socket has been locked,
rechecked, and released. Recheck x25_sk(s)->neighbour after lock_sock(),
because another path may have disconnected the socket before this path
acquired the socket lock. Restart the list walk after each disconnect
because the list lock was dropped and the previous iterator state may no
longer be valid.

A QEMU/KASAN run against origin/master reproduced a slab-use-after-free in
x25_kill_by_neigh().

Fixes: 7781607938c8 ("net/x25: Fix null-ptr-deref caused by x25_disconnect")
Cc: stable@vger.kernel.org
Signed-off-by: David Lee <david.lee@trailofbits.com>
Assisted-by: Codex:gpt-5.5
---
Trail of Bits has a reproducer that triggers kernel panic demonstrating the bug, and can share it if needed.

net/x25/af_x25.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index c31d2af5dd22..8aae9273b7c1 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1768,15 +1768,19 @@ void x25_kill_by_neigh(struct x25_neigh *nb)
 {
 	struct sock *s;
 
+again:
 	write_lock_bh(&x25_list_lock);
 
 	sk_for_each(s, &x25_list) {
 		if (x25_sk(s)->neighbour == nb) {
+			sock_hold(s);
 			write_unlock_bh(&x25_list_lock);
 			lock_sock(s);
-			x25_disconnect(s, ENETUNREACH, 0, 0);
+			if (x25_sk(s)->neighbour == nb)
+				x25_disconnect(s, ENETUNREACH, 0, 0);
 			release_sock(s);
-			write_lock_bh(&x25_list_lock);
+			sock_put(s);
+			goto again;
 		}
 	}
 	write_unlock_bh(&x25_list_lock);

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

* Re: [PATCH net] net/x25: fix use-after-free in x25_kill_by_neigh()
  2026-07-13 10:47 [PATCH net] net/x25: fix use-after-free in x25_kill_by_neigh() David Lee
@ 2026-07-14  6:42 ` Martin Schiller
  0 siblings, 0 replies; 2+ messages in thread
From: Martin Schiller @ 2026-07-14  6:42 UTC (permalink / raw)
  To: David Lee
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Dominik 'Disconnect3d' Czarnota, linux-x25,
	netdev, linux-kernel, stable

On 2026-07-13 12:47, David Lee wrote:
> x25_kill_by_neigh() walks the global X.25 socket list looking for 
> sockets
> attached to a terminating neighbour. x25_list_lock protects list 
> membership
> while the lookup is in progress, but it does not pin a socket's 
> lifetime
> after the lock is dropped.
> 
> The function currently drops x25_list_lock before calling lock_sock(s). 
> A
> concurrent close can run x25_release(), remove the same socket from
> x25_list, and drop the last socket reference in that window. The 
> neighbour
> teardown path can then lock or inspect a freed struct sock/struct 
> x25_sock.
> 
> Take sock_hold(s) while x25_list_lock still proves that the list entry 
> is
> live, then drop the temporary reference after the socket has been 
> locked,
> rechecked, and released. Recheck x25_sk(s)->neighbour after 
> lock_sock(),
> because another path may have disconnected the socket before this path
> acquired the socket lock. Restart the list walk after each disconnect
> because the list lock was dropped and the previous iterator state may 
> no
> longer be valid.
> 
> A QEMU/KASAN run against origin/master reproduced a slab-use-after-free 
> in
> x25_kill_by_neigh().
> 
> Fixes: 7781607938c8 ("net/x25: Fix null-ptr-deref caused by 
> x25_disconnect")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Lee <david.lee@trailofbits.com>
> Assisted-by: Codex:gpt-5.5
> ---
> Trail of Bits has a reproducer that triggers kernel panic
> demonstrating the bug, and can share it if needed.
> 
> net/x25/af_x25.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> index c31d2af5dd22..8aae9273b7c1 100644
> --- a/net/x25/af_x25.c
> +++ b/net/x25/af_x25.c
> @@ -1768,15 +1768,19 @@ void x25_kill_by_neigh(struct x25_neigh *nb)
>  {
>  	struct sock *s;
> 
> +again:
>  	write_lock_bh(&x25_list_lock);
> 
>  	sk_for_each(s, &x25_list) {
>  		if (x25_sk(s)->neighbour == nb) {
> +			sock_hold(s);
>  			write_unlock_bh(&x25_list_lock);
>  			lock_sock(s);
> -			x25_disconnect(s, ENETUNREACH, 0, 0);
> +			if (x25_sk(s)->neighbour == nb)
> +				x25_disconnect(s, ENETUNREACH, 0, 0);
>  			release_sock(s);
> -			write_lock_bh(&x25_list_lock);
> +			sock_put(s);
> +			goto again;
>  		}
>  	}
>  	write_unlock_bh(&x25_list_lock);

LGTM, Thanks.

Acked-by: Martin Schiller <ms@dev.tdt.de>

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

end of thread, other threads:[~2026-07-14  6:42 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-13 10:47 [PATCH net] net/x25: fix use-after-free in x25_kill_by_neigh() David Lee
2026-07-14  6:42 ` Martin Schiller

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