[PW_SID:1164165] Bluetooth: Fix 2 devcoredump bugs and improve event length checks - #762
Open
BluezTestBot wants to merge 122 commits into
Open
BluezTestBot wants to merge 122 commits into
BluezTestBot wants to merge 122 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>
added 2 commits
September 14, 2026 05:29
…in hci_le_ev_table[] hci_le_meta_evt() validates skb->len against subev->max_len from the entry in hci_le_ev_table[], By then, the HCI event header and LE Meta subevent header have already been stripped by skb_pull(), So the max LE Meta subevent payload is 254, but hci_le_ev_table[] still uses HCI_MAX_EVENT_SIZE (260) for it, which is imprecise. Fix by defining HCI_MAX_LE_SUBEVT_PLEN (254) and using it instead. Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
…c_table[] hci_cc_func() validates skb->len against cc->max_len from the entry in hci_cc_table[], But By then, the HCI event and CC headers have already been stripped by skb_pull(). So the max CC payload is 252, but hci_cc_table[] still uses HCI_MAX_EVENT_SIZE (260) for it, which is imprecise. Fix by defining HCI_MAX_CC_PLEN (252) and using it instead. Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
|
CheckPatch |
|
VerifyFixes |
|
VerifySignedoff |
|
GitLint |
|
SubjectPrefix |
|
BuildKernel |
|
CheckAllWarning |
|
CheckSparse |
|
BuildKernel32 |
|
CheckKernelLLVM |
|
TestRunnerSetup |
|
TestRunner_l2cap-tester |
|
TestRunner_iso-tester |
|
TestRunner_bnep-tester |
|
TestRunner_mgmt-tester |
|
TestRunner_rfcomm-tester |
|
TestRunner_sco-tester |
|
TestRunner_ioctl-tester |
|
TestRunner_mesh-tester |
|
TestRunner_smp-tester |
|
TestRunner_userchan-tester |
|
TestRunner_6lowpan-tester |
|
IncrementalBuild |
github-actions
Bot
force-pushed
the
workflow
branch
4 times, most recently
from
September 16, 2026 21:48
dd2b970 to
940d0d6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For hci_devcd_append(hdev, skb):
it consumes the skb when CONFIG_DEV_COREDUMP=y, but its stub does not free
the skb. this inconsistency causes skb leak when CONFIG_DEV_COREDUMP=n.
NXP btuart device driver encounters this leak:
btnxpuart.c:
nxp_process_fw_dump(hdev, skb)
{
...
// the skb_clone() is leaked.
err = hci_devcd_append(hdev, skb_clone(skb, GFP_ATOMIC));
if (err < 0)
goto free_skb;
...
free_skb:
kfree_skb(skb); /* frees the original, not the clone */
return 0;
}
Fix by freeing the skb in the stub to keep consistent behavior.
Fixes: 9695ef8 ("Bluetooth: Add support for hci devcoredump")
Signed-off-by: Zijun Hu zijun.hu@oss.qualcomm.com
include/net/bluetooth/coredump.h | 1 +
1 file changed, 1 insertion(+)