mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
Bluetooth: L2CAP: refuse __l2cap_chan_add if chan already has conn
l2cap_chan may be linked to l2cap_conn at most once. This is assumed in
several places, eg l2cap_chan_del cleanup.
There is a TOCTOU race where the invariant is violated:
[Task 1] [Task 2]
l2cap_chan_connect l2cap_sock_bind
l2cap_chan_lock lock_sock
l2cap_state_change if (sk->sk_state != BT_OPEN)
chan->state = BT_CONNECT
l2cap_sock_state_change_cb chan->state = BT_BOUND
sk->sk_state = BT_BOUND
lock_sock <------------------ release_sock
sk->sk_state = BT_CONNECT
l2cap_sock_connect() does not check sk->sk_state, so since chan->state
is now BT_BOUND, subsequent connect() ends up with second
__l2cap_chan_add.
Explicitly document and check the invariant in __l2cap_chan_add with
WARN_ON_ONCE. The only callsite where it could be hit is
l2cap_chan_connect, so add pre-check there to avoid relying on
chan->state. chan->state read/write is not properly guarded currently so
there can be other TOCTOUC problems.
Add l2cap_lock_chan in l2cap_sock_bind() to guard chan->state write.
Fixes: b66774b48d ("Bluetooth: L2CAP: Fix UAF in channel timeout by holding conn ref")
Assisted-by: deepseek-4-flash # finding the race condition
Signed-off-by: Pauli Virtanen <pav@iki.fi>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This commit is contained in:
parent
a0c672c410
commit
86b55991d3
|
|
@ -623,6 +623,10 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
|
|||
BT_DBG("conn %p, psm 0x%2.2x, dcid 0x%4.4x", conn,
|
||||
__le16_to_cpu(chan->psm), chan->dcid);
|
||||
|
||||
/* Caller must ensure l2cap_chan is linked to l2cap_conn only once */
|
||||
if (WARN_ON_ONCE(chan->conn || test_bit(FLAG_DEL, &chan->flags)))
|
||||
return;
|
||||
|
||||
conn->disc_reason = HCI_ERROR_REMOTE_USER_TERM;
|
||||
|
||||
chan->conn = l2cap_conn_get(conn);
|
||||
|
|
@ -7631,7 +7635,8 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
|
|||
}
|
||||
}
|
||||
|
||||
if (cid && __l2cap_get_chan_by_dcid(conn, cid)) {
|
||||
if ((cid && __l2cap_get_chan_by_dcid(conn, cid)) || chan->conn ||
|
||||
test_bit(FLAG_DEL, &chan->flags)) {
|
||||
hci_conn_drop(hcon);
|
||||
err = -EBUSY;
|
||||
goto chan_unlock;
|
||||
|
|
|
|||
|
|
@ -109,6 +109,7 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, i
|
|||
return -EINVAL;
|
||||
}
|
||||
|
||||
l2cap_chan_lock(chan);
|
||||
lock_sock(sk);
|
||||
|
||||
if (sk->sk_state != BT_OPEN) {
|
||||
|
|
@ -174,6 +175,7 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, i
|
|||
|
||||
done:
|
||||
release_sock(sk);
|
||||
l2cap_chan_unlock(chan);
|
||||
return err;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user