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

[Martin Schiller <ms@dev.tdt.de>]

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>