[PW_SID:1163708] [v2] Bluetooth: btusb: Add USB ID 13d3:3626 for RTL8851BE - #759
BluezTestBot wants to merge 119 commits into
Conversation
…uest A synchronous HCI command that never receives a response leaves HCI_CMD_PENDING set: hci_req_cmd_complete() is the only place that clears it, and it only runs when a response matching the last command sent arrives. hci_send_cmd_sync() populates hdev->req_skb only when the flag transitions from clear to set, while hci_dev_open_sync() and hci_dev_close_sync() drop req_skb without clearing the flag. After a timeout followed by either, the two disagree: the flag claims a request is outstanding while req_skb is NULL. Subsequent synchronous commands are then sent with no req_skb, so hci_event_packet() has nothing to match an arriving event against, and the caller times out even though the controller answered. Commands answered by Command Complete recover on their own, since hci_req_cmd_complete() clears the flag as a side effect. Drivers using __hci_cmd_sync_ev() with a custom event do not, because a vendor event never reaches that path. On a WCN3988 (hci_qca over UART) this makes a controller firmware hang unrecoverable: the driver injects a hardware error and re-runs qca_setup(), qca_read_soc_version() waits for HCI_EV_VENDOR, the reply arrives within 4 ms and is discarded, and every retry fails the same way. The adapter is left down until the driver is unbound and rebound, or power is removed. Clear the flag wherever the last request is dropped, restoring the invariant that req_skb is non-NULL exactly when HCI_CMD_PENDING is set. Verified on hardware by forcing a command timeout: without this change setup fails on every attempt, with it setup succeeds on the first. Fixes: 2615fd9 ("Bluetooth: hci_sync: Fix overwriting request callback") Cc: stable@vger.kernel.org Signed-off-by: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
The hci_dev_init_sync() failure path in hci_dev_open_sync() and the cleanup code in hci_dev_close_sync() have a bunch of common code. Factor this duplicate code out into a hci_dev_drop_last_cmd_req_and_close() helper function. Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
The strcpy() function is deprecated across the kernel tree and moving towards complete elimination. It provides no verification limits against buffer overflows and does not guarantee strict boundary restrictions [1][2]. Replace instances of strcpy() in `net/bluetooth/bnep/core.c` with the safer strscpy() alternative. Since both target destination blocks are statically allocated fixed-size arrays within their structure definitions, leverage the compile time sizeof() operator to explicitly pass the destination buffer capacities. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy [1] Link: KSPP/linux#88 [2] Signed-off-by: Ajith P V <ajithpv.linux@gmail.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
In btmrvl_process_event(), all error handling paths except one do a direct return. Update the only one that makes a goto to be consistent. This does not change the behavior because ret is known to be != 0 when 'exit' is reached. This simplifies the code, saves 2 LoC and pleases one of my coccinelle script that tries to spot erroneously mixed goto and return statements. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
The IMC Networks Bluetooth controller with USB ID 13d3:3558 uses an RTL8821CE. Without BTUSB_REALTEK, btusb uses generic initialization and does not load the controller firmware. BLE connections then fail before pairing with HCI error 0x3e. Add the device ID to the RTL8821CE table. This enables loading rtl_bt/rtl8821c_fw.bin and rtl_bt/rtl8821c_config.bin, after which a BLE HID keyboard pairs successfully. Signed-off-by: Richard Nunley <richard.w.nunley@gmail.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
…gative bcm_request_irq() calls pm_runtime_use_autosuspend(), but bcm_close() does not call the matching pm_runtime_dont_use_autosuspend() when tearing down runtime PM. If the autosuspend delay is set to a negative value while autosuspend is enabled, the runtime PM core increments usage_count to prevent runtime suspend. Without calling pm_runtime_dont_use_autosuspend() during driver teardown, this reference is not dropped and usage_count remains unbalanced. Add the missing pm_runtime_dont_use_autosuspend() call before disabling runtime PM. This issue was found by manual code inspection. Fixes: e88ab30 ("Bluetooth: hci_bcm: Add suspend/resume runtime PM functions") Cc: stable@vger.kernel.org Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
…ative h5_btrtl_open() calls pm_runtime_use_autosuspend(), but h5_btrtl_close() does not call the matching pm_runtime_dont_use_autosuspend() when tearing down runtime PM. If the autosuspend delay is set to a negative value while autosuspend is enabled, the runtime PM core increments usage_count to prevent runtime suspend. Without calling pm_runtime_dont_use_autosuspend() during driver teardown, this reference is not dropped and usage_count remains unbalanced. Add the missing pm_runtime_dont_use_autosuspend() call before disabling runtime PM. This issue was found by manual code inspection. Fixes: d9dd833 ("Bluetooth: hci_h5: Add runtime suspend") Cc: stable@vger.kernel.org Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
…negative intel_set_power() calls pm_runtime_use_autosuspend() when powering on the device, but the power-off path does not call the matching pm_runtime_dont_use_autosuspend() before disabling runtime PM. If the autosuspend delay is set to a negative value while autosuspend is enabled, the runtime PM core increments usage_count to prevent runtime suspend. Without calling pm_runtime_dont_use_autosuspend() during teardown, this reference is not dropped and usage_count remains unbalanced. Add the missing pm_runtime_dont_use_autosuspend() call before disabling runtime PM. This issue was found by manual code inspection. Fixes: 74cdad3 ("Bluetooth: hci_intel: Add runtime PM support") Cc: stable@vger.kernel.org Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Since commit b66774b ("Bluetooth: L2CAP: Fix UAF in channel timeout by holding conn ref") l2cap_chan::conn has held reference and remains non-NULL also after the corresponding hci_conn is deleted. In this state accessing various fields eg. hci_conn::hdev is invalid, which leads to KASAN crash in l2cap_sock_setsockopt() access of conn->hcon->hdev. Check l2cap_chan::conn.hcon corresponds to an alive hci_conn before trying to use it in l2cap_sock.c. Hold l2cap_chan_lock() in getsockopt/setsockopt to ensure it stays alive, and to avoid data races in l2cap_chan fields. Fixes: b66774b ("Bluetooth: L2CAP: Fix UAF in channel timeout by holding conn ref") Reported-by: syzbot+b106284c2a0b7bc80cf9@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?extid=b106284c2a0b7bc80cf9 Signed-off-by: Pauli Virtanen <pav@iki.fi> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
New sk should not be added to parent socket accept queue after last
l2cap_sock_cleanup_listen() has run in l2cap_sock_teardown_cb() and
state set to BT_CLOSED, as that can result to UAF on dereferencing the
dangling parent reference.
l2cap_sock_new_connection_cb() may race with parent l2cap_chan teardown,
due to chan->state accessed without consistent locking:
[Task 1] [Task 2]
l2cap_sock_release(parent) l2cap_connect
l2cap_sock_shutdown pchan = l2cap_global_chan_by_psm
l2cap_chan_lock(pchan)
l2cap_chan_close
l2cap_sock_teardown_cb
pchan->state = BT_CLOSED
l2cap_chan_unlock(pchan) ------> l2cap_chan_lock(pchan)
l2cap_new_connection
l2cap_sock_new_connection_cb
l2cap_chan_lock(pchan) <-------- l2cap_chan_unlock(pchan)
l2cap_sock_kill(parent) /* bt_sk(sk)->parent dangling */
Fix by adding check for sk_state == BT_LISTEN after acquiring sk lock in
l2cap_sock_new_connection_cb(). Add lock_sock() around sk_state writes
where missing, to avoid data races.
Although the data races on pchan->state should be fixed too, this
defensive sk_state check probably makes sense in any case.
Fixes: 2ff1a41 ("Bluetooth: L2CAP: Fix null-ptr-deref in l2cap_sock_state_change_cb()")
Reported-by: syzbot+9265e754091c2d27ea29@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9265e754091c2d27ea29
Signed-off-by: Pauli Virtanen <pav@iki.fi>
Reported-by: syzbot+9265e754091c2d27ea29@syzkaller.appspotmail.com
Tested-by: syzbot+9265e754091c2d27ea29@syzkaller.appspotmail.com
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
…or paths When virtbt_open_vdev() fails in virtbt_probe(), hci_free_dev(hdev) is called without first calling hci_unregister_dev(hdev). Since hci_register_dev() already succeeded, the HCI device remains registered while its memory is freed, leading to a use-after-free when accessed via sysfs or HCI sockets. Additionally, the probe function leaks the virtio_bluetooth structure (vbt) in several error paths: - When virtio_find_vqs() fails, vbt is not freed. - When hci_alloc_dev() or hci_register_dev() fails, vbt is not freed. - When virtbt_open_vdev() fails, vbt is not freed. Furthermore, when virtbt_open_vdev() fails after virtio_device_ready() has been called, the device is left live (DRIVER_OK set) while its virtqueues are torn down, and any scheduled work is not flushed, potentially allowing a use-after-free from device-initiated callbacks. Fix all of these by restructuring the error labels to properly unwind in reverse order of the allocation/registration sequence. The new labels err_del_vqs and err_free_vbt ensure that del_vqs and kfree(vbt) are called as appropriate for each failure point. For the virtbt_open_vdev() failure path, call virtio_reset_device() and virtbt_close_vdev() before unregistering the HCI device, matching the cleanup pattern in virtbt_remove(). Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
btmtksdio_close() and btmtksdio_reset() call cancel_work_sync() on bdev->txrx_work while holding the sdio host lock, which is also acquired by btmtksdio_txrx_work(). If txrx_work is queued when close/reset runs, a worker thread may start it after the host lock is taken and block in sdio_claim_host(), while cancel_work_sync() waits for the work to finish. The host lock is only released after cancel_work_sync() returns, so both sides wait forever, deadlocking close/reset. Fix this by releasing the sdio host lock before calling cancel_work_sync(), then re-acquiring it afterwards. In btmtksdio_close() the interrupt is already disabled by sdio_release_irq(), which also unregisters the IRQ handler, so no new work can be scheduled and cancel_work_sync() fully quiesces txrx_work. btmtksdio_reset() must additionally unregister the IRQ handler before dropping the host lock: btmtksdio_txrx_work() unconditionally re-enables the device interrupt (C_INT_EN_SET) when the handler is still registered, so an in-flight worker would re-enable interrupts and be rescheduled while the device is being reset, defeating the cancellation. The IRQ is re-claimed by btmtksdio_open() when the HCI device is re-opened after the reset. This mirrors the pattern already used by btmtksdio_flush(), which cancels the work without holding the host lock. Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
…ce() hci_uart_unregister_device() frees the HCI device (hci_free_dev) before cancelling write_work via cancel_work_sync(). If write_work is executing concurrently on another CPU, it can access hu->hdev and write to hdev->stat after the memory has been freed. Additionally, HCI_UART_PROTO_READY is not cleared until after cancel_work_sync, so the write_wakeup serdev callback can still schedule write_work via hci_uart_tx_wakeup() even after hci_free_dev has freed the device. Fix this by mirroring the same ordering used in the tty/ldisc path (hci_uart_tty_close, hci_ldisc.c:565-593): 1. Save the PROTO_READY state and clear it under the write lock so a concurrent hci_uart_tx_wakeup() cannot re-schedule write_work 2. Cancel write_work (no new work can be scheduled and no work is in flight) 3. Unregister the HCI device 4. Close the protocol (may access hu->hdev and the serdev device) 5. Close the serdev port (safe now that write_work is quiesced and protocol is done) 6. Free the HCI device Also free any partially transmitted frame (hu->tx_skb) left over by write_work once the transmit path is quiesced, since hci_uart_close() would skip hci_uart_flush() because HCI_UART_PROTO_READY is cleared. Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
'uuid_count' member of struct 'discovery_state' is assigned and read
without any locks, so there is a chance of situation when
uuid_count != 0, but uuids is NULL and there will be NULL pointer
dereference.
Possible race:
'hci_update_passive_scan_sync'
'hci_discovery_filter_clear'
hdev->discovery.uuid_count = 0;
<----------------------preempted----------------------------->
'start_service_discovery'
// Set uuid_count to value != 0
hdev->discovery.uuid_count = uuid_count;
hdev->discovery.uuids = kmemdup(...);
<----------------------preempted----------------------------->
spin_lock(&hdev->discovery.lock);
kfree(hdev->discovery.uuids);
hdev->discovery.uuids = NULL;
spin_unlock(&hdev->discovery.lock);
Now uuids == NULL and uuid_count != 0.
So 'mgmt_device_found' -> 'is_filter_match' -> 'eir_has_uuids' receives
non consistent discovery state, where NULL dereference of uuids happens.
To fix it let's add discovery.lock around every read/write of uuid_count,
uuids pair of struct members. It is also important to assign uuid_count
value only after success kmemdup() allocation in
start_service_discovery(), otherwise uuids is NULL, because kmemdup failed,
but uuid_count is already assigned to non zero value.
The following panic happens:
[ ] ------------[ cut here ]------------
[ ] Unable to handle kernel NULL pointer dereference at virtual
address 0000000000000000
[ ] Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP
[ ] CPU: 0 PID: 15056 Comm: kworker/u9:2
[ ] Workqueue: hci0 hci_rx_work
[ ] pstate: 10400009 (nzcV daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ ] pc : eir_has_uuids+0x2d8/0x590
[ ] lr : is_filter_match+0x258/0x320
...
[ ] Call trace:
[ ] eir_has_uuids+0x2d8/0x590
[ ] is_filter_match+0x258/0x320
[ ] mgmt_device_found+0x5b0/0xafc
[ ] process_adv_report.part.0+0x8c8/0xf14
[ ] hci_le_adv_report_evt+0x338/0x3f0
[ ] hci_le_meta_evt+0x1f0/0x4c8
[ ] hci_event_packet+0x440/0xc9c
[ ] hci_rx_work+0x44c/0xaf8
[ ] process_one_work+0x54c/0x103c
[ ] worker_thread+0x6c4/0x10c4
[ ] kthread+0x274/0x2ec
[ ] ret_from_fork+0x10/0x20
[ ] Code: 14000004 91004021 eb14003f 54000180 (f9400024)
[ ] ---[ end trace 0000000000000000 ]---
Fixes: 2935e55 ("Bluetooth: hci_sync: fix double free in 'hci_discovery_filter_clear()'")
Signed-off-by: Pavel Shpakovskiy <pashpakovskii@salutedevices.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
For L2CAP sockets without owning sk->sk_socket, reading
l2cap_pi(sk)->chan may race against concurrent l2cap_sock_kill() ->
l2cap_sock_put_chan(). This excludes simultaneous proto_ops callbacks,
but access in l2cap_sock_cleanup_listen() has unsafe lockless read.
[Task 1] [Task 2 (hdev->workqueue)]
l2cap_sock_release(parent) l2cap_disconn_cfm
l2cap_sock_cleanup_listen l2cap_conn_del
bt_accept_dequeue l2cap_chan_del
lock_sock(sk) l2cap_sock_teardown_cb
bt_accept_unlink
bt_sk(sk)->parent = NULL
release_sock(sk) ----------------> lock_sock(sk)
parent = /* NULL */
lock_sock(sk) <--------------------- release_sock(sk)
sock_set_flag(sk, SOCK_ZAPPED)
l2cap_sock_close_cb
l2cap_sock_kill(sk)
l2cap_sock_put_chan
chan = READ l2cap_pi(sk)->chan l2cap_pi(sk)->chan = NULL
l2cap_chan_hold_unless_zero l2cap_put_chan(chan)
kref_get_unless_zero(&chan->ref)
Task 1 may observe NULL which causes null-ptr-deref.
Fix the race by taking lock_sock() in l2cap_sock_kill() to
synchronize with l2cap_sock_cleanup_listen(). hold_unless_zero() is not
needed here, l2cap_pi(sk)->chan owns reference if it is non-NULL.
Clarify code comments vs. locking.
Fixes: 6fef032 ("Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb()")
Reported-by: syzbot+e6382a2f53f5fc7453ac@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e6382a2f53f5fc7453ac
Signed-off-by: Pauli Virtanen <pav@iki.fi>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
btmtk_usb_subsys_reset() validates the subsystem reset by reading the chip id back. When that read succeeds at the bus level but yields an id of zero, the reset has demonstrably not taken effect: the function logs "Can't get device id, subsys reset fail." and then returns the return value of btmtk_usb_id_get(), which in that case is zero, i.e. success. btusb_mtk_reset() returns that value unchanged, so its caller cannot tell a completed reset from a failed one. Return -ENODEV when the chip id reads back as zero, leaving the existing MT6639 exemption intact. Observed on an MT7902 [13d3:3579]. The path can be reached on demand by asking the controller for a coredump, since btmtk requests a reset once the dump completes: # echo 1 > /sys/class/bluetooth/hci0/device/coredump Bluetooth: hci0: Mediatek coredump end Bluetooth: hci0: Can't get device id, subsys reset fail. usb 3-10: reset high-speed USB device number 5 using xhci_hcd usb 3-10: device descriptor read/64, error -110 usb usb3-port10: attempt power cycle usb usb3-port10: unable to enumerate USB device The same sequence occurs unprompted when the controller firmware asserts on its own. Note that this corrects the error reporting only; it does not by itself make the controller recoverable in the case above. Fixes: 25b6d75 ("Bluetooth: btmtk: introduce btmtk reset work") Signed-off-by: Ismail Tarim <ismailtarim7@gmail.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
When the MTK_BT_RST_DONE poll times out, btmtk_usb_subsys_reset() logs "Reset timeout" and keeps the error in err, but err is then overwritten by the return value of the following btmtk_usb_id_get() call, so the timeout is never reported to the caller. Commit 25b6d75 ("Bluetooth: btmtk: introduce btmtk reset work") discarded the return value of the chip id read, so the function returned the timeout error as intended. Commit 3dcb122 ("Bluetooth: btusb: mediatek: return error for failed reg access") started assigning err at that call and silently dropped it. Keep the timeout in a separate variable and return it, restoring the original behaviour without changing the control flow. Fixes: 3dcb122 ("Bluetooth: btusb: mediatek: return error for failed reg access") Signed-off-by: Ismail Tarim <ismailtarim7@gmail.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
btmtksdio_tx_packet() prepends the MediaTek SDIO header with skb_push() and writes into that space after only checking the headroom size. On a cloned SKB that headroom belongs to a buffer shared with the other owner, which the driver has no right to write to. Cloned SKBs do reach this path: hci_send_cmd_sync() keeps a clone of every HCI command in hdev->sent_cmd before handing the SKB to the driver, and l2cap_ertm_send() clones SKBs for retransmission. Replace the open-coded headroom check with skb_cow_head(), which both guarantees the headroom and reallocates a private buffer when the SKB is cloned. The cost is one reallocation and copy per cloned packet, the usual price of this pattern in network drivers. This has no observable effect on its own, as the driver only writes in front of skb->data where no other owner looks. It is a prerequisite for "Bluetooth: btmtksdio: Fix out-of-bounds DMA read in the TX path", which writes padding behind skb->tail, and carries the same Fixes: tag so that both are backported together. Fixes: 9aebfd4 ("Bluetooth: mediatek: add support for MediaTek MT7663S and MT7668S SDIO devices") Signed-off-by: Chris Lu <chris.lu@mediatek.com> Assisted-by: Claude:claude-opus-5 Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
btmtksdio_tx_packet() rounds the transfer size up to the SDIO block size of 256 bytes, but hands the host controller the SKB buffer as is: err = sdio_writesb(bdev->func, MTK_REG_CTDR, skb->data, round_up(skb->len, MTK_SDIO_BLOCK_SIZE)); Only skb->len bytes hold packet data, so the controller reads up to 255 bytes of uninitialised memory and sends it to the device over the SDIO bus. Depending on how much tailroom slack the SKB allocation happens to carry, that read can also extend past the end of the buffer. Compute the padded length up front, ensure the SKB has tailroom for it, and zero-fill the padding with skb_put_zero(). skb->len then covers the padding, so sdio_writesb() no longer needs to round up. byte_tx keeps counting the header and the payload only, and the error path restores the SKB so that the caller can requeue it. Writing behind skb->tail is only safe because the driver owns the buffer, which "Bluetooth: btmtksdio: Take exclusive ownership of the SKB before TX" ensures. Fixes: 9aebfd4 ("Bluetooth: mediatek: add support for MediaTek MT7663S and MT7668S SDIO devices") Signed-off-by: Chris Lu <chris.lu@mediatek.com> Assisted-by: Claude:claude-opus-5 Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
…pwrseq The current code uses of_graph_is_present() to decide whether to enter the pwrseq path. However, of_graph_is_present() only checks for the structural presence of a port/ports sub-node and does not check the status property. This causes problems when a DT overlay disables the remote M.2 connector node (e.g., switching from PCIe WiFi to SDIO WiFi): the port node still exists, so of_graph_is_present() returns true, but the pwrseq provider never registers because the connector is disabled, leading to an infinite -EPROBE_DEFER loop. Replace of_graph_is_present() with a new helper that traverses the OF graph to the remote port parent (the M.2 connector node) and checks of_device_is_available(). When the remote connector is disabled, the pwrseq path is skipped, allowing the BT driver to fall through to the direct bluetooth child node path. Fixes: e48e332 ("Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq") Signed-off-by: Sherry Sun <sherry.sun@nxp.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Add lockdep check for RCU || hdev->lock in hci_conn_hash lookups that return hci_conn pointer, as dereferencing that without locks can be TOCTOU issue. It used to be several callsites did not hold appropriate locks. The check is equivalent to removing rcu_read_lock() and doing instead list_for_each_entry_rcu(c, &h->list, list, lockdep_is_held(&hdev->lock)) Although there should not be any remaining callsites without locks, don't remove the rcu_read_lock() for now, and just add the warning here. Signed-off-by: Pauli Virtanen <pav@iki.fi> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Add context analysis annotations to functions doing conditional locking, to suppress analysis warnings. Fixes: cdc36db ("Bluetooth: hci_sync: Fix advertising data UAFs") Tested-by: Nathan Chancellor <nathan@kernel.org> # build Signed-off-by: Pauli Virtanen <pav@iki.fi> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
…/dcid Replace the maybe-return-locked pattern in l2cap_get_chan_by_scid/dcid() by doing locking in the caller after NULL check. This allows adding context analysis annotations for the locking. Reviewed-by: Bart Van Assche <bvanassche@acm.org> Signed-off-by: Pauli Virtanen <pav@iki.fi> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Add minimal context analysis annotations to l2cap_chan_lock/unlock() and callers required for no warnings. Reviewed-by: Bart Van Assche <bvanassche@acm.org> Signed-off-by: Pauli Virtanen <pav@iki.fi> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Remove context analysis suppression for include/net/bluetooth/*, now that previous commits have resolved the warnings. Reviewed-by: Bart Van Assche <bvanassche@acm.org> Signed-off-by: Pauli Virtanen <pav@iki.fi> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
nxp_process_fw_dump() pulls the ACL header off the frame and then reads seq_num and buf_len from a struct nxp_fw_dump_hdr placed at skb->data, without checking that the ACL payload is long enough to contain it. h4_recv_buf() collects HCI_ACL_HDR_SIZE bytes of header followed by the number of payload bytes named in that header, so skb->len is 4 + dlen with dlen supplied by the controller and possibly smaller than the 8 byte dump header, or zero. A short frame with connection handle 0xfff therefore reads both fields from beyond the received data. Beyond the read itself, buf_len is what terminates a dump: a value of zero makes the driver call hci_devcd_complete() and reset the controller, so a truncated frame can end a dump early. Use skb_pull_data() to validate and pull the FW dump header before accessing its fields. Warn and reject the chunk if the header is truncated. Fixes: 998e447 ("Bluetooth: btnxpuart: Add support for HCI coredump feature") Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
eir_get_service_data() walks the advertising data for a Service Data
field with a matching UUID. On a mismatch it advances:
eir += dlen;
eir_len -= dlen;
eir_get_data() reports dlen as the field's data length, but the field
spans dlen + 2 bytes once its length and type bytes count, and more
when non-Service-Data fields were skipped to reach it. The pointer
lands correctly on the next field. eir_len does not, and the shortfall
compounds across fields until eir_get_data() reads the length and type
bytes of a "field" past the end of the buffer.
For an ISO broadcast sink that buffer is hcon->le_per_adv_data[], filled
from the periodic advertising reports of a remote broadcaster. A PA
payload packed with mismatching Service Data fields walks off the array
into the rest of struct hci_conn. A drifted field that matches the BAA
UUID puts those bytes in iso_pi(sk)->base, where user space reads them
back with getsockopt(BT_ISO_BASE).
Recompute eir_len from the end of the buffer each iteration.
Fixes: 8f9ae5b ("Bluetooth: eir: Add helpers for managing service data")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
hci_send_cmd_sync() returns -EINVAL when skb_clone() fails for sent_cmd, which describes an invalid argument rather than an allocation failure. Return -ENOMEM instead. The only caller, hci_cmd_work(), tests the result for zero, so there is no functional change. Signed-off-by: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
…4378 Commit ed2a2ef ("Bluetooth: Add quirk to ignore reserved PHY bits in LE Extended Adv Report") added a quirk to handle creative use of the reserved bits in the PHY fields for 4388 controllers in Apple silicon. I observed the same issue with the BCM4378 Bluetooth controller (14e4:5f69, rev 05) on an Apple MacBook Pro (13-inch, M2, 2022): > HCI Event: LE Meta Event (0x3e) plen 51 LE Extended Advertising Report (0x0d) Num reports: 1 Entry 0 Event type: 0x2513 Props: 0x0013 Connectable Scannable Use legacy advertising PDUs Data status: Complete Reserved (0x2500) Legacy PDU Type: Reserved (0x2513) Address type: Random (0x01) Address: EA:C1:82:F0:24:C6 (Static) Primary PHY: Reserved Secondary PHY: No packets SID: no ADI field (0xff) TX power: 127 dBm RSSI: -57 dBm (0xc7) Periodic advertising interval: 0.00 msec (0x0000) Direct address type: Public (0x00) Direct address: 00:00:00:00:00:00 (OUI 00-00-00) Data length: 25 This results in the firmware rejecting connection attempts with "Unsupported Feature or Parameter Value" (0x11). Fix the issue by using the same quirk for BCM4378 devices too. I tested this locally and confirmed that the issue is resolved. This was observed when attempting to connect a Kinesis Advantage 360 keyboard to the MacBook. Assisted-by: Claude:claude-fable-5 Fixes: 2e7ed5f ("Bluetooth: hci_sync: Use advertised PHYs on hci_le_ext_create_conn_sync") Cc: stable@vger.kernel.org Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org> Reviewed-by: Sven Peter <sven@kernel.org> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
… dropping The kernel sets HCI_AUTO_OFF when a controller is first registered and starts a 2-second timer. On slower boots bluetoothd and the HCI_AUTO_OFF timer can race: hci_power_off() is already queued while bluetoothd is still in the middle of its adapter setup sequence. hci_cmd_sync_clear() then cancels any pending mgmt commands with -ECANCELED, including the MGMT_OP_REMOVE_ADV_MONITOR sent by reset_adv_monitors() early in the setup sequence. When auto_off=1, hci_dev_close_sync() skips __mgmt_power_off() entirely, so there is no fallback path to reply to the cancelled commands. mgmt_remove_adv_monitor_complete() silently returns on -ECANCELED, leaving the command with no reply. Since bluez's mgmt queue is strictly serialised, this stalls all subsequent commands indefinitely, leaving bluetoothd unable to register the adapter. Fix by mapping -ECANCELED to MGMT_STATUS_CANCELLED in mgmt_errno_status() and replying to the cancelled command in mgmt_remove_adv_monitor_complete() instead of returning early. Signed-off-by: Shuai Zhang <shuai.zhang@oss.qualcomm.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
The firmware log line in btmtk_setup_firmware_79xx() never says which file was requested, which matters because btmtk_fw_get_filename() derives the name from the device id and firmware version at runtime. It reports the firmware's own hwver field as the HW version, when the device id the driver read from the chip is the more useful value and is already available as dev_id. And it prints datetime, a u8[16] with no guaranteed NUL terminator, with %s. Log the filename before the image is parsed, so a file that fails validation still says which file it was, report dev_id as the HW version, and bound datetime with %.16s. The datetime change is hardening rather than a fix, hence no Fixes: tag: every file in linux-firmware terminates the field, and provoking the over-read needs a malformed file in /lib/firmware. Log output on MT7922 after the change: [ 365.245951] Bluetooth: hci0: Loading BT firmware: mediatek/BT_RAM_CODE_MT7922_1_1_hdr.bin [ 365.245956] Bluetooth: hci0: BT HW ver: 0x7922, SW ver: 0x008a, Build Time: 20260605203811 Signed-off-by: Chris Lu <chris.lu@mediatek.com> Assisted-by: Claude:claude-opus-5 Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
The flag field of a BTMTK_WMT_PATCH_DWNLD packet tells the device where the packet sits in the download sequence, but both download loops write the bare values 1, 2 and 3, so the reader has to infer the meaning from the surrounding conditionals. Add enum btmtk_wmt_pkt_flag and use it in btmtk_setup_firmware_79xx() and btmtk_setup_firmware(). No functional change. The other bare flag values in this driver belong to different WMT opcodes (BTMTK_WMT_FUNC_CTRL, BTMTK_WMT_RST, BTMTK_WMT_SEMAPHORE and so on), where the field means something else entirely, so they are left alone. Signed-off-by: Chris Lu <chris.lu@mediatek.com> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de> Assisted-by: Claude:claude-opus-5 Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
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: 8961987 ("Bluetooth: Enumerate local supported codec and cache details") Fixes: 9ae6640 ("Bluetooth: Add support for Read Local Supported Codecs V2") Cc: stable@vger.kernel.org Suggested-by: Luiz Augusto von Dentz <luiz.dentz@gmail.com> Signed-off-by: Laxman Acharya Padhya <acharyalaxman8848@gmail.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
checkpatch.pl flags the zero-length array members in struct
qca_dump_hdr:
ERROR: Use C99 flexible arrays - see
https://docs.kernel.org/process/deprecated.html#zero-length-and-one-element-arrays
#3110: FILE: drivers/bluetooth/btusb.c:3110:
+ u8 data0[0];
Replace them with DECLARE_FLEX_ARRAY(), since C99 flexible array
members are not permitted inside unions or as the sole member of
a struct.
The struct layout is unchanged, and everything compiles with the
change.
Link: https://docs.kernel.org/process/deprecated.html#zero-length-and-one-element-arrays
Signed-off-by: Jeremy Dean <deaner92@yahoo.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
hci_uart_close() closes the serdev port if HCI_QUIRK_NON_PERSISTENT_SETUP is set (for example, for the WCN399x family). A failed hci_dev_open_sync() following a successful qca_setup() calls hdev->close() but not hdev->shutdown(), so the port is closed while power->vregs_on is left true. qca_serdev_remove() then passes its power->vregs_on test and calls qca_power_off(), which writes to the closed port unconditionally. Seen on a WCN3988 by unbinding the driver after a controller failure. The trace below is from a 7.0.0 based kernel, where qca_power_off() was still named qca_power_shutdown(): Unable to handle kernel NULL pointer dereference at virtual address 0000000000000038 Call trace: tty_set_termios+0x50/0x238 (P) ttyport_set_baudrate+0x84/0xc0 serdev_device_set_baudrate+0x24/0x40 qca_power_shutdown+0x158/0x1fc [hci_uart] qca_serdev_remove+0x54/0x68 [hci_uart] serdev_drv_remove+0x1c/0x2c device_remove+0x4c/0x80 device_release_driver_internal+0x1cc/0x224 device_driver_detach+0x18/0x24 unbind_store+0xb4/0xc0 Check HCI_UART_PROTO_READY, which hci_uart_close() clears in the same place it closes the port, before writing to it. The regulator disable is left unconditional so the controller is still powered down. The dangling serport->tty that turns this into a use-after-free is addressed in a separate patch. Fixes: fa9ad87 ("Bluetooth: hci_qca: Add support for Qualcomm Bluetooth chip wcn3990") Signed-off-by: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
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: b938790 ("Bluetooth: hci_codec: Fix leaking content of local_codecs")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
smp_e() already clears the AES key in its "struct aes_enckey aes" on the stack before leaving the function - but the very same information is also available as raw key data in the tmp[] array, so this should get cleared, too. Signed-off-by: Thomas Huth <thuth@redhat.com>
This patch adds workflow files for ci: [sync.yml] - The workflow file for scheduled work - Sync the repo with upstream repo and rebase the workflow branch - Review the patches in the patchwork and creates the PR if needed [ci.yml] - The workflow file for CI tasks - Run CI tests when PR is created Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
This replaces the bzcafe action with bluez/action-ci so we can maintain everything in the github bluez organization Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This attempts to sync every 5 minutes instead of 30. Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
bluez/action-ci uses master as default branch for workflow which is incorrect for kernel Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
The CI action now creates individual GitHub Check Runs per test, which requires 'checks: write' permission on the GITHUB_TOKEN. Also make the pull_request trigger types explicit to include 'reopened', allowing CI to be retriggered by closing and reopening a PR.
Teach btusb to recognize another IMC Networks / Realtek RTL8851BE
Bluetooth radio variant.
Without BTUSB_REALTEK, the device enumerates and HCI comes up, but
btrtl never loads rtl8851bu_fw.bin, so discovery finds no devices.
/sys/kernel/debug/usb/devices reports for this device:
T: Bus=01 Lev=01 Prnt=01 Port=10 Cnt=02 Dev#= 3 Spd=12 MxCh= 0
D: Ver= 1.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs= 1
P: Vendor=13d3 ProdID=3626 Rev= 0.00
S: Manufacturer=Realtek
S: Product=Bluetooth Radio
S: SerialNumber=00e04c000001
C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=500mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E: Ad=81(I) Atr=03(Int.) MxPS= 16 Ivl=1ms
E: Ad=02(O) Atr=02(Bulk) MxPS= 64 Ivl=0ms
E: Ad=82(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 0 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 0 Ivl=1ms
Tested on Dell Precision 7820 (Fedora 44, kernel 7.2.4) with the
combo RTL8851BE Wi-Fi/BT card. After the quirk, dmesg shows:
Bluetooth: hci0: RTL: examining hci_ver=0c hci_rev=000b
lmp_ver=0c lmp_subver=8851
Bluetooth: hci0: RTL: loading rtl_bt/rtl8851bu_fw.bin
Bluetooth: hci0: RTL: fw version 0x048ad230
and LE discovery/pairing works (Logitech M720 Triathlon).
Signed-off-by: Martin Luker-Brown <mlukerbrown@gmail.com>
v2: Wrap long dmesg line in the commit message so checkpatch/gitlint
stay within the 75/80 character limits.
|
CheckPatch |
|
VerifyFixes |
|
VerifySignedoff |
|
GitLint |
|
SubjectPrefix |
|
BuildKernel |
|
CheckAllWarning |
|
CheckSparse |
|
BuildKernel32 |
|
CheckKernelLLVM |
|
TestRunnerSetup |
|
IncrementalBuild |
dd2b970 to
940d0d6
Compare
Teach btusb to recognize another IMC Networks / Realtek RTL8851BE
Bluetooth radio variant.
Without BTUSB_REALTEK, the device enumerates and HCI comes up, but
btrtl never loads rtl8851bu_fw.bin, so discovery finds no devices.
/sys/kernel/debug/usb/devices reports for this device:
T: Bus=01 Lev=01 Prnt=01 Port=10 Cnt=02 Dev#= 3 Spd=12 MxCh= 0
D: Ver= 1.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs= 1
P: Vendor=13d3 ProdID=3626 Rev= 0.00
S: Manufacturer=Realtek
S: Product=Bluetooth Radio
S: SerialNumber=00e04c000001
C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=500mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E: Ad=81(I) Atr=03(Int.) MxPS= 16 Ivl=1ms
E: Ad=02(O) Atr=02(Bulk) MxPS= 64 Ivl=0ms
E: Ad=82(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 0 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 0 Ivl=1ms
Tested on Dell Precision 7820 (Fedora 44, kernel 7.2.4) with the
combo RTL8851BE Wi-Fi/BT card. After the quirk, dmesg shows:
Bluetooth: hci0: RTL: examining hci_ver=0c hci_rev=000b
lmp_ver=0c lmp_subver=8851
Bluetooth: hci0: RTL: loading rtl_bt/rtl8851bu_fw.bin
Bluetooth: hci0: RTL: fw version 0x048ad230
and LE discovery/pairing works (Logitech M720 Triathlon).
Signed-off-by: Martin Luker-Brown mlukerbrown@gmail.com
v2: Wrap long dmesg line in the commit message so checkpatch/gitlint
stay within the 75/80 character limits.
drivers/bluetooth/btusb.c | 1 +
1 file changed, 1 insertion(+)