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);
}
}