[PW_SID:1127425] Bluetooth: L2CAP: take chan_l lock in ecred defer recvmsg path to fix list corruption#438
[PW_SID:1127425] Bluetooth: L2CAP: take chan_l lock in ecred defer recvmsg path to fix list corruption#438BluezTestBot wants to merge 1 commit into
Conversation
… list corruption When a deferred L2CAP_MODE_EXT_FLOWCTL connection is accepted, l2cap_sock_recvmsg() (BT_CONNECT2 + BT_SK_DEFER_SETUP branch) calls __l2cap_ecred_conn_rsp_defer() while holding only lock_sock(sk). That function walks conn->chan_l via __l2cap_chan_list_id() and, on the authorization/refuse path, removes channels with l2cap_chan_del() -> list_del(&chan->list) -- all without conn->lock. conn->chan_l is serialised by conn->lock and is concurrently mutated by the RX worker, which processes inbound signalling (e.g. an L2CAP_DISCONN_REQ -> l2cap_chan_del()) under conn->lock. A lockless traversal/removal racing the RX worker's list_del() corrupts the list: list_del corruption, ...->next is LIST_POISON1 (dead000000000100) WARNING lib/list_debug.c:56 __list_del_entry_valid_or_report l2cap_chan_del+0xdb/0x1140 __l2cap_ecred_conn_rsp_defer+0x61d/0x700 l2cap_sock_recvmsg+0x758/0x8b0 Oops: general protection fault KASAN: maybe wild-memory-access in range [dead000000000100-...] __l2cap_ecred_conn_rsp_defer+0x3c6/0x700 l2cap_sock_recvmsg+0x758/0x8b0 This is the recvmsg-path sibling of commit 41c2713 ("Bluetooth: L2CAP: Fix possible crash on l2cap_ecred_conn_rsp"), which addressed the same conn->chan_l manipulation on the signalling (l2cap_ecred_conn_rsp) side; the deferred-accept path was left unguarded. Take conn->lock around __l2cap_ecred_conn_rsp_defer(). The established lock order is conn->lock -> chan->lock -> sk_lock (the RX worker reaches the socket via l2cap_chan_del() -> l2cap_sock_teardown_cb() -> lock_sock_nested()), so the socket lock is dropped before conn->lock is taken, mirroring l2cap_sock_shutdown() and l2cap_sock_cleanup_listen(). The conn is pinned with l2cap_conn_hold_unless_zero() across the unlocked window. Only the EXT_FLOWCTL branch needs this; the LE and BR/EDR defer paths respond for a single channel and do not walk conn->chan_l. Found by 0sec (https://0sec.ai). Fixes: 15f02b9 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode") Cc: stable@vger.kernel.org Assisted-by: 0sec Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
|
CheckPatch |
|
VerifyFixes |
|
VerifySignedoff |
|
GitLint |
|
SubjectPrefix |
|
BuildKernel |
|
CheckAllWarning |
|
CheckSparse |
|
BuildKernel32 |
|
CheckKernelLLVM |
|
TestRunnerSetup |
|
TestRunner_l2cap-tester |
|
IncrementalBuild |
2 participants
When a deferred L2CAP_MODE_EXT_FLOWCTL connection is accepted,
l2cap_sock_recvmsg() (BT_CONNECT2 + BT_SK_DEFER_SETUP branch) calls
__l2cap_ecred_conn_rsp_defer() while holding only lock_sock(sk). That
function walks conn->chan_l via __l2cap_chan_list_id() and, on the
authorization/refuse path, removes channels with l2cap_chan_del() ->
list_del(&chan->list) -- all without conn->lock.
conn->chan_l is serialised by conn->lock and is concurrently mutated by
the RX worker, which processes inbound signalling (e.g. an
L2CAP_DISCONN_REQ -> l2cap_chan_del()) under conn->lock. A lockless
traversal/removal racing the RX worker's list_del() corrupts the list:
list_del corruption, ...->next is LIST_POISON1 (dead000000000100)
WARNING lib/list_debug.c:56 __list_del_entry_valid_or_report
l2cap_chan_del+0xdb/0x1140
__l2cap_ecred_conn_rsp_defer+0x61d/0x700
l2cap_sock_recvmsg+0x758/0x8b0
Oops: general protection fault
KASAN: maybe wild-memory-access in range [dead000000000100-...]
__l2cap_ecred_conn_rsp_defer+0x3c6/0x700
l2cap_sock_recvmsg+0x758/0x8b0
This is the recvmsg-path sibling of commit 41c2713 ("Bluetooth:
L2CAP: Fix possible crash on l2cap_ecred_conn_rsp"), which addressed the
same conn->chan_l manipulation on the signalling (l2cap_ecred_conn_rsp)
side; the deferred-accept path was left unguarded.
Take conn->lock around __l2cap_ecred_conn_rsp_defer(). The established
lock order is conn->lock -> chan->lock -> sk_lock (the RX worker reaches
the socket via l2cap_chan_del() -> l2cap_sock_teardown_cb() ->
lock_sock_nested()), so the socket lock is dropped before conn->lock is
taken, mirroring l2cap_sock_shutdown() and l2cap_sock_cleanup_listen().
The conn is pinned with l2cap_conn_hold_unless_zero() across the
unlocked window. Only the EXT_FLOWCTL branch needs this; the LE and
BR/EDR defer paths respond for a single channel and do not walk
conn->chan_l.
Found by 0sec (https://0sec.ai).
Fixes: 15f02b9 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode")
Cc: stable@vger.kernel.org
Assisted-by: 0sec
Signed-off-by: Doruk Tan Ozturk doruk@0sec.ai
net/bluetooth/l2cap_sock.c | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)