| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9430e38fd2fb34abfac0d90bc84880667749294d.camel@iki.fi>
Date: Sat, 01 Nov 2025 17:56:20 +0200
From: Pauli Virtanen <pav@....fi>
To: Cen Zhang <zzzccc427@....com>, Luiz Augusto von Dentz
<luiz.dentz@...il.com>, johan.hedberg@...il.com, marcel@...tmann.org
Cc: linux-kernel@...r.kernel.org, linux-bluetooth@...r.kernel.org,
baijiaju1990@...il.com, r33s3n6@...il.com, gality369@...il.com,
zhenghaoran154@...il.com
Subject: Re: [PATCH v3] bluetooth: sco: Serialize state check in
sco_sock_connect to fix UAF
Hi,
la, 2025-11-01 kello 10:45 +0000, Cen Zhang kirjoitti:
> Concurrent sco_sock_connect() calls could race on the same socket since the
> state checks (BT_OPEN/BT_BOUND) were done without holding the socket lock.
> This allowed two parallel connects to proceed and end up binding two
> separate sco_conn objects to the same sk. Later, when sk->conn had been
> updated to point to the second conn, closing the socket could free the
> second conn and the socket, while the first conn's connect confirm path
> still referenced the stale sk/conn, triggering a KASAN use-after-free.
>
> Fix by taking lock_sock(sk) before checking sk->sk_state and sk->sk_type,
> performing the destination address assignment under the lock, and releasing
> it before invoking sco_connect() (which will acquire the lock as needed).
> This serializes concurrent connect attempts for the same sk and prevents the
> interleaving that caused the double-attachment and subsequent UAF.
>
> Thread 1: Thread 2: Thread3:
> check sk_state check sk_state
> sco_sock_connect(sk) sco_sock_connect(sk) sco_connect_cfm(sk->conn)
> conn1->sk = sk
> conn2->sk = sk
> sk->conn = conn1
> sk->conn = conn2
> sco_sock_release
> free conn2 and sk
> sco_connect_cfm
> sco_conn_del
> sco_conn_free
> UAF on sk
>
> The representative KASAN report excerpt:
>
> BUG: KASAN: slab-use-after-free in sco_conn_free net/bluetooth/sco.c:94
> ...
> Write of size 8 at addr ffff88810d2be350 by task kworker/u25:1/88
> ...
> Call Trace:
> sco_conn_free net/bluetooth/sco.c:94 [inline]
> kref_put include/linux/kref.h:65 [inline]
> sco_conn_put+0x49d/0xfc0 net/bluetooth/sco.c:115
> sco_conn_del+0x46d/0x8d0 net/bluetooth/sco.c:280
> sco_connect_cfm+0x83d/0x1ee0 net/bluetooth/sco.c:1468
> hci_connect_cfm include/net/bluetooth/hci_core.h:2082 [inline]
> ...
> Allocated by task 294:
> ...
> sco_sock_create+0x22d/0xc00 net/bluetooth/sco.c:616
> ...
> Freed by task 295:
> __sk_destruct+0x4b0/0x630 net/core/sock.c:2373
> sock_put include/net/sock.h:1962 [inline]
> sco_sock_kill+0x64d/0x9b0 net/bluetooth/sco.c:526
> sco_sock_release+0x770/0xa50 net/bluetooth/sco.c:1359
> ...
>
> Reported-by: Cen Zhang <zzzccc427@....com>
> Signed-off-by: Cen Zhang <zzzccc427@....com>
>
> ---
> v2 and v3:
> - Fix the patch format
> ---
>
> ---
> net/bluetooth/sco.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index ab0cf442d..0af266880 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -651,13 +651,18 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
> addr->sa_family != AF_BLUETOOTH)
> return -EINVAL;
>
> - if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND)
> + lock_sock(sk);
> +
> + if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND){
> + release_sock(sk);
> return -EBADFD;
> + }
>
> - if (sk->sk_type != SOCK_SEQPACKET)
> - err = -EINVAL;
> + if (sk->sk_type != SOCK_SEQPACKET){
> + release_sock(sk);
> + return -EINVAL;
> + }
>
> - lock_sock(sk);
> /* Set destination address and psm */
> bacpy(&sco_pi(sk)->dst, &sa->sco_bdaddr);
> release_sock(sk);
This looks correct in principle, sk_state shall be accessed only under
lock.
However, the lock is released before sco_connect, so looks like two
connect calls can still be at this point at the same time, so AFAICS
the above only restricts the time window for the race.
Probably something along the following is more sure. It's important the
check is under same lock_sock() in sco_connect where sk_state is
modified; in addition sk_state check could be added (or maybe moved)
also there.
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index ab0cf442d57b..06c20d99521d 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -298,7 +298,7 @@ static int sco_chan_add(struct sco_conn *conn,
struct sock *sk,
int err = 0;
sco_conn_lock(conn);
- if (conn->sk)
+ if (conn->sk || sco_pi(sk)->conn)
err = -EBUSY;
else
__sco_chan_add(conn, sk, parent);
@@ -356,6 +356,7 @@ static int sco_connect(struct sock *sk)
err = sco_chan_add(conn, sk, NULL);
if (err) {
release_sock(sk);
+ hci_conn_drop(hcon);
goto unlock;
}
The test bot also says:
"Bluetooth: " prefix is not specified in the subject
The patch subject should start "Bluetooth: SCO:" not "bluetooth: sco:".
The following errors in test bot afaik are known pre-existing failures,
and can be ignored here:
Failed Test Cases
Read Exp Feature - Success Failed 0.102
seconds
LL Privacy - Add Device 3 (AL is full) Failed 0.196
seconds
Mesh - Send cancel - 1 Timed out 2.022
seconds
Mesh - Send cancel - 2 Timed out 1.996
seconds
--
Pauli Virtanen
Powered by blists - more mailing lists