Commit 56f0aa7
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 baf9b03 commit 56f0aa7
1 file changed
Lines changed: 21 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1330 | 1330 | | |
1331 | 1331 | | |
1332 | 1332 | | |
1333 | | - | |
| 1333 | + | |
| 1334 | + | |
| 1335 | + | |
| 1336 | + | |
1334 | 1337 | | |
1335 | 1338 | | |
1336 | 1339 | | |
| |||
1352 | 1355 | | |
1353 | 1356 | | |
1354 | 1357 | | |
| 1358 | + | |
| 1359 | + | |
| 1360 | + | |
| 1361 | + | |
| 1362 | + | |
| 1363 | + | |
| 1364 | + | |
| 1365 | + | |
| 1366 | + | |
| 1367 | + | |
| 1368 | + | |
| 1369 | + | |
| 1370 | + | |
| 1371 | + | |
| 1372 | + | |
1355 | 1373 | | |
1356 | 1374 | | |
1357 | 1375 | | |
| |||
1364 | 1382 | | |
1365 | 1383 | | |
1366 | 1384 | | |
1367 | | - | |
| 1385 | + | |
1368 | 1386 | | |
1369 | 1387 | | |
1370 | 1388 | | |
| |||
1949 | 1967 | | |
1950 | 1968 | | |
1951 | 1969 | | |
1952 | | - | |
| 1970 | + | |
1953 | 1971 | | |
1954 | 1972 | | |
1955 | 1973 | | |
| |||
0 commit comments