Skip to content

Commit eb71d5a

Browse files
lrfegregkh
authored andcommitted
Bluetooth: RFCOMM: take rfcomm_mutex for the deferred setup accept
commit 43a556b upstream. rfcomm_sock_recvmsg() completes a deferred setup by calling rfcomm_dlc_accept() without holding any RFCOMM lock: if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) { rfcomm_dlc_accept(d); return 0; } and rfcomm_dlc_accept() dereferences the session on its first line: struct sock *sk = d->session->sock->sk; Every other path that touches d->session runs under rfcomm_mutex: rfcomm_dlc_open(), rfcomm_dlc_close(), rfcomm_dlc_exists(), rfcomm_dlc_send_rpn(), and the RFCOMM thread through rfcomm_process_sessions(). rfcomm_connect_ind() is even documented as "called under rfcomm_lock()". This call site is the only one that skips it. The RFCOMM_DEFER_SETUP bit looks like it serialises the accept against teardown, since __rfcomm_dlc_close() returns early when it wins the test_and_clear. But rfcomm_recv_disc() forces the state first: d->state = BT_CLOSED; __rfcomm_dlc_close(d, err); and the early return only covers BT_CONNECT, BT_CONFIG, BT_OPEN and BT_CONNECT2. With the state already BT_CLOSED that switch does not match, the bit is never consulted, and __rfcomm_dlc_close() falls through to rfcomm_dlc_unlink(), which sets d->session = NULL. So a remote DISC on a deferred dlc clears the session while leaving RFCOMM_DEFER_SETUP set. The next recvmsg() then passes the test_and_clear and dereferences a NULL session. No timing window is needed: once the DISC has been processed, the dereference is unconditional. Give rfcomm_dlc_accept() the same shape as rfcomm_dlc_open() and rfcomm_dlc_close(): an exported wrapper that takes rfcomm_mutex and re-checks the session, around a __rfcomm_dlc_accept() that the two in-core callers, which already hold the mutex, keep using. Reproduced on a KASAN + PROVE_LOCKING kernel with a BR/EDR peer emulated over /dev/vhci: the peer brings up an ACL link, opens L2CAP on the RFCOMM PSM, starts a session, opens a dlc on a channel bound with BT_DEFER_SETUP, and sends DISC after the socket is accepted. recv() on the accepted socket then hits: Oops: general protection fault KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017] RIP: 0010:rfcomm_dlc_accept+0x54/0x350 Call Trace: rfcomm_sock_recvmsg+0x1cd/0x230 sock_recvmsg+0x166/0x1c0 __sys_recvfrom+0x20d/0x300 0x10 is the offset of sock in struct rfcomm_session. With this patch the same run completes with recv() returning 0 and no report, and lockdep stays quiet, confirming rfcomm_mutex is still taken before lock_sock on this path as it is on the thread side. Fixes: bb23c0a ("Bluetooth: Add support for deferring RFCOMM connection setup") Cc: stable@vger.kernel.org Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 5e52eb0 commit eb71d5a

1 file changed

Lines changed: 21 additions & 3 deletions

File tree

net/bluetooth/rfcomm/core.c

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,7 +1330,10 @@ static struct rfcomm_session *rfcomm_recv_disc(struct rfcomm_session *s,
13301330
return s;
13311331
}
13321332

1333-
void rfcomm_dlc_accept(struct rfcomm_dlc *d)
1333+
/* Must be called with rfcomm_mutex held, so that the session cannot be
1334+
* unlinked from under us.
1335+
*/
1336+
static void __rfcomm_dlc_accept(struct rfcomm_dlc *d)
13341337
{
13351338
struct sock *sk = d->session->sock->sk;
13361339
struct l2cap_conn *conn = l2cap_pi(sk)->chan->conn;
@@ -1352,6 +1355,21 @@ void rfcomm_dlc_accept(struct rfcomm_dlc *d)
13521355
rfcomm_send_msc(d->session, 1, d->dlci, d->v24_sig);
13531356
}
13541357

1358+
void rfcomm_dlc_accept(struct rfcomm_dlc *d)
1359+
{
1360+
rfcomm_lock();
1361+
1362+
/* rfcomm_recv_disc() sets the dlc state to BT_CLOSED before calling
1363+
* __rfcomm_dlc_close(), so the RFCOMM_DEFER_SETUP handshake there is
1364+
* skipped and the session can already be unlinked by the time the
1365+
* deferred accept runs from rfcomm_sock_recvmsg().
1366+
*/
1367+
if (d->session)
1368+
__rfcomm_dlc_accept(d);
1369+
1370+
rfcomm_unlock();
1371+
}
1372+
13551373
static void rfcomm_check_accept(struct rfcomm_dlc *d)
13561374
{
13571375
if (rfcomm_check_security(d)) {
@@ -1364,7 +1382,7 @@ static void rfcomm_check_accept(struct rfcomm_dlc *d)
13641382
d->state_change(d, 0);
13651383
rfcomm_dlc_unlock(d);
13661384
} else
1367-
rfcomm_dlc_accept(d);
1385+
__rfcomm_dlc_accept(d);
13681386
} else {
13691387
set_bit(RFCOMM_AUTH_PENDING, &d->flags);
13701388
rfcomm_dlc_set_timer(d, RFCOMM_AUTH_TIMEOUT);
@@ -1949,7 +1967,7 @@ static void rfcomm_process_dlcs(struct rfcomm_session *s)
19491967
d->state_change(d, 0);
19501968
rfcomm_dlc_unlock(d);
19511969
} else
1952-
rfcomm_dlc_accept(d);
1970+
__rfcomm_dlc_accept(d);
19531971
}
19541972
continue;
19551973
} else if (test_and_clear_bit(RFCOMM_AUTH_REJECT, &d->flags)) {

0 commit comments

Comments
 (0)