From 43a556b2fd43f2df6dded59c2e26560a27874c24 Mon Sep 17 00:00:00 2001 From: Ali Ahmet Memis Date: Fri, 7 Aug 2026 02:03:44 +0000 Subject: [PATCH] Bluetooth: RFCOMM: take rfcomm_mutex for the deferred setup accept 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: bb23c0ab8246 ("Bluetooth: Add support for deferring RFCOMM connection setup") Cc: stable@vger.kernel.org Signed-off-by: Ali Ahmet Memis Signed-off-by: Luiz Augusto von Dentz --- net/bluetooth/rfcomm/core.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c index 2e8c080b4d9e..9cdfea666a2c 100644 --- a/net/bluetooth/rfcomm/core.c +++ b/net/bluetooth/rfcomm/core.c @@ -1331,7 +1331,10 @@ static struct rfcomm_session *rfcomm_recv_disc(struct rfcomm_session *s, return s; } -void rfcomm_dlc_accept(struct rfcomm_dlc *d) +/* Must be called with rfcomm_mutex held, so that the session cannot be + * unlinked from under us. + */ +static void __rfcomm_dlc_accept(struct rfcomm_dlc *d) { struct sock *sk = d->session->sock->sk; struct l2cap_conn *conn = l2cap_pi(sk)->chan->conn; @@ -1353,6 +1356,21 @@ void rfcomm_dlc_accept(struct rfcomm_dlc *d) rfcomm_send_msc(d->session, 1, d->dlci, d->v24_sig); } +void rfcomm_dlc_accept(struct rfcomm_dlc *d) +{ + rfcomm_lock(); + + /* rfcomm_recv_disc() sets the dlc state to BT_CLOSED before calling + * __rfcomm_dlc_close(), so the RFCOMM_DEFER_SETUP handshake there is + * skipped and the session can already be unlinked by the time the + * deferred accept runs from rfcomm_sock_recvmsg(). + */ + if (d->session) + __rfcomm_dlc_accept(d); + + rfcomm_unlock(); +} + static void rfcomm_check_accept(struct rfcomm_dlc *d) { if (rfcomm_check_security(d)) { @@ -1365,7 +1383,7 @@ static void rfcomm_check_accept(struct rfcomm_dlc *d) d->state_change(d, 0); rfcomm_dlc_unlock(d); } else - rfcomm_dlc_accept(d); + __rfcomm_dlc_accept(d); } else { set_bit(RFCOMM_AUTH_PENDING, &d->flags); rfcomm_dlc_set_timer(d, RFCOMM_AUTH_TIMEOUT); @@ -1958,7 +1976,7 @@ static void rfcomm_process_dlcs(struct rfcomm_session *s) d->state_change(d, 0); rfcomm_dlc_unlock(d); } else - rfcomm_dlc_accept(d); + __rfcomm_dlc_accept(d); } continue; } else if (test_and_clear_bit(RFCOMM_AUTH_REJECT, &d->flags)) {