[PATCH net] net: x25: fix use-after-free in x25_kill_by_neigh()

[PATCH net] net: x25: fix use-after-free in x25_kill_by_neigh()

[Ibrahim Hashimov]

x25_kill_by_neigh() walks x25_list under x25_list_lock and, for each
socket whose neighbour matches the one going down, drops the list lock,
calls lock_sock()/x25_disconnect()/release_sock() on the socket
(x25_disconnect() can sleep and must not be called with a bh-disabled
spinlock held), and then re-acquires x25_list_lock before continuing the
sk_for_each() walk. No reference is taken on the socket before the list
lock is dropped.

A concurrent close() of that same socket runs x25_release() ->
__x25_destroy_socket() -> x25_remove_socket() (unlinks it from x25_list)
-> eventually the final sock_put(), which frees the kmalloc-2k sock
object. If this happens while x25_kill_by_neigh() has dropped
x25_list_lock, both the lock_sock(s) call right after the unlock and,
once x25_list_lock is re-taken, the sk_for_each() walk's implicit read of
s->sk_node.next can dereference the freed socket.

Reproduced on a v6.19 KASAN kernel under -smp 4 with a killer thread
free-running NETDEV_DOWN toggles (driving x25_device_event() ->
x25_kill_by_neigh()) against several threads closing/recreating
neighbour-bound AF_X25 sockets: KASAN slab-use-after-free in
x25_kill_by_neigh()+0xfd/0x110, "Read of size 8" at offset 104 into a
freed kmalloc-2k object -- exactly the sk->sk_node.next field of the
socket concurrently freed by close(). 145 splats fired over roughly
64,400 kill rounds in the racing configuration; a serialized control run
(single kill, sockets closed sequentially, no concurrent free) produced
zero KASAN reports. With this patch applied the same free-running
reproducer no longer triggers the report.

Fix this the same way the rest of net/x25 protects a socket found by
walking x25_list under x25_list_lock (see x25_find_listener() and
__x25_find_socket(), which sock_hold() the socket before dropping the
list lock): take a reference on the socket before dropping x25_list_lock,
and release it with sock_put() once lock_sock() / x25_disconnect() /
release_sock() are done. Since a concurrent x25_remove_socket() can still
unlink the socket from x25_list (and reinitialize its list node) while
the lock is dropped, simply re-acquiring x25_list_lock and resuming the
sk_for_each() walk from the old s is not safe either way, so restart the
scan from the head of x25_list instead of trying to resume it.
x25_disconnect() clears x25_sk(s)->neighbour, so the just-handled socket
will not match nb again and the restarted scan makes forward progress on
each pass.

Fixes: 7781607938c8 ("net/x25: Fix null-ptr-deref caused by x25_disconnect")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
 net/x25/af_x25.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index c31d2af5dd22..725d35596e3f 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1768,15 +1768,18 @@ void x25_kill_by_neigh(struct x25_neigh *nb)
 {
 	struct sock *s;
 
+restart:
 	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);
 			release_sock(s);
-			write_lock_bh(&x25_list_lock);
+			sock_put(s);
+			goto restart;
 		}
 	}
 	write_unlock_bh(&x25_list_lock);
-- 
2.50.1 (Apple Git-155)