mirror of
https://github.com/torvalds/linux.git
synced 2026-10-16 12:50:02 +02:00
master
5867 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Chengfeng Ye
|
e32d80293a |
Bluetooth: ISO: Serialize concurrent connect calls
iso_sock_connect() checks the socket state before taking the socket lock
and drops the lock again before setting up a connection. Two callers can
both pass the admission check while the socket is open or bound.
Caller A can copy its destination for route selection, then caller B can
replace the socket destination before A binds or connects the CIS. A
then uses B's destination with the route selected for its own request.
B can also proceed after A attaches a connection and sets BT_CONNECT.
For deferred BIS setup, B can bind a second BIS and overwrite
iso_pi(sk)->conn, leaving the first connection's reference and conn->sk
back-pointer stranded. Concurrent CIS connects can likewise replace the
socket's connection. Later teardown only detaches the current connection,
so a callback on the old connection can race with socket release and
access a freed socket.
KASAN reported:
BUG: KASAN: slab-use-after-free in iso_sock_hold+0xf7/0x1b0
Call Trace:
iso_sock_hold+0xf7/0x1b0
iso_conn_del+0x7b/0x1d0
iso_connect_cfm+0x186/0x16a0
hci_conn_failed+0x154/0x280
hci_abort_conn_sync+0x3dc/0x7d0
hci_cmd_sync_work+0x173/0x300
Allocated by task 121:
sk_alloc+0x2b/0x6d0
bt_sock_alloc+0x29/0x370
iso_sock_alloc.constprop.0+0x19/0x300
iso_sock_create+0x94/0x100
Freed by task 121:
kfree+0x121/0x3c0
__sk_destruct+0x42b/0x540
iso_sock_release+0x29d/0x340
__sock_release+0xa1/0x260
Check admission under the socket lock and mark the connect operation in
progress before publishing its destination. Keep that flag set across
route lookup and connection setup, rejecting another connect with
-EBADFD before it can change the destination or attach a connection.
Clear the flag after either helper returns, including on failure, so a
failed setup can be retried.
A separate flag is needed because the socket lock must be released before
acquiring the HCI device lock, while BT_CONNECT requires an attached
connection. Keep the existing state transitions and the deferred CIS
completion through iso_sock_recvmsg().
Reject an existing socket attachment in __iso_chan_add() before taking
a new reference or publishing either pointer, so every caller preserves
the connection association. Keep the same-socket, same-connection success
case for deferred CIS setup. Reject an attached socket before BIS setup
so a reusable BIS is not claimed before the attachment is rejected.
In CIS setup, reject a different HCI connection before iso_conn_add()
and drop the per-attempt HCI hold directly. An unowned iso_conn may still
carry another reference after detachment, so its destruction cannot be
relied on to release that hold.
iso_listen_bis() creates a fresh PA HCI/ISO pair under the device lock,
so the temporary iso_conn_put() frees the ISO candidate and drops its
HCI hold on rejection. iso_conn_ready() uses a freshly allocated,
unpublished child with no connection, making the socket-side rejection
unreachable.
Fixes:
|
||
|
Chengfeng Ye
|
555ff78fa7 |
Bluetooth: ISO: Reject concurrent BIS listener setup
iso_sock_listen() drops the socket lock before calling iso_listen_bis()
to preserve the hdev->lock ordering. Two concurrent listen() calls on
the same broadcast socket can therefore both observe BT_BOUND. Caller
A can attach connA and release the locks, then caller B can acquire
them and attach connB before A publishes BT_LISTEN.
__iso_chan_add() checks whether connB already has a socket, but not
whether the socket already has a connection. The second attachment
overwrites iso_pi(sk)->conn, leaking the connA reference and leaving
connA->sk intact. Socket teardown follows connB, so later events on
connA can cause a stale socket access.
Recheck BT_BOUND and require iso_pi(sk)->conn to be NULL after taking
both the HCI device and socket locks in iso_listen_bis(). The connection
check also covers the interval between attachment and publication of
BT_LISTEN. Reject a competing setup with -EBADFD through the existing
unlock path before allocating another connection.
A recorded KASAN report from an instrumented kernel shows:
BUG: KASAN: slab-use-after-free in iso_sock_hold+0x119/0x1f0
Call Trace:
iso_sock_hold+0x119/0x1f0
iso_conn_del+0xe4/0x270
hci_disconn_complete_evt+0x351/0x8c0
hci_event_packet+0x71b/0xb20
hci_rx_work+0x293/0x730
Allocated by task 93:
sk_prot_alloc+0x113/0x220
sk_alloc+0x2b/0x6d0
bt_sock_alloc+0x29/0x370
iso_sock_alloc.constprop.0+0x19/0x300
iso_sock_create+0x94/0x100
Freed by task 93:
kfree+0x121/0x3c0
__sk_destruct+0x42b/0x540
iso_sock_release+0x29d/0x340
__sock_release+0xa1/0x260
sock_close+0x10/0x20
Fixes:
|
||
|
Chengfeng Ye
|
9af1227206 |
Bluetooth: hci_sync: Fix command skb lifetime during scan setup
During PA sync scan setup, hci_le_set_ext_scan_param_sync() gets the
address used for the PA_LINK lookup from hci_sent_cmd_data(). The
returned pointer borrows storage from sent_cmd or req_skb, without
holding a reference to either skb.
The scan worker runs on req_workqueue while hci_cmd_work() runs on the
separate device workqueue. After the scan worker loads sent_cmd, the
command worker can free and replace it before hci_sent_cmd_data()
dereferences the skb. The command payload can also be freed between
returning from the helper and comparing the address. The connection
lookup's RCU critical section does not protect the command skb.
KASAN reported:
BUG: KASAN: slab-use-after-free in hci_sent_cmd_data+0x27c/0x2f0
Workqueue: hci0 hci_cmd_sync_work
Call Trace:
hci_sent_cmd_data+0x27c/0x2f0
hci_le_set_scan_param_sync+0x43a/0x850
hci_passive_scan_sync+0xb09/0x1460
hci_update_passive_scan_sync+0x47c/0x6e0
hci_le_pa_create_sync+0x200/0xa70
hci_cmd_sync_work+0x13c/0x290
Allocated by task 95:
skb_clone+0x13f/0x340
hci_cmd_work+0x2a9/0x7e0
Freed by task 95:
kmem_cache_free+0xba/0x3a0
hci_cmd_work+0x29c/0x7e0
Use hdev->lock to serialize the address snapshot with sent_cmd
replacement. Protect req_skb replacement and completion cleanup with
the same lock because the helper falls back to that skb. Copy the
address before releasing the lock and use the copy for the existing
RCU-protected connection lookup. Release the mutex before sending or
waiting for commands, preserving the lookup and completion ordering.
Fixes:
|
||
|
Chengfeng Ye
|
381a0925c5 |
Bluetooth: hci_sock: Serialize dead-device detachment
Rebinding an HCI socket after its controller is unregistered can race
monitor control replay and crash the kernel. hci_sock_bind() clears
hci_pi(sk)->hdev and drops the device reference under only the socket
lock, whereas send_monitor_control_replay() holds hci_sk_list.lock for
reading.
Replay can observe a non-NULL hdev, then bind can clear it and drop the
last reference before create_monitor_ctrl_open() reads hdev->id. This
can cause a NULL pointer dereference if the pointer is reloaded, or a
use-after-free if the old pointer is retained.
The kernel reported:
KASAN: null-ptr-deref in range [0x0000000000000068-0x000000000000006f]
RIP: 0010:create_monitor_ctrl_open+0x579/0x800
Call Trace:
hci_sock_bind+0xd96/0x1190
__sys_bind+0x166/0x200
__x64_sys_bind+0x6d/0xb0
do_syscall_64+0xdd/0x4a0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
The same missing serialization lets hci_send_to_sock() select a socket
for the old device and queue its frame after bind has detached the socket
or installed a new binding.
Take hci_sk_list.lock for writing when clearing the device pointer and
resetting the socket state. This waits for existing replay and delivery
readers to finish before detachment and prevents later readers from
using the old binding. Keep hci_dev_put() outside the critical section
because the final device release can sleep.
Fixes:
|
||
|
Palla Raghunath
|
08e9063337 |
Bluetooth: RFCOMM: Fix NULL tty_dev dereference in rfcomm_dev_shutdown
syzbot hit a NULL pointer dereference in rfcomm_dev_shutdown() when an
RFCOMM tty is closed.
rfcomm_dev_add() puts the new device on rfcomm_dev_list first and only
then registers the tty device and saves it in dev->tty_dev. If that
registration fails, dev->tty_dev is never set at all. The device can be
looked up in the meantime, so an open() of /dev/rfcommN gets through
rfcomm_tty_install() and attaches to it. When that tty is closed again,
rfcomm_dev_shutdown() uses dev->tty_dev without checking it:
Oops: general protection fault, probably for non-canonical address
KASAN: null-ptr-deref in range [0x0000000000000040-0x0000000000000047]
RIP: 0010:rfcomm_dev_shutdown+0x48/0xb0 net/bluetooth/rfcomm/tty.c:131
Call Trace:
tty_port_shutdown+0x1bf/0x220 drivers/tty/tty_port.c:372
tty_port_close+0x4e/0x150 drivers/tty/tty_port.c:705
tty_release+0x359/0x1670 drivers/tty/tty_io.c:1718
__fput+0x418/0xa50 fs/file_table.c:512
rfcomm_dev_destruct() already copes with a NULL dev->tty_dev. Add the
same check to rfcomm_dev_shutdown(), so the tty device is only moved
when it was actually registered.
Fixes:
|
||
|
Iaroslav Voitovych
|
86ef0f58bd |
Bluetooth: MGMT: Fix status of pending commands flushed on power off
cmd_complete_rsp() receives a struct cmd_lookup and, for a pending command that has no cmd_complete callback, falls through to cmd_status_rsp(cmd, data). cmd_status_rsp() reads data as a u8 *, so the status sent comes from the first byte of match->sk's representation, not from match->mgmt_status. commit |
||
|
Mikhail Gavrilov
|
ac022970e9 |
Bluetooth: RFCOMM: connect the session socket without rfcomm_mutex
An RFCOMM connect() issued while a BR/EDR link is being authenticated
makes lockdep report a circular dependency, and the reported cycle is a
real AB/BA between rfcomm_mutex and hdev->lock.
rfcomm_security_cfm() is called from the HCI event path, which already
holds hdev->lock:
hci_rx_work()
hci_event_packet()
hci_cc_read_enc_key_size() [hdev->lock]
hci_encrypt_cfm() [hci_cb_list_lock]
rfcomm_security_cfm() [rfcomm_mutex]
while an RFCOMM connect() from userspace takes the same two locks the
other way round:
rfcomm_sock_connect()
rfcomm_dlc_open() [rfcomm_mutex]
__rfcomm_dlc_open()
rfcomm_session_create()
kernel_connect()
l2cap_sock_connect()
l2cap_chan_connect() [hdev->lock]
WARNING: possible circular locking dependency detected
kworker/u131:1/1128 is trying to acquire lock:
rfcomm_mutex, at: rfcomm_security_cfm+0x31/0x3e0 [rfcomm]
but task is already holding lock:
hci_cb_list_lock, at: hci_cc_read_enc_key_size+0x1d2/0xcc0
Chain exists of:
rfcomm_mutex --> &hdev->lock --> hci_cb_list_lock
hci_auth_complete_evt() and hci_encrypt_change_evt() reach the callback
the same way.
Both orders have to be seen in the same boot, which is why a BR/EDR
connection alone is not enough to show it: a session set up by the
remote side is created by rfcomm_accept_connection() in krfcommd, which
calls kernel_accept() and never takes hdev->lock under rfcomm_mutex.
Connecting a device that authenticates and encrypts the link and then
calling connect() on an RFCOMM socket towards any address - the connect
does not have to succeed, the order is recorded before the page timeout
- reports it every time.
Only the session socket has to be connected with the lock held, and it
does not: nothing else can see the socket before it is put on the
session list. So connect it first and take rfcomm_mutex afterwards,
which removes the rfcomm_mutex -> hdev->lock order for good, rather
than keeping the HCI event path out of rfcomm_mutex.
rfcomm_session_create() becomes rfcomm_session_connect(), which returns
the connected socket without touching the session list, and
rfcomm_dlc_open() adds the session once it holds the lock again. If
another opener added a session for the same pair while this socket was
connecting, that session is used and this socket is dropped.
__rfcomm_dlc_open() now takes the session it should use, and its state
check runs after the lock is re-acquired, so a DLC that was opened or
closed in the meantime is still handled.
Over an existing ACL link the connection can complete before the
session reaches the list, and the wakeup from the socket callback is
then lost, so krfcommd is woken once the session is visible.
Connecting without the lock opens a window in which the socket can be
closed. The DLC is not on a session yet, so rfcomm_dlc_close() finds
nothing to do and returns without touching it, and rfcomm_dlc_open()
would then attach a DLC whose owner is gone. rfcomm_dlc_close() now
marks such a DLC with RFCOMM_CLOSED, and rfcomm_dlc_open() checks the
mark once it holds rfcomm_mutex again and drops the socket instead of
attaching the DLC. The mark is cleared when an open starts, so a DLC
that is reused is not affected. This covers both callers of
rfcomm_dlc_open(), the socket and the tty layer.
Fixes:
|
||
|
Aldo Ariel Panzardo
|
816fb1590a |
Bluetooth: SCO: serialise sco_conn lifetime against sco_recv_scodata()
sco_recv_scodata() upgrades the weak hcon->sco_data back-pointer to a
strong reference under hdev->lock:
hci_dev_lock(hdev);
hcon = hci_conn_hash_lookup_handle(hdev, handle);
...
conn = sco_conn_hold_unless_zero(hcon->sco_data);
hci_dev_unlock(hdev);
but the pointer is cleared from the other side without that lock. When
the last sco_conn reference is dropped, sco_conn_free() ran
conn->hcon->sco_data = NULL;
with no hdev->lock held, so the RX path could read hcon->sco_data and
call kref_get_unless_zero() on an sco_conn that was concurrently freed:
BUG: KASAN: slab-use-after-free in sco_conn_hold_unless_zero+0xbe/0x160
Write of size 4 by task kworker/u17:0
Workqueue: hci0 hci_rx_work
Call Trace:
sco_conn_hold_unless_zero+0xbe/0x160
sco_recv_scodata+0x13f/0x490
hci_rx_work+0x3af/0x730
kref_get_unless_zero() only guards against a zero refcount, not against
the backing memory already being freed.
Give hcon->sco_data an actual reference on the sco_conn it points to, so
the object cannot be freed while the pointer is still installed, and only
clear and drop it from sco_conn_del(), which runs under hdev->lock (its
callers, sco_connect_cfm() and sco_disconn_cfm(), hold it). The reader in
sco_recv_scodata() also takes hdev->lock, so the store and the read are
now serialised: the RX path either observes NULL or a reference that is
guaranteed to stay valid until it drops its own.
sco_conn_add() no longer hands its kref_init() reference to the caller as
the connection's only reference; that initial reference is the one owned
by hcon->sco_data, and callers get their own via sco_conn_hold().
Because the sco_conn can now outlive the socket (the association keeps it
alive until the link goes down), the hci_conn can no longer be dropped
from sco_conn_free() without regressing socket close: closing a connected
SCO socket must still tear the link down. Move hci_conn ownership to the
socket instead: __sco_chan_add() takes an hci_conn reference and it is
released from sco_chan_del() and sco_sock_destruct(), exactly once. The
sco_conn no longer owns an hci_conn reference, so sco_conn_add() stops
consuming one and sco_connect() drops the hci_connect_sco() reference on
every path; sco_connect_cfm() no longer needs its hci_conn_hold() dance.
Fixes:
|
||
|
Maxim Skokov
|
9d1afbf60e |
Bluetooth: RFCOMM: free the skb when the DLC has no owner
rfcomm_recv_data() hands the skb to d->data_ready() and returns 0
without freeing it, so the callback owns the skb from that point on.
The tty implementation frees it when the DLC has been detached from its
device:
static void rfcomm_dev_data_ready(struct rfcomm_dlc *dlc,
struct sk_buff *skb)
{
struct rfcomm_dev *dev = dlc->owner;
if (!dev) {
kfree_skb(skb);
return;
}
The socket implementation returns without freeing it, and the skb is
leaked.
A connected DLC can have no owner: rfcomm_sock_destruct() clears
d->owner, while the DLC itself stays on the session until it is closed.
Data frames that arrive in that window still find d->state ==
BT_CONNECTED in rfcomm_recv_data() and are handed to the callback, one
leaked skb each.
The leak does not stop on its own either. The remote runs out of
credits only once RFCOMM_RX_THROTTLED is set, and for a socket the only
place that sets it is rfcomm_sk_data_ready() itself, below the return.
rfcomm_process_dlcs() keeps granting credits, so the remote keeps
sending and the leak keeps growing.
Free the skb, as the tty side does.
Fixes:
|
||
|
Chengfeng Ye
|
848a7a91c7 |
Bluetooth: hci_core: Serialize SCO and ISO scheduling with teardown
hci_low_sent() selects a connection under RCU but drops the read lock
before calculating its quota and returning it to the scheduler. The
connection is then used without lifetime protection by hci_sched_iso()
and hci_sched_sco().
After the TX worker drops the RCU read lock, hci_abort_conn_sync() on
hdev->req_workqueue can remove the connection from the hash, complete
synchronize_rcu(), purge its queues and release it. The TX worker on
hdev->workqueue can then access the freed connection in hci_quote_sent()
or while dequeuing packets and updating conn->sent.
KASAN reported:
BUG: KASAN: slab-use-after-free in hci_low_sent+0x730/0x840
Workqueue: hci0 hci_tx_work
Call Trace:
hci_low_sent+0x730/0x840
hci_sched_iso+0x25e/0x4d0
hci_tx_work+0x239/0xcb0
Allocated by task 93:
__hci_conn_add+0x16f/0x1b40
hci_bind_bis+0x782/0x17b0
hci_connect_bis+0xa0/0x510
iso_sock_connect+0x589/0x1050
Freed by task 88:
kfree+0x131/0x3c0
device_release+0xc8/0x240
kobject_put+0x14d/0x280
hci_conn_del+0x55a/0xe80
hci_disconnect_sync+0x156/0x180
hci_abort_conn_sync+0x3e7/0x940
hci_cmd_sync_work+0x13c/0x290
Hold hci_dev_lock() across connection selection and transmission in both
SCO and ISO scheduling, serializing them with connection teardown. This
also prevents queuing completion timestamps after the connection queues
have been purged. Extending RCU across transmission would be unsafe
because the transmit path can sleep. Keep the ISO timeout check outside
the mutex since hci_link_tx_to() takes it itself.
The preceding channel fix already holds this mutex in the ACL and LE
schedulers, which call the SCO scheduler between packets. Move the SCO
body to __hci_sched_sco(), assert that its caller holds the mutex, and
use it directly from these locked paths. Keep a locking hci_sched_sco()
wrapper for the direct calls from hci_tx_work(). This avoids recursively
acquiring the device mutex while preserving the scheduling order.
Remove the obsolete claim that connection removal disables TX.
Fixes:
|
||
|
Chengfeng Ye
|
f4845ab191 |
Bluetooth: hci_core: Serialize ACL scheduling with channel deletion
hci_chan_sent() selects a channel under RCU but drops the read lock before
accessing chan->conn and returning the channel. The ACL and LE schedulers
then use its packet queue and update its transmit counters without any
protection against channel deletion.
After TX selects a channel and releases RCU, a disconnect command timeout
on the separate request workqueue can run hci_conn_failed() and
l2cap_conn_del(). hci_chan_del() can then unlink the channel, complete
synchronize_rcu(), purge its queue and free it before TX resumes. This
causes use-after-free both in hci_chan_sent() and in its callers.
KASAN reported:
BUG: KASAN: slab-use-after-free in hci_chan_sent+0x892/0x9b0
Workqueue: hci0 hci_tx_work
Call Trace:
hci_chan_sent+0x892/0x9b0
hci_tx_work+0x5e6/0xb70
Allocated by task 91:
hci_chan_create+0xe3/0x350
l2cap_conn_add.part.0+0x12/0xa30
l2cap_chan_connect+0x110d/0x1b60
l2cap_sock_connect+0x310/0x530
Freed by task 99:
hci_chan_del+0x11f/0x170
l2cap_conn_del+0x4f1/0x800
l2cap_connect_cfm+0x88c/0xd30
hci_conn_failed+0x150/0x250
hci_abort_conn_sync+0x3e3/0x800
hci_cmd_sync_run+0x7e/0xc0
hci_abort_conn+0x105/0x1f0
disconnect_sync+0x157/0x290
hci_cmd_sync_work+0x13c/0x290
Hold the existing device mutex across channel selection and transmission
in both schedulers to serialize them with channel teardown. Keep timeout
handling outside the critical sections because hci_link_tx_to() acquires
the same mutex. This also permits the transmit path to sleep, unlike
extending the RCU read-side critical section across packet submission.
Fixes:
|
||
|
Chengfeng Ye
|
81a2345f19 |
Bluetooth: Serialize SMP remote OOB data access
build_pairing_cmd() looks up remote OOB data and copies its contents
without holding hdev->lock, which serializes the list's writers. After
SMP finds an entry, a concurrent management Remove Remote OOB Data
command can unlink and free it before SMP reads its present flag or
copies its random and confirmation values. Removal can also invalidate
an entry while the lookup is still traversing the list.
KASAN reported:
BUG: KASAN: slab-use-after-free in build_pairing_cmd+0x948/0x9b0
Call Trace:
build_pairing_cmd+0x948/0x9b0
smp_recv_cb+0x459f/0x8110
l2cap_recv_frame+0xf14/0x9190
l2cap_recv_acldata+0xa64/0xd40
hci_rx_work+0x4ca/0x730
Allocated by task 87:
hci_add_remote_oob_data+0x11d/0x530
add_remote_oob_data+0x282/0x400
hci_sock_sendmsg+0x1033/0x1ea0
Freed by task 93:
hci_remote_oob_data_clear+0x108/0x1c0
remove_remote_oob_data+0x198/0x220
hci_sock_sendmsg+0x1033/0x1ea0
Taking hdev->lock in build_pairing_cmd() would recurse for callers that
already hold it and invert the device-to-L2CAP lock order on the receive
path. Add a per-device remote_oob_lock instead, held across the SMP
lookup and copies and by the add, remove and clear helpers. Cover
initialization and in-place updates as well, so SMP cannot read partially
initialized or updated OOB values. Release the mutex on allocation
failure, preserving the existing error return.
The new critical sections acquire no device, connection or channel locks.
Writers retain their existing hdev->lock protection, which continues to
serialize the other readers without changing their locking or behavior.
Link: https://lore.kernel.org/r/00660cd3-7d71-13a4-f617-229e6defb701@gmail.com
Fixes:
|
||
|
Chengfeng Ye
|
2d696c1ed3 |
Bluetooth: RFCOMM: Fix initial port reference race
For RFCOMM_RELEASE_ONHUP devices, rfcomm_tty_install() and
__rfcomm_release_dev() both use RFCOMM_TTY_OWNED to decide who drops the
initial tty_port reference. The release path tests the bit separately
from the install path setting it, so both can drop that reference.
The synchronous hangup does not prevent this race: port->tty is not
assigned until tty_port_open(), after installation. The following
interleaving is possible:
1. Install and release each obtain a reference with rfcomm_dev_get().
2. Release observes RFCOMM_TTY_OWNED clear.
3. Install sets RFCOMM_TTY_OWNED and drops the initial reference.
4. Release drops the same initial reference again.
5. TTY cleanup drops its reference and frees the device.
6. Release performs its final tty_port_put() on the freed port.
KASAN reported:
BUG: KASAN: slab-use-after-free in tty_port_put+0x22/0x190
Write of size 4 at addr ffff8881001d3d5c by task poc/92
Call Trace:
tty_port_put+0x22/0x190
rfcomm_dev_ioctl+0x1d4/0x1930
sock_do_ioctl+0x110/0x260
sock_ioctl+0x380/0x590
__x64_sys_ioctl+0x134/0x1c0
Allocated by task 88:
rfcomm_dev_ioctl+0x8f6/0x1930
sock_do_ioctl+0x110/0x260
sock_ioctl+0x380/0x590
__x64_sys_ioctl+0x134/0x1c0
Freed by task 65:
kfree+0x131/0x3c0
rfcomm_dev_destruct+0x23b/0x2f0
release_one_tty+0xc1/0x370
process_one_work+0x661/0x1090
Use test_and_set_bit() in both paths so that only the caller that changes
RFCOMM_TTY_OWNED from clear to set drops the initial reference. Each path
keeps its own lookup reference until release or TTY cleanup, preserving
the existing callback ordering and error handling.
Fixes:
|
||
|
Chengfeng Ye
|
8ed67535fd |
Bluetooth: hci_sync: Fix inquiry cache use-after-free
The sync command worker holds hdev->req_lock, but inquiry-cache updates and flushes use hdev->lock. Both hci_acl_create_conn_sync() and hci_stop_discovery_sync() look up entries and read their fields without taking hdev->lock. After either lookup returns, a concurrent HCIINQUIRY ioctl can acquire hdev->lock and flush the cache, freeing the entry. The worker then reads the freed entry while preparing a create-connection or remote-name-cancel command. The list traversal also races with cache updates and removal. KASAN reported these accesses: BUG: KASAN: slab-use-after-free in hci_acl_create_conn_sync+0x5f1/0x650 Workqueue: hci0 hci_cmd_sync_work Call Trace: hci_acl_create_conn_sync+0x5f1/0x650 hci_cmd_sync_work+0x13c/0x290 Allocated by task 90: hci_inquiry_cache_update+0x3e6/0x7d0 hci_inquiry_result_evt+0x3cb/0x560 Freed by task 93: hci_inquiry_cache_flush+0x111/0x2b0 hci_inquiry+0x2f2/0x780 hci_sock_ioctl+0x269/0x5f0 BUG: KASAN: slab-use-after-free in hci_stop_discovery_sync+0x3b1/0x3c0 Workqueue: hci0 hci_cmd_sync_work Call Trace: hci_stop_discovery_sync+0x3b1/0x3c0 hci_cmd_sync_work+0x173/0x300 Allocated by task 86: hci_inquiry_cache_update+0x483/0x940 hci_inquiry_result_evt+0x3cb/0x560 Freed by task 91: hci_inquiry_cache_flush+0x13e/0x2f0 hci_inquiry+0x2f2/0x780 hci_sock_ioctl+0x269/0x5f0 Hold hdev->lock across each lookup and all reads from its result. Copy the remote address before unlocking so discovery cancellation does not retain a cache entry pointer. Release the lock before sending synchronous HCI commands, since their completion handlers may need the same lock. Fixes: |
||
|
Nguyen Ngoc Thang
|
b1f24c1ab5 |
Bluetooth: hci_sync: don't drain cmd_sync backlog on unregister
hci_unregister_dev() disables cmd_work and cmd_timer, then calls
hci_cmd_sync_clear(), whose cancel_work_sync() waits for
hci_cmd_sync_work() to return. That worker dequeues and runs every
entry on cmd_sync_work_list. With cmd_work disabled nothing reaches
the controller, so each queued HCI command waits the full
HCI_CMD_TIMEOUT (2s).
The backlog has no bound. Userspace can keep queuing MGMT commands
such as MGMT_OP_GET_CLOCK_INFO against a controller that doesn't
answer. Closing /dev/vhci then blocks in vhci_release() for
backlog * 2s:
INFO: task syz-executor:5749 blocked for more than 143 seconds.
cancel_work_sync
hci_cmd_sync_clear
hci_unregister_dev
vhci_release
In a local reproduction the backlog held more than 5000 entries,
which comes to hours of hang.
Stop the worker from taking new entries once HCI_UNREGISTER is set,
and wake any request still waiting with -ENODEV before cancelling
the work. The entries left over are destroyed with -ECANCELED by the
existing sweep in hci_cmd_sync_clear(). They stay on the list until
the work has stopped, so hci_cmd_sync_dequeue() and friends still see
them. A callback already running may still issue another command,
which delays unregister by at most one timeout per remaining command,
not by the whole backlog.
Fixes:
|
||
|
Chengfeng Ye
|
1a572db43c |
Bluetooth: hci_core: Serialize fragmented ISO packet queueing
The fragmented path in hci_queue_iso() calls __skb_queue_tail() without
holding conn->data_q.lock. The socket lock serializes senders, but the
transmit worker can concurrently remove packets from this queue using
skb_dequeue().
After an insertion saves the old tail pointer, hci_sched_iso() can dequeue
that packet and pass it to the driver. The driver can free the packet
before the insertion resumes and writes to the old tail's next pointer,
causing a use-after-free. Concurrent updates can also corrupt the queue.
KASAN reported:
BUG: KASAN: slab-use-after-free in hci_send_iso+0xc7d/0xe10
Call Trace:
hci_send_iso+0xc7d/0xe10
iso_sock_sendmsg+0x710/0x900
__sys_sendto+0x34a/0x3a0
Allocated by task 86:
__alloc_skb+0xdd/0x820
alloc_skb_with_frags+0x7e/0x770
sock_alloc_send_pskb+0x67a/0x820
bt_skb_sendmsg.constprop.0+0xc0/0x6c0
iso_sock_sendmsg+0x44e/0x900
Freed by task 90:
kmem_cache_free+0xcb/0x3d0
vhci_read+0x33f/0x4d0
vfs_read+0x177/0xa20
Hold the queue lock across the entire fragment batch, as hci_queue_acl()
does, to serialize insertion against dequeue while preserving fragment
ordering.
Fixes:
|
||
|
Chengfeng Ye
|
024e05f73a |
Bluetooth: hci_conn: Lock parent access during enhanced SCO setup
Bluetooth: hci_conn: Lock parent access during enhanced SCO setup
hci_enhanced_setup_sync() runs on the request workqueue without the
hci_dev_lock held by its caller when setup was queued. Its CVSD
capability check and find_next_esco_param() dereference conn->parent
while the receive workqueue can unlink and release that parent.
The following interleaving can cause a use-after-free:
hci_enhanced_setup_sync() hci_disconn_complete_evt()
load conn->parent
hci_dev_lock()
hci_conn_del(ACL parent)
unlink SCO child
drop link's parent reference
clear child->parent
release ACL parent
bt_link_release()
kfree(parent)
hci_dev_unlock()
read parent->features[0][3]
The reference held for the queued SCO child does not keep its ACL parent
alive after unlinking. Commit
|
||
|
Sang-Hoon Choi
|
d1ac915fb4 |
Bluetooth: hci_core: free the HCI ID if naming fails
hci_register_dev() allocates an ID before calling dev_set_name().
If naming fails, it returns without releasing that ID. The device has
not been registered and hdev->id has not been assigned, so the normal
unregister path cannot release it.
Free the allocated ID before returning the naming error.
Fixes:
|
||
|
Linmao Li
|
74847177ec |
Bluetooth: hci_core: Fix inquiry cache timestamps on 64-bit systems
On 64-bit systems, outgoing BR/EDR connections always fall back to page
scan repetition mode R2 with no clock offset once the system has been
up for more than five minutes, even when inquiry found the peer only
seconds earlier. This lengthens paging and can increase the risk of a
Page Timeout.
The inquiry cache stores jiffies in __u32 timestamps, but its age
helpers subtract them from unsigned long jiffies. INITIAL_JIFFIES casts
-300 * HZ through unsigned int, so jiffies crosses 2^32 five minutes
after boot on 64-bit systems. Assigning it to __u32 then drops the upper
32 bits. In one trace, a 7.6-second-old entry (HZ=1000) was reported as
2^32 + 7620 ticks old and rejected by hci_acl_create_conn_sync().
hci_inquiry() is affected by the same truncation when checking the whole
cache. 32-bit systems are unaffected because unsigned long is 32 bits
wide there.
Use unsigned long for both timestamps so they have the same width as
jiffies on 32-bit and 64-bit systems, and update the debugfs format
specifier accordingly.
Fixes:
|
||
|
Hui Peng
|
6d91041bb3 |
Bluetooth: RFCOMM: Reject short EA=0 frames in rfcomm_recv_frame()
While rfcomm_recv_frame() verifies that skb->len is at least
sizeof(*hdr) + 1 (4 bytes: 3-byte header + 1-byte FCS), an RFCOMM frame
with an extended 2-byte length field (!__test_ea(hdr->len)) has a 4-byte
header plus a 1-byte FCS (5 bytes minimum, sizeof(*hdr) + 2).
When a 4-byte RFCOMM frame with EA == 0 arrives:
1. The initial skb->len < sizeof(*hdr) + 1 check passes (4 < 4 is false).
2. Trimming the FCS byte decrements skb->len to 3.
3. If __check_fcs() succeeds, skb_pull(skb, 4) fails (4 > 3) and returns
NULL without advancing skb->data.
4. Because the return value of skb_pull() is ignored, the un-pulled
3-byte struct rfcomm_hdr remains at skb->data and is either queued as
application payload via rfcomm_recv_data() or parsed as a multiplexer
control command via rfcomm_recv_mcc() on DLCI 0.
Fix this by extending the length check in rfcomm_recv_frame() to also
require skb->len >= sizeof(*hdr) + 2 when !__test_ea(hdr->len).
Fixes:
|
||
|
Hui Peng
|
46f8ffd0a1 |
Bluetooth: RFCOMM: fix NULL dereference of dlc->session in RFCOMM_CONNINFO
The RFCOMM_CONNINFO getsockopt handler accepts a socket that is not
connected as long as deferred setup is enabled:
if (sk->sk_state != BT_CONNECTED &&
!rfcomm_pi(sk)->dlc->defer_setup) {
err = -ENOTCONN;
break;
}
l2cap_sk = rfcomm_pi(sk)->dlc->session->sock->sk;
dlc->defer_setup is set in rfcomm_sock_init() when rfcomm_connect_ind()
creates a child socket for an incoming connection on a listening socket
that has BT_DEFER_SETUP enabled. It is never cleared afterwards. The
session, however, can go away underneath it.
rfcomm_recv_disc() forces the dlc state before tearing it down:
d->state = BT_CLOSED;
__rfcomm_dlc_close(d, err);
The RFCOMM_DEFER_SETUP early return in __rfcomm_dlc_close() only covers
BT_CONNECT, BT_CONFIG, BT_OPEN and BT_CONNECT2, so with the state
already BT_CLOSED that switch does not match and the function falls
through to rfcomm_dlc_unlink(), which sets d->session = NULL, while
d->defer_setup stays 1.
A getsockopt(SOL_RFCOMM, RFCOMM_CONNINFO) on the accepted socket after
that point therefore skips the -ENOTCONN path -- sk->sk_state is
BT_CLOSED, but dlc->defer_setup is still set -- and dereferences the
NULL session. No race is needed: once the DISC has been processed, the
dereference is unconditional.
Reproduced on a KASAN kernel under QEMU 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 and sends SABM for a channel bound with
BT_DEFER_SETUP, and sends DISC for that dlci after the socket has been
accepted. getsockopt(SOL_RFCOMM, RFCOMM_CONNINFO) on the accepted
socket then hits:
Oops: general protection fault, probably for non-canonical address
0xdffffc0000000002: 0000 [#1] SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
CPU: 1 UID: 0 PID: 150 Comm: init Tainted: G B 7.3.0-rc3-g5dd1818b15d9
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
RIP: 0010:rfcomm_sock_getsockopt+0x529/0x780
Call Trace:
<TASK>
do_sock_getsockopt+0x3ad/0x7d0
__sys_getsockopt+0x10e/0x1b0
__x64_sys_getsockopt+0xc2/0x160
do_syscall_64+0xda/0x4b0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
</TASK>
0x10 is the offset of sock in struct rfcomm_session;
rfcomm_sock_getsockopt_old() is inlined into rfcomm_sock_getsockopt().
Commit
|
||
|
Hui Peng
|
f0ca020cbb |
Bluetooth: bnep: fix out-of-bounds reads on short RX/TX frames and control fallthrough
Fix multiple out-of-bounds reads in Bluetooth BNEP frame processing:
1. In bnep_rx_frame() and bnep_ctrl_frame() (net/bluetooth/bnep/core.c),
use pskb_may_pull() to verify the BNEP header, control type byte,
filter count, and extension headers exist before reading them, and
return 0 after handling BNEP_CONTROL instead of falling through to
Ethernet frame submission when no extension headers follow.
2. In bnep_net_xmit() (net/bluetooth/bnep/netdev.c), verify skb->len >=
ETH_HLEN with pskb_may_pull() before reading the 14-byte Ethernet
header to prevent an out-of-bounds heap read and infoleak on short
AF_PACKET TX frames.
Fixes:
|
||
|
Aldo Ariel Panzardo
|
b5dbb41b21 |
Bluetooth: mgmt: fix race in read_unconf_index_list()
read_unconf_index_list() counts unconfigured controllers before allocating
its response, then checks the device flags again while filling it.
hci_dev_list_lock stabilizes list membership, but it does not serialize the
per-device flags. During asynchronous controller setup, the worker can set
HCI_UNCONFIGURED and clear HCI_SETUP between the two passes. A controller
omitted from the allocation count can then become eligible for the fill
pass, causing an out-of-bounds write to rp->index[].
Allocate space for every device on hci_dev_list. Since list membership
cannot change while hci_dev_list_lock is held, the response remains large
enough regardless of flag transitions. The reported count and response
length still include only eligible unconfigured controllers.
Fixes:
|
||
|
Aldo Ariel Panzardo
|
6c78a213d9 |
Bluetooth: L2CAP: validate frame length before control and FCS access
l2cap_data_rcv() unpacks either a two-byte or four-byte control field without first ensuring that it is present. A short ERTM or streaming-mode frame can therefore cause an out-of-bounds read. There is a second short-frame case when CRC16 is enabled. After the control field is pulled, l2cap_check_fcs() subtracts two from skb->len without checking it. If fewer than two bytes remain, the subtraction wraps; skb_trim() leaves the buffer unchanged and the subsequent FCS load reads past the logical end of the frame. Validate that the frame contains both its control field and, when enabled, its FCS before either field is accessed. Fixes: |
||
|
Aldo Ariel Panzardo
|
4c94557dd0 |
Bluetooth: ISO: balance the parent hold in hci_bind_bis()
hci_conn_link() takes a lifetime reference to its parent with
hci_conn_get(), but only takes an operational hold on the child.
hci_conn_unlink() later balances both a hold and a reference on the
parent.
The SCO and CIS paths pass a parent acquired from a connect helper, so
it already has a hold. For an additional BIS, hci_bind_bis() obtains the
parent from hci_conn_hash_lookup_big(), which returns a bare pointer.
Unlinking the child then drops the parent's existing hold and can
schedule it for disconnection while its socket is still using it.
Take a hold on the parent before linking it and drop that hold if linking
fails. A successful link transfers the hold to hci_conn_unlink().
Fixes:
|
||
|
Aldo Ariel Panzardo
|
b0a6cf99af |
Bluetooth: hci_sock: validate event length before filtering
is_filtered_packet() reads the event code from skb->data[0] without first
checking that the skb is nonempty. When an opcode filter is configured,
it also reads the command opcode at offsets 3 or 4 without checking that
a Command Complete or Command Status event is long enough.
hci_send_to_sock() invokes the filter before hci_event_packet() validates
the event header. A malformed event supplied by a controller or a vhci
device can therefore cause an out-of-bounds read.
Keep the unmasked event code for the opcode checks. The masked value is
needed for the 64-bit event bitmap, but using it to identify command events
aliases event codes above 0x3f. In particular, Synchronous Train Complete
(0x4f) was treated as Command Status (0x0f) even though its payload has no
opcode.
Reject actual command events that are too short for the field being
inspected. A truncated command event cannot match a configured opcode.
Fixes:
|
||
|
Aldo Ariel Panzardo
|
e93fad891c |
Bluetooth: hci_sock: reject out-of-range OCF values
The raw HCI socket security filter has 128 OCF bits per supported OGF,
but masks the 10-bit OCF with 127 before looking up the command. An
unprivileged socket can therefore submit a reserved OCF that aliases an
allowlisted command modulo 128.
A conforming controller should reject reserved opcodes. Nevertheless,
the security decision must apply to the opcode that will actually be
sent, especially since controller-specific behavior is outside the host
stack's control.
Reject OCF values that cannot be represented by the security filter
instead of aliasing them onto an unrelated command.
Fixes:
|
||
|
Aldo Ariel Panzardo
|
0fcd4dad55 |
Bluetooth: ISO: release unused CIS holds after channel attach
hci_bind_cis() and hci_connect_cis() return one hci_conn hold for the
ISO layer. A new channel association consumes that hold, which is
eventually released by iso_conn_free().
There are two cases where iso_chan_add() does not create an association:
it returns success when the socket is already attached to the same
iso_conn, and it returns -EBUSY when another socket is attached. The
hold returned for the current call is unused in both cases. This occurs
when deferred setup calls iso_connect_cis() again for its existing
socket, or when another socket attempts to reuse the CIS.
Detect the idempotent case while the connection is locked and release
the unused hold after iso_chan_add(). Also release it on -EBUSY. Do not
drop it for other errors: a newly allocated iso_conn releases the
transferred hold when its last temporary reference is put.
Fixes:
|
||
|
Aldo Ariel Panzardo
|
e06d549fcd |
Bluetooth: hci_conn: fix CIS hold ownership on reuse
Commit |
||
|
Lee Jones
|
71af682ba4 |
Bluetooth: mgmt: Dequeue pending mesh_send_sync entries on cancel
In send_cancel(), pending mesh_tx objects are removed from the
hdev->mesh_pending list and freed via mesh_send_complete(). However, if
a mesh transmission was already queued onto hdev->cmd_sync_work_list via
mesh_next(), the queued entry retains a raw pointer to mesh_tx.
When hci_cmd_sync_work later processes the entry, it attempts to execute
mesh_send_sync and its destroy callback mesh_send_start_complete using
the already freed mesh_tx pointer, leading to a use-after-free.
Fix this by invoking hci_cmd_sync_dequeue() for mesh_send_sync on the
target mesh_tx before completing it. If the entry is found and dequeued,
its destroy callback will complete and free the object; otherwise,
mesh_send_complete() is called directly.
Additionally, ensure the transmission queue advances after cancellation
or errors. In mesh_send_start_complete(), call mesh_next() on error
unless err is -ECANCELED, because hci_cmd_sync_dequeue() holds
hdev->cmd_sync_work_lock and calling mesh_next() synchronously would
deadlock. Instead, advance the queue in send_cancel() once the lock is
released and if no transmission is in progress.
Fixes:
|
||
|
Christiano Amora
|
f033482d76 |
Bluetooth: SMP: reject Security Request over BR/EDR
Bose QC Ultra Headphones (dual-mode, same public address on both
transports) occasionally send an SMP Security Request on the BR/EDR
SMP fixed channel right after the ACL link is encrypted. The kernel
handles it as if it were an LE link: smp_cmd_security_req() has no
transport check, smp_ltk_encrypt() looks up an LTK with the ACL
connection's dst_type, and hci_find_ltk() matches the peer's LE LTK
because the LE public address type is stored as ADDR_LE_DEV_PUBLIC (0),
the same value as BDADDR_BREDR. HCI_OP_LE_START_ENC is then issued on
the ACL handle, the controller rejects it with Invalid HCI Command
Parameters, and hci_cs_le_start_enc() disconnects the link with
HCI_ERROR_AUTH_FAILURE. The headphones drop within a second of
connecting, before any profile is up; a manual reconnect works.
btmon (MediaTek MT7922, kernel 7.0.12):
> HCI Event: Encryption Change (0x08) plen 4
Status: Success (0x00)
Handle: 50 Address: BC:87:FA:47:73:5E (Bose Corporation)
Encryption: Enabled with AES-CCM (0x02)
> ACL Data RX: Handle 50 flags 0x02 dlen 6
BR/EDR SMP: Security Request (0x0b) len 1
Authentication requirement: No bonding, No MITM, SC (0x08)
< HCI Command: LE Start Encryption (0x08|0x0019) plen 28
Handle: 50 Address: BC:87:FA:47:73:5E (Bose Corporation)
> HCI Event: Command Status (0x0f) plen 4
LE Start Encryption (0x08|0x0019) ncmd 1
Status: Invalid HCI Command Parameters (0x12)
< HCI Command: Disconnect (0x01|0x0006) plen 3
Handle: 50 Address: BC:87:FA:47:73:5E (Bose Corporation)
Reason: Authentication Failure (0x05)
SMP over BR/EDR is limited to cross-transport key derivation; the
Security Request procedure (Core Specification Vol 3, Part H, Section
2.4.6, PDU in Section 3.6.7) has no BR/EDR counterpart. Reply with
Pairing Failed / Command Not Supported on a non-LE link, before the PDU
is parsed, and keep the connection. The reply is sent directly rather
than through smp_failure(): rejecting a command on the wrong transport
is not an authentication failure, and MGMT_EV_AUTH_FAILED would make
bluetoothd disconnect the device.
Tested on the affected host (kernel 7.0.12, MediaTek MT7922, Bose QC
Ultra) with the patched module built out of tree: 7 days and 49
reconnects without a drop, against 2 drops in the 3 days before the
patch. Every disconnect in that week had a userspace or remote reason.
Fixes:
|
||
|
Juan Perdomo
|
801fb950ca |
Bluetooth: RFCOMM: avoid socket lock inversion in listener cleanup
rfcomm_sock_cleanup_listen() closes unaccepted child sockets through
rfcomm_sock_close(), which takes the child socket lock before
rfcomm_dlc_close() acquires rfcomm_mutex. The RFCOMM worker takes these
locks in reverse order while handling connections and DLC state changes,
so lockdep reports a possible deadlock.
Close dequeued children without taking their socket lock. The accept queue
owns a reference to each child, and bt_accept_dequeue() locks the child
while unlinking it and clearing its parent pointer.
Dropping the child lock makes it important to prevent a concurrent
rfcomm_connect_ind() from enqueueing a new child after cleanup observes an
empty queue. Set a listening socket to BT_CLOSED while its lock is still
held, before dropping the lock and draining the queue. The state check in
rfcomm_connect_ind() then rejects new children once cleanup starts.
Reported-by: syzbot+0cece8fa7d83523f47a3@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0cece8fa7d83523f47a3
Fixes:
|
||
|
Radek Podgorny
|
555cd2bd86 |
Bluetooth: keep dst_type with dst when reusing an LE connection
hci_connect_le() swaps the caller's identity address for the peer's
cached RPA when one is known, and stamps the matching
ADDR_LE_DEV_RANDOM on the local dst_type. On the conn-reuse path only
the address is copied into the connection:
if (conn) {
bacpy(&conn->dst, dst);
so conn->dst ends up holding an RPA while conn->dst_type still names the
identity it was resolved from, and hci_le_create_conn_sync() puts that
pair on air unchanged. An RPA declared as a public address is not
something any peer can answer.
Measured on a CYW43438 against a peer advertising an RPA the host holds
the IRK for, connecting to the identity address over a raw L2CAP socket.
The first attempt creates the connection, the second takes the reuse
path:
LE Create Connection 3C:78:95:78:37:C3 type public
LE Create Connection 5B:75:A2:26:D6:18 type public
LE Connection Complete: Unknown Connection Identifier (0x02)
The second address is the peer's RPA. btmon annotates it with an OUI
lookup rather than "(Resolvable)" precisely because the command declares
it public; the same bit pattern annotates as resolvable once the type is
right.
The mistyped pair is also why nothing downstream repairs it.
hci_bdaddr_is_rpa() tests the type before the address, so an RPA carrying
a public type is not recognised as one, and hci_find_irk_by_addr() then
searches for an identity address that does not match it either.
Copy the type along with the address.
The assignment used to be unconditional just below this block and covered
both paths; it moved into hci_conn_add_unset(), which the reuse path does
not go through.
Cc: stable@vger.kernel.org
Fixes:
|
||
|
Luiz Augusto von Dentz
|
296e7f3c50 |
Bluetooth: ISO: set BT_LISTEN before requesting a BIG sync
A BIS connection is matched to its parent socket by looking for a
socket in BT_LISTEN state with the same BIG handle:
iso_conn_ready()
if (test_bit(HCI_CONN_BIG_SYNC, &hcon->flags))
parent = iso_get_sock(hdev, &hcon->src, &hcon->dst,
BT_LISTEN, iso_match_big_hcon, hcon);
The socket was only moved to BT_LISTEN after iso_conn_big_sync()
returned, while the LE BIG Create Sync command has already been queued
by then. If the BIG sync is established before the state is updated,
which is easy to hit with an emulated controller as the command may
complete in a few hundred microseconds, no parent is found and the BIS
connections are never notified to the listening socket.
The user space is then left waiting for connections that never arrive,
e.g. bluetoothd never completes a MediaTransport1.Acquire of a
Broadcast Sink transport.
Move the socket to BT_LISTEN before requesting the BIG sync, so the
state is visible by the time the command is queued, and restore the
previous state if the request could not be started. Since the socket is
briefly visible as a listening socket, child sockets may have been
queued in the meantime, so drain the accept queue before restoring the
state: the cleanup paths of BT_CONNECT2/BT_CONNECTED don't do it and the
children would be left with a dangling parent pointer.
Fixes:
|
||
|
Luiz Augusto von Dentz
|
ca18ee413a |
Bluetooth: ISO: Fix parent socket leak in iso_conn_ready()
iso_get_sock() returns the parent socket with a reference held, which is
dropped by sock_put() once the child socket has been set up. The error
path taken when iso_sock_alloc() fails only calls release_sock() and
returns, leaking the reference and thus the parent socket itself.
Drop the reference on that path as well.
Fixes:
|
||
|
Chengfeng Ye
|
9a10987a2f |
Bluetooth: hci_sync: Serialize local codec list cleanup
hci_dev_close_sync() clears hdev->local_codecs after releasing hdev->lock.
Codec list additions and both traversals in sco_sock_getsockopt() use that
lock, but the close path does not. A close and BT_CODEC query can therefore
interleave as follows:
hci_dev_close_sync() sco_sock_getsockopt()
hci_dev_lock()
fetch codec entry
hci_codec_list_clear()
kfree(entry)
read entry->id
The reader then accesses an entry which the close path has freed. KASAN
reported:
BUG: KASAN: slab-use-after-free in sco_sock_getsockopt+0xfa0/0xfe0
Read of size 1 at addr ffff8881001c3450
Call Trace:
sco_sock_getsockopt+0xfa0/0xfe0
do_sock_getsockopt+0x537/0x7b0
__sys_getsockopt+0xf2/0x170
Allocated by task 92:
hci_codec_list_add.isra.0+0x2c/0x440
hci_read_codec_capabilities+0x224/0x590
hci_read_supported_codecs+0x2c2/0x640
Freed by task 92:
kfree+0x131/0x3c0
hci_codec_list_clear+0xd8/0x160
hci_dev_close_sync+0x92a/0xfa0
Take hdev->lock around the clear operation at its existing point in the
close path. This makes the clear wait for active readers and prevents a new
traversal until the list is empty without changing teardown ordering.
Fixes:
|
||
|
Laxman Acharya Padhya
|
d0795cfd6f |
Bluetooth: hci_codec: validate vendor codec count length
The Read Local Supported Codecs parsers consume the variable-sized standard codec array before parsing the vendor codec count. Although the initial reply-size check includes a vendor count byte in the fixed layout, it does not guarantee that the byte remains after the standard codec array. If a controller reply ends immediately after that array, calculating the vendor codec array size reads vnd_codecs->num beyond the skb data. Use skb_pull_data() to validate and consume each codec header before using its count in both command variants. Fixes: |
||
|
Radek Podgorny
|
4914c49989 |
Bluetooth: put the peer's on-air address on air when we cannot resolve
An identity address only reaches a peer that is advertising an RPA if the
controller resolves it on our behalf. Where it cannot, the host has to put
the peer's on-air address on air itself.
hci_connect_le() still swaps the caller's identity address for the peer's
cached RPA before creating the connection, but __hci_conn_add() resolves
the RPA back to the identity address when it stores it, so the identity is
what goes out. Storing the identity is right when the controller
translates it on the way to the radio; without LL Privacy, or with this
peer absent from the resolving list, nothing does.
A peer advertising an RPA cannot answer its identity address, so the
attempt burns a full create-connection timeout. That is not merely a slow
connect: a controller without extended scanning cannot scan while it is
initiating, so every dead attempt also takes the scanner off the air for
the whole timeout.
Measured on a CYW43438, which reports neither LL Privacy nor extended
advertising (LE features 3f 00 00 08 00 00 00 00), against a peer
advertising a resolvable private address the host holds the IRK for, with
the connection requested on the peer's identity address:
before: LE Create Connection to the identity address, public type
1.61s -> 22.07s, then LE Create Connection Cancel
LE Connection Complete: Unknown Connection Identifier (0x02)
after: LE Create Connection to the peer's RPA, random type
LE Connection Complete: Success
Advertising reports reaching the host per second, same window, same five
unrelated devices on the adapter:
before 1s:2 [nothing from 2s through 21s] 22s:5 23s:3
after 0s:11 1s:5 2s:2 3s:5 4s:3 5s:4 ... 21s:2 22s:1 23s:2
One dead connect costs twenty seconds of scanning for every device on the
adapter, not just the one being dialled.
Keep the RPA in conn->dst unless the controller will translate the
identity address: address resolution enabled and the peer's identity
actually programmed into the resolving list. Testing ll_privacy_capable()
alone would not be enough: it reports the feature bit, not whether
resolution is switched on and not whether this peer is in the list.
Resolution is cleared with the other volatile flags on power-off and
switched off again while suspend pauses scanning, and a peer's IRK is only
programmed along the accept list path, so a direct-connect target, a peer
without HCI_CONN_FLAG_ADDRESS_RESOLUTION, and one that did not fit in a
full list are all absent from it.
With the peer programmed, the identity address stays in conn->dst and the
controller translates it: measured on an Intel controller, the host dials
the identity and LE Enhanced Connection Complete reports Resolved Public
with the peer's RPA in the separate peer resolvable private address field.
With the peer absent from the list the same setup dials the RPA itself.
Everything downstream already copes with an RPA in conn->dst: it is what
every outgoing LE connection stored before
|
||
|
Weiming Shi
|
d236517c26 |
Bluetooth: coredump: Quiesce dump work on unregister
hci_devcd_handle_pkt_init() arms dump_timeout and coredump producers
queue dump_rx without holding an hdev reference. Unregister leaves both
works live, so disconnecting during an active dump lets them access hdev
after hci_release_dev() frees it.
Shut down coredump processing during unregister. Close the producer gate
under dump_q.lock before disabling both works, then free the active buffer
and queued packets under hci_dev_lock. Serializing the gate with enqueue
prevents controller-specific workers from adding packets after the final
purge.
Fixes:
|
||
|
ThangNN99
|
6610c6fe4b |
Bluetooth: hci_core: Fix queuing tx_work after workqueue is drained
hci_send_acl(), hci_send_sco() and hci_send_iso() queue hdev->tx_work
unconditionally. They can run from the L2CAP/SCO/ISO socket send path
while hci_dev_close_sync() is draining hdev->workqueue (HCIDEVDOWN
racing with a socket write). Since that queue_work() is not chained
work from the tx_work worker itself, __queue_work() sees the queue
marked __WQ_DRAINING, warns "cannot queue %ps on wq %s", and drops
the work:
WARNING: CPU: 1 PID: 5985 at kernel/workqueue.c:2352 __queue_work
Call Trace:
queue_work_on
l2cap_chan_send
l2cap_sock_sendmsg
...
hci_dev_close_sync() already sets HCI_CMD_DRAIN_WORKQUEUE before
draining, but only hci_cmd_work() and handle_cmd_cnt_and_timer()
check it before queuing. Route the tx_work producers through the
same guard via a shared hci_sched_tx() helper.
Fixes:
|
||
|
Aamir Ahmed
|
e824176679 |
Bluetooth: eir: validate service data length before reading UUID
eir_get_service_data() reads a 16-bit UUID from the service data using
get_unaligned_le16() without first checking that the data is long enough
to hold a UUID16 (2 bytes). If a malformed EIR entry has a service data
field with only 1 byte of payload (field_len=2), eir_get_data() returns
dlen=1. The subsequent get_unaligned_le16() then reads 1 byte past the
field boundary.
Additionally, if the corrupted UUID happens to match, the length
calculation "dlen - 2" underflows to SIZE_MAX since dlen is size_t.
Current callers either pass NULL for the length parameter or bounds-check
the returned length, but future callers may not.
Add a check that dlen >= sizeof(u16) and skip fields that are too short
to contain a valid UUID16.
Fixes:
|
||
|
Linus Torvalds
|
7844502343 |
Nothing too exciting, usual stream of fixes.
Including fixes from Netfilter, Bluetooth and WPAN.
Current release - new code bugs:
- Bluetooth: hci_sync: fix not setting CE length properly
- eth: enic: match mailbox replies to request numbers
Previous releases - regressions:
- tunnels: drop stale dst when building an ICMP error for PMTUD
- ipv6: null-check fib6_node before accessing in __ip6_del_rt_siblings()
(bug in the rtnl_lock -> RCU conversion)
- eth: bnxt_en: fix crashes on Thor2 due to OOB coalescing buffer accesses
- eth: bnxt_en: prevent queue stop with deferred completions
Previous releases - always broken:
- eth: ice: don't dereference pointers from TP_printk()
- eth: fix OOB writes on ethtool flow rule dump in 3 drivers
- eth: mlx5: fix FEC configuration with RS_544_514_INTERLEAVED_QUAD
- dsa: tag_brcm: legacy FCS: request needed tailroom
Misc:
- net: cap tx_queue_len at S16_MAX to prevent oversized ring alloc
- ipv6: flowlabel: cap duplicate leases per socket
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
-----BEGIN PGP SIGNATURE-----
iQIzBAABCgAdFiEE6jPA+I1ugmIBA4hXMUZtbf5SIrsFAmqi3pQACgkQMUZtbf5S
IrvUIg//X9nIxY2F5PzJ5jD9p5ccXrQMLe7kT5AW2tP5PDC8d5PIv4Q5XzFKQPU7
XElAUKvBxmofwU2lqILYGi8AeUpqHZtKPY7XzKeqd6i72KOD6mGzzYNijqttBXcM
vFVtIeKExXjAwvNc2as1SeXVEAAAkBtrCFuMNHMq0C56yK4md/XVkCDHaJkomNit
geke1U8gut3rZddWKxp4WDbL8Wmx9yM0uDMBznO/+cwITObA0Hme3IgRndglzz7n
n4Ih+EG4tRrD3kUf6oePzKQ47cd+qnSVlVTCZUwB5E/HKqWJFXxSN4Sv+mez0sAS
rrI5hl+luNKUYrZ8/jiNlvajgAL4+AYpCKPDJbXrOW+z+x4BC2VYZBAHLoUr5ZAq
Z5OYU9SgD1oGntqkI8mAEiRTEu+4gjhIEhjENHEzqdjUogaBIp7MWwCrNBAnFWvs
2McmNfZZMVhxKpyYnndUStsVQySVPASb0CXeqTIO6PJsAp/HBjoMYKYKPhhwk0Gp
lE8zHjEnPVofRfXfT+oZnbS8is2nC9FjBy9ksIGcC7vyTOdPsoIBoB8JY0x/INRM
SOJvyxrdnVkMjiBejkdOa5X9HbD1cA/NVyzT2WEaZBGPmIqfNBgUIOxnXD4CShuy
9zX8qtHsUmmYxPteF30Uhfe0kyLQ9OnjUz2Bl++EruxpIE3+i48=
=f9J9
-----END PGP SIGNATURE-----
Merge tag 'net-7.3-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
Pull networking fixes from Jakub Kicinski:
"Nothing too exciting, usual stream of fixes. Including fixes from
Netfilter, Bluetooth and WPAN.
Current release - new code bugs:
- Bluetooth: hci_sync: fix not setting CE length properly
- eth: enic: match mailbox replies to request numbers
Previous releases - regressions:
- tunnels: drop stale dst when building an ICMP error for PMTUD
- ipv6: null-check fib6_node before accessing in __ip6_del_rt_siblings()
(bug in the rtnl_lock -> RCU conversion)
- eth: bnxt_en:
- fix crashes on Thor2 due to OOB coalescing buffer accesses
- prevent queue stop with deferred completions
Previous releases - always broken:
- eth:
- ice: don't dereference pointers from TP_printk()
- fix OOB writes on ethtool flow rule dump in 3 drivers
- mlx5: fix FEC configuration with RS_544_514_INTERLEAVED_QUAD
- dsa: tag_brcm: legacy FCS: request needed tailroom
Misc:
- net: cap tx_queue_len at S16_MAX to prevent oversized ring alloc
- ipv6: flowlabel: cap duplicate leases per socket"
* tag 'net-7.3-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net: (164 commits)
selftests: tc-testing: test action batch failure cleanup
net/sched: act_api: release all action references on NEWACTION failure
openvswitch: fix wrong flag value in get_ipv6_ext_hdrs()
ipmr: account multicast table and route memory
net: phy: dp83td510: handle the active-high LED polarity mode
net: macb: initialize PTP state before registering clock
net: hsr: enable promiscuous mode on interlink port with fwd offload
ipv6: fix fib6 walker UAF on seq stop
net: stmmac: fix TX descriptor availability check for TSO traffic
net/rds: fix tcp stream corruption with large pages
net: mana: restore the XDP program pointer when pre-allocation fails
net: phy: dp83867: handle the active-high LED polarity mode
octeontx2-af: fix PF/CGX debugfs PCI bus lookup
net: net_failover: Fix the deadlock in net_failover_slave_name_change()
net: phy: mediatek-ge: disable EEE on the MT7530 PHY
tcp: reject non zerocopy devmem tx
net: ethernet: mtk_eth_soc: populate lpi_interfaces to fix EEE support
net: dsa: mt7530: populate lpi_interfaces to fix EEE support
net: hinic: fix mailbox segment buffer overflow
net: sun4i-emac: fix missing of_node_put() for phy_node
...
|
||
|
Krystian Kaniewski
|
9b851b09b3 |
Bluetooth: hci_sysfs: Fix NULL pointer dereference in device_del()
A NULL pointer dereference in klist_put() occurs when a child device (such as a BNEP network device in bnep_session) is concurrently being unregistered while hci_conn_del_sysfs() reparents child devices. This is caused by a race condition between hci_conn_del_sysfs() and concurrent child device unregistration (e.g. bnep_session calling unregister_netdev()). During device unregistration, device_del() snapshots a non-NULL parent pointer. Concurrently, hci_conn_del_sysfs() finds the child device using device_find_any_child() and calls device_move() to reparent it to NULL, which removes the node from its parent's klist and clears knode_parent. Subsequently, device_del() calls klist_del(&dev->p->knode_parent) using the stale parent snapshot, causing klist_put() to dereference knode_klist(n)->put on an already removed node, resulting in a NULL pointer dereference. This race was introduced by commit |
||
|
Luiz Augusto von Dentz
|
3621f78d43 |
Bluetooth: hci_sync: Fix not setting CE length properly
Both hci_le_set_def_rate_sync() and hci_le_conn_rate_request_sync() were
leaving Min_CE_Length and Max_CE_Length set to 0x0000, but the connection
event length recommended in requests by a Peripheral has a valid range of
0x0001 to 0x7CFF (Time = N * 125 us, Time Range: 0.125 ms to 3.999875 s),
so 0x0000 cannot be used.
Set both to the minimum valid value, which is safe since the Controller
is not required to use these values:
BLUETOOTH CORE SPECIFICATION Version 6.2 | Vol 4, Part E
7.8.157. LE Connection Rate Request command
7.8.158. LE Set Default Rate Parameters command
The Min_CE_Length and Max_CE_Length parameters provide the Controller
with the expected minimum and maximum length of the connection events.
The Controller is not required to use these values.
Fixes:
|
||
|
Kees Cook
|
3a2c4d55e3 |
treewide: refresh kmalloc_obj() conversions
This is another run of the Coccinelle script for converting kmalloc() family of allocations to kmalloc_obj() via the existing rules in scripts/coccinelle/api/kmalloc_objs.cocci This catches both the set of kmalloc() uses added since the first kmalloc_obj() conversions in v7.0 and adds a large group missed in the first pass due to Coccinelle not interacting well with the cleanup.h scoped_...() family of macros[1]. I worked around this with spatch's "--macro-file" argument to a file with all the scoped_...() macros mapped to Coccinelle's YACFE_ITERATOR[2] as that was the closest viable control flow indicator I could find. Build tested allmodconfig on x86, arm64, arm, loongarch, mips, powerpc, riscv, and s390 with no new warnings. Link: https://lore.kernel.org/lkml/202609021314.8A9C0B8@keescook/ [1] Link: https://github.com/coccinelle/coccinelle/blob/master/standard.h [2] Signed-off-by: Kees Cook <kees+treewide@kernel.org> |
||
|
Pauli Virtanen
|
0d77683237 |
Bluetooth: L2CAP: clear FLAG_DEFER_SETUP only for same PID/PSM
l2cap_ecred_defer_connect() clears FLAG_DEFER_SETUP also for channels
with different PID/PSM, which will not be added to the same
ECRED_CONN_REQ in any case. Consequently, only one ECRED connection
group can work at a time although it appears intended they would be
separate for each PID/PSM combination.
Fix by clearing FLAG_DEFER_SETUP only for the connections that could be
added in the request. Retain test_bit(FLAG_DEFER_SETUP) before calling
get_peer_pid as it may be NULL otherwise.
Fixes:
|
||
|
Pauli Virtanen
|
56c2b5831d |
Bluetooth: L2CAP: fix out-of-bounds write in l2cap_ecred_connect
l2cap_chan_connect() tries to ensure there are no more than
L2CAP_ECRED_CONN_SCID_MAX pending ECRED channels, so they fit in the
same L2CAP_ECRED_CONN_REQ that l2cap_ecred_connect() constructs.
However, the check only counts deferred channels. If 6 L2CAP sockets
are connected at the same time in order DDDDND (D=deferred,
N=non-deferred), the last can bump the total to max+1. It results to
one __le16 written out of bounds of the scid array, and an invalid
ECRED_CONN_REQ being sent.
Fix by leaving room for the non-deferred pending ECRED channels in the
counting in l2cap_chan_connect(), so the limit can't be exceeded.
Move counting under same critical section where the channel is added.
Although race conditions involving this appear unreachable, it's easier
to see.
Also add WARN_ON_ONCE check in l2cap_ecred_defer_connect() to make this
less brittle.
Fixes:
|
||
|
Pauli Virtanen
|
4ef05db5b0 |
Bluetooth: L2CAP: fix chan mode for LE_CONN_REQ + EXT_FLOWCTL pchan
l2cap_new_connection() sets default value of channel mode to match the
parent channel. l2cap_le_connect_req() left this at the default, and
created L2CAP_MODE_EXT_FLOWCTL channels if listening pchan has that
mode. This causes FLAG_DEFER_SETUP channels to reply to
L2CAP_LE_CONN_REQ with L2CAP_ECRED_CONN_RSP, which is incorrect.
It can also result to stack OOB write (of l2cap_alloc_cid determined
values) in l2cap_ecred_rsp_defer(), as l2cap_le_connect_req() does not
limit maximum number of deferred channels or check for duplicate ident.
Fix by setting chan->mode correctly in l2cap_le_connect_req().
Also check channel mode in l2cap_ecred_rsp_defer(), and do WARN_ON_ONCE
instead of OOB write to make it less brittle.
Fixes:
|
||
|
Aleksandr Nogikh
|
57938bbdb9 |
Bluetooth: hci_core: Fix race condition during device registration
In hci_register_dev(), the power_on work item is queued to
hdev->req_workqueue before initializing hdev->adv_monitors_idr and
registering the MSFT extension via msft_register(). For devices marked with
quirks such as HCI_QUIRK_RAW_DEVICE, the HCI_UNCONFIGURED flag is set on
the device. When the power_on work item runs concurrently on another CPU,
hci_power_on() detects that the device is unconfigured and immediately
invokes hci_dev_do_close(), which calls msft_do_close().
Concurrently, msft_register() allocates the msft structure and exposes it
to hdev->msft_data prior to calling mutex_init(&msft->filter_lock). If
msft_do_close() executes while hdev->msft_data is already assigned but the
mutex has not yet been initialized, mutex_lock(&msft->filter_lock) operates
on an uninitialized mutex, triggering a DEBUG_LOCKS warning:
DEBUG_LOCKS_WARN_ON(lock->magic != lock)
WARNING: kernel/locking/mutex.c:625 at __mutex_lock_common
kernel/locking/mutex.c:625 [inline]
WARNING: kernel/locking/mutex.c:625 at __mutex_lock+0x12d8/0x1550
kernel/locking/mutex.c:821
...
Call Trace:
<TASK>
msft_do_close+0x308/0x7b0 net/bluetooth/msft.c:693
hci_dev_close_sync+0x86b/0x10a0 net/bluetooth/hci_sync.c:5522
hci_dev_do_close net/bluetooth/hci_core.c:499 [inline]
hci_power_on+0x32c/0x750 net/bluetooth/hci_core.c:937
process_one_work kernel/workqueue.c:3322 [inline]
process_scheduled_works+0xa8e/0x14e0 kernel/workqueue.c:3405
worker_thread+0x92d/0xe10 kernel/workqueue.c:3486
kthread+0x388/0x470 kernel/kthread.c:436
ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
</TASK>
Fix this by moving the queue_work() call in hci_register_dev() to after
idr_init(&hdev->adv_monitors_idr) and msft_register(hdev) so that device
structures and extensions are fully initialized before asynchronous tasks
can access them. Additionally, assign hdev->msft_data in msft_register()
only after mutex_init(&msft->filter_lock) has completed.
Fixes:
|
||
|
Chengfeng Ye
|
ebe6674292 |
Bluetooth: RFCOMM: serialize session teardown
rfcomm_kill_listener() walks session_list and deletes every session
without holding rfcomm_mutex, unlike the normal session processing and
connect error paths.
Under normal operation, an open RFCOMM socket pins rfcomm.ko, so
rfcomm_kill_listener() does not run concurrently with rfcomm_dlc_open().
However, forced module unload via delete_module(O_TRUNC) can stop
krfcommd while a failed connect is still unwinding.
connect task forced unload / krfcommd
------------ ------------------------
rfcomm_lock()
rfcomm_session_add()
delete_module("rfcomm", O_TRUNC)
rfcomm_kill_listener()
fetch session from session_list
kernel_connect() fails
rfcomm_session_del()
remove and free session
rfcomm_session_del(session)
The final call then reads the freed session and may corrupt the list.
KASAN reported with mdelay() to enlarge critical window:
BUG: KASAN: slab-use-after-free in rfcomm_run+0x3802/0x3f00 [rfcomm]
Read of size 8 at addr ffff888111058d40 by task krfcommd/79
Tainted: [R]=FORCED_RMMOD
Allocated by task 86:
rfcomm_session_add+0xa1/0x300 [rfcomm]
rfcomm_dlc_open+0x8b2/0xf30 [rfcomm]
rfcomm_sock_connect+0x34c/0x530 [rfcomm]
Freed by task 86:
kfree+0x121/0x3c0
rfcomm_dlc_open+0xab7/0xf30 [rfcomm]
rfcomm_sock_connect+0x34c/0x530 [rfcomm]
Hold rfcomm_mutex across the teardown traversal so every reachable
session_list walk uses the same serialization.
Reviewed-by: Ali Ahmet Memis <ali@iusegentoo.com>
Tested-by: Ali Ahmet Memis <ali@iusegentoo.com>
Reviewed-by: Pauli Virtanen <pav@iki.fi>
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
|