linux-kernel - Possible sleep-in-atomic in BT SCO code

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <s5hilz8n1an.wl-tiwai@suse.de>
Date:   Fri, 10 Sep 2021 13:00:00 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     linux-bluetooth@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, Nicolai Stange <nstange@...e.de>
Subject: Possible sleep-in-atomic in BT SCO code

Hi,

while investigation of the recent BT fixes, Nicolai found out that the
change in the commit 27c24fda62b6 ("Bluetooth: switch to lock_sock in
SCO") may cause a sleep-in-atomic.

The commit replaced bh_lock_sock() with lock_sock(), which can sleep.
Meanwhile, in sco_conn_ready(), this is called after sco_conn_lock(),
and sco_conn_lock() is a simple spinlock.  So this may lead to a
sleep-in-atomic.

I can imagine a fix like the below, but this also made us wonder
whether the sco_conn_lock() would be needed at all.  In the code path,
conn->hcon won't be changed, right?

thanks,

Takashi

--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -1118,18 +1118,22 @@ static void sco_conn_ready(struct sco_conn *conn)
 			return;
 		}

+		sock_hold(parent);
+		sco_conn_unlock(conn);
+
 		lock_sock(parent);

 		sk = sco_sock_alloc(sock_net(parent), NULL,
 				    BTPROTO_SCO, GFP_ATOMIC, 0);
 		if (!sk) {
 			release_sock(parent);
-			sco_conn_unlock(conn);
+			sock_put(parent);
 			return;
 		}

 		sco_sock_init(sk, parent);

+		sco_conn_lock(conn);
 		bacpy(&sco_pi(sk)->src, &conn->hcon->src);
 		bacpy(&sco_pi(sk)->dst, &conn->hcon->dst);

@@ -1143,10 +1147,10 @@ static void sco_conn_ready(struct sco_conn *conn)

 		/* Wake up parent */
 		parent->sk_data_ready(parent);
+		sco_conn_unlock(conn);

 		release_sock(parent);
-
-		sco_conn_unlock(conn);
+		sock_put(parent);
 	}
 }