From 3c44a431d62bf4a20d7b901f98266ae3f4676d48 Mon Sep 17 00:00:00 2001 From: Zhengping Jiang Date: Tue, 21 Feb 2023 16:17:56 -0800 Subject: Bluetooth: hci_sync: Resume adv with no RPA when active scan The address resolution should be disabled during the active scan, so all the advertisements can reach the host. The advertising has to be paused before disabling the address resolution, because the advertising will prevent any changes to the resolving list and the address resolution status. Skipping this will cause the hci error and the discovery failure. According to the bluetooth specification: "7.8.44 LE Set Address Resolution Enable command This command shall not be used when: - Advertising (other than periodic advertising) is enabled, - Scanning is enabled, or - an HCI_LE_Create_Connection, HCI_LE_Extended_Create_Connection, or HCI_LE_Periodic_Advertising_Create_Sync command is outstanding." If the host is using RPA, the controller needs to generate RPA for the advertising, so the advertising must remain paused during the active scan. If the host is not using RPA, the advertising can be resumed after disabling the address resolution. Fixes: 9afc675edeeb ("Bluetooth: hci_sync: allow advertise when scan without RPA") Signed-off-by: Zhengping Jiang Signed-off-by: Luiz Augusto von Dentz --- net/bluetooth/hci_sync.c | 64 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 20 deletions(-) (limited to 'net/bluetooth/hci_sync.c') diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 117eedb6f709..7e152e912e8c 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -2367,6 +2367,45 @@ static int hci_resume_advertising_sync(struct hci_dev *hdev) return err; } +static int hci_pause_addr_resolution(struct hci_dev *hdev) +{ + int err; + + if (!use_ll_privacy(hdev)) + return 0; + + if (!hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION)) + return 0; + + /* Cannot disable addr resolution if scanning is enabled or + * when initiating an LE connection. + */ + if (hci_dev_test_flag(hdev, HCI_LE_SCAN) || + hci_lookup_le_connect(hdev)) { + bt_dev_err(hdev, "Command not allowed when scan/LE connect"); + return -EPERM; + } + + /* Cannot disable addr resolution if advertising is enabled. */ + err = hci_pause_advertising_sync(hdev); + if (err) { + bt_dev_err(hdev, "Pause advertising failed: %d", err); + return err; + } + + err = hci_le_set_addr_resolution_enable_sync(hdev, 0x00); + if (err) + bt_dev_err(hdev, "Unable to disable Address Resolution: %d", + err); + + /* Return if address resolution is disabled and RPA is not used. */ + if (!err && scan_use_rpa(hdev)) + return err; + + hci_resume_advertising_sync(hdev); + return err; +} + struct sk_buff *hci_read_local_oob_data_sync(struct hci_dev *hdev, bool extended, struct sock *sk) { @@ -2402,7 +2441,7 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev) u8 filter_policy; int err; - /* Pause advertising if resolving list can be used as controllers are + /* Pause advertising if resolving list can be used as controllers * cannot accept resolving list modifications while advertising. */ if (use_ll_privacy(hdev)) { @@ -5394,27 +5433,12 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval) cancel_interleave_scan(hdev); - /* Pause advertising since active scanning disables address resolution - * which advertising depend on in order to generate its RPAs. - */ - if (use_ll_privacy(hdev) && hci_dev_test_flag(hdev, HCI_PRIVACY)) { - err = hci_pause_advertising_sync(hdev); - if (err) { - bt_dev_err(hdev, "pause advertising failed: %d", err); - goto failed; - } - } - - /* Disable address resolution while doing active scanning since the - * accept list shall not be used and all reports shall reach the host - * anyway. + /* Pause address resolution for active scan and stop advertising if + * privacy is enabled. */ - err = hci_le_set_addr_resolution_enable_sync(hdev, 0x00); - if (err) { - bt_dev_err(hdev, "Unable to disable Address Resolution: %d", - err); + err = hci_pause_addr_resolution(hdev); + if (err) goto failed; - } /* All active scans will be done with either a resolvable private * address (when privacy feature has been enabled) or non-resolvable -- cgit v1.2.3 From 1c66bee492a5fe00ae3fe890bb693bfc99f994c6 Mon Sep 17 00:00:00 2001 From: Min Li Date: Sat, 4 Mar 2023 21:50:35 +0800 Subject: Bluetooth: Fix race condition in hci_cmd_sync_clear There is a potential race condition in hci_cmd_sync_work and hci_cmd_sync_clear, and could lead to use-after-free. For instance, hci_cmd_sync_work is added to the 'req_workqueue' after cancel_work_sync The entry of 'cmd_sync_work_list' may be freed in hci_cmd_sync_clear, and causing kernel panic when it is used in 'hci_cmd_sync_work'. Here's the call trace: dump_stack_lvl+0x49/0x63 print_report.cold+0x5e/0x5d3 ? hci_cmd_sync_work+0x282/0x320 kasan_report+0xaa/0x120 ? hci_cmd_sync_work+0x282/0x320 __asan_report_load8_noabort+0x14/0x20 hci_cmd_sync_work+0x282/0x320 process_one_work+0x77b/0x11c0 ? _raw_spin_lock_irq+0x8e/0xf0 worker_thread+0x544/0x1180 ? poll_idle+0x1e0/0x1e0 kthread+0x285/0x320 ? process_one_work+0x11c0/0x11c0 ? kthread_complete_and_exit+0x30/0x30 ret_from_fork+0x22/0x30 Allocated by task 266: kasan_save_stack+0x26/0x50 __kasan_kmalloc+0xae/0xe0 kmem_cache_alloc_trace+0x191/0x350 hci_cmd_sync_queue+0x97/0x2b0 hci_update_passive_scan+0x176/0x1d0 le_conn_complete_evt+0x1b5/0x1a00 hci_le_conn_complete_evt+0x234/0x340 hci_le_meta_evt+0x231/0x4e0 hci_event_packet+0x4c5/0xf00 hci_rx_work+0x37d/0x880 process_one_work+0x77b/0x11c0 worker_thread+0x544/0x1180 kthread+0x285/0x320 ret_from_fork+0x22/0x30 Freed by task 269: kasan_save_stack+0x26/0x50 kasan_set_track+0x25/0x40 kasan_set_free_info+0x24/0x40 ____kasan_slab_free+0x176/0x1c0 __kasan_slab_free+0x12/0x20 slab_free_freelist_hook+0x95/0x1a0 kfree+0xba/0x2f0 hci_cmd_sync_clear+0x14c/0x210 hci_unregister_dev+0xff/0x440 vhci_release+0x7b/0xf0 __fput+0x1f3/0x970 ____fput+0xe/0x20 task_work_run+0xd4/0x160 do_exit+0x8b0/0x22a0 do_group_exit+0xba/0x2a0 get_signal+0x1e4a/0x25b0 arch_do_signal_or_restart+0x93/0x1f80 exit_to_user_mode_prepare+0xf5/0x1a0 syscall_exit_to_user_mode+0x26/0x50 ret_from_fork+0x15/0x30 Fixes: 6a98e3836fa2 ("Bluetooth: Add helper for serialized HCI command execution") Cc: stable@vger.kernel.org Signed-off-by: Min Li Signed-off-by: Luiz Augusto von Dentz --- net/bluetooth/hci_sync.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'net/bluetooth/hci_sync.c') diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 7e152e912e8c..5b8dc8fb2e27 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -643,6 +643,7 @@ void hci_cmd_sync_clear(struct hci_dev *hdev) cancel_work_sync(&hdev->cmd_sync_work); cancel_work_sync(&hdev->reenable_adv_work); + mutex_lock(&hdev->cmd_sync_work_lock); list_for_each_entry_safe(entry, tmp, &hdev->cmd_sync_work_list, list) { if (entry->destroy) entry->destroy(hdev, entry->data, -ECANCELED); @@ -650,6 +651,7 @@ void hci_cmd_sync_clear(struct hci_dev *hdev) list_del(&entry->list); kfree(entry); } + mutex_unlock(&hdev->cmd_sync_work_lock); } void __hci_cmd_sync_cancel(struct hci_dev *hdev, int err) -- cgit v1.2.3 From bce56405201111807cc8e4f47c6de3e10b17c1ac Mon Sep 17 00:00:00 2001 From: Sungwoo Kim Date: Mon, 20 Mar 2023 21:50:18 -0400 Subject: Bluetooth: HCI: Fix global-out-of-bounds To loop a variable-length array, hci_init_stage_sync(stage) considers that stage[i] is valid as long as stage[i-1].func is valid. Thus, the last element of stage[].func should be intentionally invalid as hci_init0[], le_init2[], and others did. However, amp_init1[] and amp_init2[] have no invalid element, letting hci_init_stage_sync() keep accessing amp_init1[] over its valid range. This patch fixes this by adding {} in the last of amp_init1[] and amp_init2[]. ================================================================== BUG: KASAN: global-out-of-bounds in hci_dev_open_sync ( /v6.2-bzimage/net/bluetooth/hci_sync.c:3154 /v6.2-bzimage/net/bluetooth/hci_sync.c:3343 /v6.2-bzimage/net/bluetooth/hci_sync.c:4418 /v6.2-bzimage/net/bluetooth/hci_sync.c:4609 /v6.2-bzimage/net/bluetooth/hci_sync.c:4689) Read of size 8 at addr ffffffffaed1ab70 by task kworker/u5:0/1032 CPU: 0 PID: 1032 Comm: kworker/u5:0 Not tainted 6.2.0 #3 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04 Workqueue: hci1 hci_power_on Call Trace: dump_stack_lvl (/v6.2-bzimage/lib/dump_stack.c:107 (discriminator 1)) print_report (/v6.2-bzimage/mm/kasan/report.c:307 /v6.2-bzimage/mm/kasan/report.c:417) ? hci_dev_open_sync (/v6.2-bzimage/net/bluetooth/hci_sync.c:3154 /v6.2-bzimage/net/bluetooth/hci_sync.c:3343 /v6.2-bzimage/net/bluetooth/hci_sync.c:4418 /v6.2-bzimage/net/bluetooth/hci_sync.c:4609 /v6.2-bzimage/net/bluetooth/hci_sync.c:4689) kasan_report (/v6.2-bzimage/mm/kasan/report.c:184 /v6.2-bzimage/mm/kasan/report.c:519) ? hci_dev_open_sync (/v6.2-bzimage/net/bluetooth/hci_sync.c:3154 /v6.2-bzimage/net/bluetooth/hci_sync.c:3343 /v6.2-bzimage/net/bluetooth/hci_sync.c:4418 /v6.2-bzimage/net/bluetooth/hci_sync.c:4609 /v6.2-bzimage/net/bluetooth/hci_sync.c:4689) hci_dev_open_sync (/v6.2-bzimage/net/bluetooth/hci_sync.c:3154 /v6.2-bzimage/net/bluetooth/hci_sync.c:3343 /v6.2-bzimage/net/bluetooth/hci_sync.c:4418 /v6.2-bzimage/net/bluetooth/hci_sync.c:4609 /v6.2-bzimage/net/bluetooth/hci_sync.c:4689) ? __pfx_hci_dev_open_sync (/v6.2-bzimage/net/bluetooth/hci_sync.c:4635) ? mutex_lock (/v6.2-bzimage/./arch/x86/include/asm/atomic64_64.h:190 /v6.2-bzimage/./include/linux/atomic/atomic-long.h:443 /v6.2-bzimage/./include/linux/atomic/atomic-instrumented.h:1781 /v6.2-bzimage/kernel/locking/mutex.c:171 /v6.2-bzimage/kernel/locking/mutex.c:285) ? __pfx_mutex_lock (/v6.2-bzimage/kernel/locking/mutex.c:282) hci_power_on (/v6.2-bzimage/net/bluetooth/hci_core.c:485 /v6.2-bzimage/net/bluetooth/hci_core.c:984) ? __pfx_hci_power_on (/v6.2-bzimage/net/bluetooth/hci_core.c:969) ? read_word_at_a_time (/v6.2-bzimage/./include/asm-generic/rwonce.h:85) ? strscpy (/v6.2-bzimage/./arch/x86/include/asm/word-at-a-time.h:62 /v6.2-bzimage/lib/string.c:161) process_one_work (/v6.2-bzimage/kernel/workqueue.c:2294) worker_thread (/v6.2-bzimage/./include/linux/list.h:292 /v6.2-bzimage/kernel/workqueue.c:2437) ? __pfx_worker_thread (/v6.2-bzimage/kernel/workqueue.c:2379) kthread (/v6.2-bzimage/kernel/kthread.c:376) ? __pfx_kthread (/v6.2-bzimage/kernel/kthread.c:331) ret_from_fork (/v6.2-bzimage/arch/x86/entry/entry_64.S:314) The buggy address belongs to the variable: amp_init1+0x30/0x60 The buggy address belongs to the physical page: page:000000003a157ec6 refcount:1 mapcount:0 mapping:0000000000000000 ia flags: 0x200000000001000(reserved|node=0|zone=2) raw: 0200000000001000 ffffea0005054688 ffffea0005054688 000000000000000 raw: 0000000000000000 0000000000000000 00000001ffffffff 000000000000000 page dumped because: kasan: bad access detected Memory state around the buggy address: ffffffffaed1aa00: f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9 00 00 00 00 ffffffffaed1aa80: 00 00 00 00 f9 f9 f9 f9 00 00 00 00 00 00 00 00 >ffffffffaed1ab00: 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 f9 f9 ^ ffffffffaed1ab80: f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9 00 00 00 f9 ffffffffaed1ac00: f9 f9 f9 f9 00 06 f9 f9 f9 f9 f9 f9 00 00 02 f9 This bug is found by FuzzBT, a modified version of Syzkaller. Other contributors for this bug are Ruoyu Wu and Peng Hui. Fixes: d0b137062b2d ("Bluetooth: hci_sync: Rework init stages") Signed-off-by: Sungwoo Kim Reviewed-by: Simon Horman Signed-off-by: Luiz Augusto von Dentz --- net/bluetooth/hci_sync.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'net/bluetooth/hci_sync.c') diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 5b8dc8fb2e27..5a6aa1627791 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -3360,6 +3360,7 @@ static const struct hci_init_stage amp_init1[] = { HCI_INIT(hci_read_flow_control_mode_sync), /* HCI_OP_READ_LOCATION_DATA */ HCI_INIT(hci_read_location_data_sync), + {} }; static int hci_init1_sync(struct hci_dev *hdev) @@ -3394,6 +3395,7 @@ static int hci_init1_sync(struct hci_dev *hdev) static const struct hci_init_stage amp_init2[] = { /* HCI_OP_READ_LOCAL_FEATURES */ HCI_INIT(hci_read_local_features_sync), + {} }; /* Read Buffer Size (ACL mtu, max pkt, etc.) */ -- cgit v1.2.3 From b62e72200eaad523f08d8319bba50fc652e032a8 Mon Sep 17 00:00:00 2001 From: Luiz Augusto von Dentz Date: Fri, 24 Mar 2023 13:18:20 -0700 Subject: Bluetooth: Fix printing errors if LE Connection times out This fixes errors like bellow when LE Connection times out since that is actually not a controller error: Bluetooth: hci0: Opcode 0x200d failed: -110 Bluetooth: hci0: request failed to create LE connection: err -110 Instead the code shall properly detect if -ETIMEDOUT is returned and send HCI_OP_LE_CREATE_CONN_CANCEL to give up on the connection. Link: https://github.com/bluez/bluez/issues/340 Fixes: 8e8b92ee60de ("Bluetooth: hci_sync: Add hci_le_create_conn_sync") Signed-off-by: Luiz Augusto von Dentz --- include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_conn.c | 7 +++++-- net/bluetooth/hci_event.c | 16 ++++++---------- net/bluetooth/hci_sync.c | 13 ++++++++++--- 4 files changed, 22 insertions(+), 15 deletions(-) (limited to 'net/bluetooth/hci_sync.c') diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 6ed9b4d546a7..d5311ceb21c6 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -954,6 +954,7 @@ enum { HCI_CONN_STK_ENCRYPT, HCI_CONN_AUTH_INITIATOR, HCI_CONN_DROP, + HCI_CONN_CANCEL, HCI_CONN_PARAM_REMOVAL_PEND, HCI_CONN_NEW_LINK_KEY, HCI_CONN_SCANNING, diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 5af3f6b011c9..e4aee5950c36 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1233,6 +1233,8 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err) { struct hci_conn *conn = data; + bt_dev_dbg(hdev, "err %d", err); + hci_dev_lock(hdev); if (!err) { @@ -1240,8 +1242,6 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err) goto done; } - bt_dev_err(hdev, "request failed to create LE connection: err %d", err); - /* Check if connection is still pending */ if (conn != hci_lookup_le_connect(hdev)) goto done; @@ -2771,6 +2771,9 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason) { int r = 0; + if (test_and_set_bit(HCI_CONN_CANCEL, &conn->flags)) + return 0; + switch (conn->state) { case BT_CONNECTED: case BT_CONFIG: diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index ad92a4be5851..e68f2a7d863a 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2881,16 +2881,6 @@ static void cs_le_create_conn(struct hci_dev *hdev, bdaddr_t *peer_addr, conn->resp_addr_type = peer_addr_type; bacpy(&conn->resp_addr, peer_addr); - - /* We don't want the connection attempt to stick around - * indefinitely since LE doesn't have a page timeout concept - * like BR/EDR. Set a timer for any connection that doesn't use - * the accept list for connecting. - */ - if (filter_policy == HCI_LE_USE_PEER_ADDR) - queue_delayed_work(conn->hdev->workqueue, - &conn->le_conn_timeout, - conn->conn_timeout); } static void hci_cs_le_create_conn(struct hci_dev *hdev, u8 status) @@ -5902,6 +5892,12 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status, if (status) goto unlock; + /* Drop the connection if it has been aborted */ + if (test_bit(HCI_CONN_CANCEL, &conn->flags)) { + hci_conn_drop(conn); + goto unlock; + } + if (conn->dst_type == ADDR_LE_DEV_PUBLIC) addr_type = BDADDR_LE_PUBLIC; else diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 5a6aa1627791..632be1267288 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -246,8 +246,9 @@ int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen, skb = __hci_cmd_sync_sk(hdev, opcode, plen, param, event, timeout, sk); if (IS_ERR(skb)) { - bt_dev_err(hdev, "Opcode 0x%4x failed: %ld", opcode, - PTR_ERR(skb)); + if (!event) + bt_dev_err(hdev, "Opcode 0x%4x failed: %ld", opcode, + PTR_ERR(skb)); return PTR_ERR(skb); } @@ -5126,8 +5127,11 @@ static int hci_le_connect_cancel_sync(struct hci_dev *hdev, if (test_bit(HCI_CONN_SCANNING, &conn->flags)) return 0; + if (test_and_set_bit(HCI_CONN_CANCEL, &conn->flags)) + return 0; + return __hci_cmd_sync_status(hdev, HCI_OP_LE_CREATE_CONN_CANCEL, - 6, &conn->dst, HCI_CMD_TIMEOUT); + 0, NULL, HCI_CMD_TIMEOUT); } static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn) @@ -6102,6 +6106,9 @@ int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn) conn->conn_timeout, NULL); done: + if (err == -ETIMEDOUT) + hci_le_connect_cancel_sync(hdev, conn); + /* Re-enable advertising after the connection attempt is finished. */ hci_resume_advertising_sync(hdev); return err; -- cgit v1.2.3 From ae051b044ebf5938940741c25a8989e07a5d7e88 Mon Sep 17 00:00:00 2001 From: Luiz Augusto von Dentz Date: Mon, 27 Feb 2023 15:50:58 -0800 Subject: Bluetooth: hci_sync: Fix smatch warning This fixes the following new warning: net/bluetooth/hci_sync.c:2403 hci_pause_addr_resolution() warn: missing error code? 'err' Reported-by: kernel test robot Reported-by: Dan Carpenter Link: https://lore.kernel.org/r/202302251952.xryXOegd-lkp@intel.com/ Signed-off-by: Luiz Augusto von Dentz --- net/bluetooth/hci_sync.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/bluetooth/hci_sync.c') diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 632be1267288..b64af017ab99 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -2403,7 +2403,7 @@ static int hci_pause_addr_resolution(struct hci_dev *hdev) /* Return if address resolution is disabled and RPA is not used. */ if (!err && scan_use_rpa(hdev)) - return err; + return 0; hci_resume_advertising_sync(hdev); return err; -- cgit v1.2.3 From f5d13029a41d6504831ab99da0a9a0332fa7ea19 Mon Sep 17 00:00:00 2001 From: Archie Pusaka Date: Mon, 6 Mar 2023 17:07:07 +0800 Subject: Bluetooth: hci_sync: Don't wait peer's reply when powering off Currently, when we initiate disconnection, we will wait for the peer's reply unless when we are suspending, where we fire and forget the disconnect request. A similar case is when adapter is powering off. However, we still wait for the peer's reply in this case. Therefore, if the peer is unresponsive, the command will time out and the power off sequence will fail, causing "bluetooth powered on by itself" to users. This patch makes the host doesn't wait for the peer's reply when the disconnection reason is powering off. Signed-off-by: Archie Pusaka Reviewed-by: Abhishek Pandit-Subedi Reviewed-by: Simon Horman Signed-off-by: Luiz Augusto von Dentz --- net/bluetooth/hci_sync.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'net/bluetooth/hci_sync.c') diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index b64af017ab99..eb5fd2fe1dc2 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -5108,10 +5108,12 @@ static int hci_disconnect_sync(struct hci_dev *hdev, struct hci_conn *conn, cp.handle = cpu_to_le16(conn->handle); cp.reason = reason; - /* Wait for HCI_EV_DISCONN_COMPLETE not HCI_EV_CMD_STATUS when not - * suspending. + /* Wait for HCI_EV_DISCONN_COMPLETE, not HCI_EV_CMD_STATUS, when the + * reason is anything but HCI_ERROR_REMOTE_POWER_OFF. This reason is + * used when suspending or powering off, where we don't want to wait + * for the peer's response. */ - if (!hdev->suspended) + if (reason != HCI_ERROR_REMOTE_POWER_OFF) return __hci_cmd_sync_status_sk(hdev, HCI_OP_DISCONNECT, sizeof(cp), &cp, HCI_EV_DISCONN_COMPLETE, -- cgit v1.2.3 From 9f5ae8fcae1227e9bae3d973fcbef8319a1b25c7 Mon Sep 17 00:00:00 2001 From: Inga Stotland Date: Wed, 8 Mar 2023 10:32:35 -0800 Subject: Bluetooth: hci_sync: Remove duplicate statement This removes the following duplicate statement in hci_le_ext_directed_advertising_sync(): cp.own_addr_type = own_addr_type; Signed-off-by: Inga Stotland Signed-off-by: Luiz Augusto von Dentz --- net/bluetooth/hci_sync.c | 1 - 1 file changed, 1 deletion(-) (limited to 'net/bluetooth/hci_sync.c') diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index eb5fd2fe1dc2..31231f0e4a28 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -5855,7 +5855,6 @@ static int hci_le_ext_directed_advertising_sync(struct hci_dev *hdev, memset(&cp, 0, sizeof(cp)); cp.evt_properties = cpu_to_le16(LE_LEGACY_ADV_DIRECT_IND); - cp.own_addr_type = own_addr_type; cp.channel_map = hdev->le_adv_channel_map; cp.tx_power = HCI_TX_POWER_INVALID; cp.primary_phy = HCI_ADV_PHY_1M; -- cgit v1.2.3 From 9695ef876fd122cb7bbc04a4a93b8727d2e36bda Mon Sep 17 00:00:00 2001 From: Abhishek Pandit-Subedi Date: Thu, 30 Mar 2023 09:58:23 -0700 Subject: Bluetooth: Add support for hci devcoredump Add devcoredump APIs to hci core so that drivers only have to provide the dump skbs instead of managing the synchronization and timeouts. The devcoredump APIs should be used in the following manner: - hci_devcoredump_init is called to allocate the dump. - hci_devcoredump_append is called to append any skbs with dump data OR hci_devcoredump_append_pattern is called to insert a pattern. - hci_devcoredump_complete is called when all dump packets have been sent OR hci_devcoredump_abort is called to indicate an error and cancel an ongoing dump collection. The high level APIs just prepare some skbs with the appropriate data and queue it for the dump to process. Packets part of the crashdump can be intercepted in the driver in interrupt context and forwarded directly to the devcoredump APIs. Internally, there are 5 states for the dump: idle, active, complete, abort and timeout. A devcoredump will only be in active state after it has been initialized. Once active, it accepts data to be appended, patterns to be inserted (i.e. memset) and a completion event or an abort event to generate a devcoredump. The timeout is initialized at the same time the dump is initialized (defaulting to 10s) and will be cleared either when the timeout occurs or the dump is complete or aborted. Signed-off-by: Abhishek Pandit-Subedi Signed-off-by: Manish Mandlik Reviewed-by: Abhishek Pandit-Subedi Signed-off-by: Luiz Augusto von Dentz --- include/net/bluetooth/coredump.h | 116 +++++++++ include/net/bluetooth/hci_core.h | 14 + net/bluetooth/Makefile | 2 + net/bluetooth/coredump.c | 535 +++++++++++++++++++++++++++++++++++++++ net/bluetooth/hci_core.c | 1 + net/bluetooth/hci_sync.c | 2 + 6 files changed, 670 insertions(+) create mode 100644 include/net/bluetooth/coredump.h create mode 100644 net/bluetooth/coredump.c (limited to 'net/bluetooth/hci_sync.c') diff --git a/include/net/bluetooth/coredump.h b/include/net/bluetooth/coredump.h new file mode 100644 index 000000000000..72f51b587a04 --- /dev/null +++ b/include/net/bluetooth/coredump.h @@ -0,0 +1,116 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (C) 2022 Google Corporation + */ + +#ifndef __COREDUMP_H +#define __COREDUMP_H + +#define DEVCOREDUMP_TIMEOUT msecs_to_jiffies(10000) /* 10 sec */ + +typedef void (*coredump_t)(struct hci_dev *hdev); +typedef void (*dmp_hdr_t)(struct hci_dev *hdev, struct sk_buff *skb); +typedef void (*notify_change_t)(struct hci_dev *hdev, int state); + +/* struct hci_devcoredump - Devcoredump state + * + * @supported: Indicates if FW dump collection is supported by driver + * @state: Current state of dump collection + * @timeout: Indicates a timeout for collecting the devcoredump + * + * @alloc_size: Total size of the dump + * @head: Start of the dump + * @tail: Pointer to current end of dump + * @end: head + alloc_size for easy comparisons + * + * @dump_q: Dump queue for state machine to process + * @dump_rx: Devcoredump state machine work + * @dump_timeout: Devcoredump timeout work + * + * @coredump: Called from the driver's .coredump() function. + * @dmp_hdr: Create a dump header to identify controller/fw/driver info + * @notify_change: Notify driver when devcoredump state has changed + */ +struct hci_devcoredump { + bool supported; + + enum devcoredump_state { + HCI_DEVCOREDUMP_IDLE, + HCI_DEVCOREDUMP_ACTIVE, + HCI_DEVCOREDUMP_DONE, + HCI_DEVCOREDUMP_ABORT, + HCI_DEVCOREDUMP_TIMEOUT, + } state; + + unsigned long timeout; + + size_t alloc_size; + char *head; + char *tail; + char *end; + + struct sk_buff_head dump_q; + struct work_struct dump_rx; + struct delayed_work dump_timeout; + + coredump_t coredump; + dmp_hdr_t dmp_hdr; + notify_change_t notify_change; +}; + +#ifdef CONFIG_DEV_COREDUMP + +void hci_devcd_reset(struct hci_dev *hdev); +void hci_devcd_rx(struct work_struct *work); +void hci_devcd_timeout(struct work_struct *work); + +int hci_devcd_register(struct hci_dev *hdev, coredump_t coredump, + dmp_hdr_t dmp_hdr, notify_change_t notify_change); +int hci_devcd_init(struct hci_dev *hdev, u32 dump_size); +int hci_devcd_append(struct hci_dev *hdev, struct sk_buff *skb); +int hci_devcd_append_pattern(struct hci_dev *hdev, u8 pattern, u32 len); +int hci_devcd_complete(struct hci_dev *hdev); +int hci_devcd_abort(struct hci_dev *hdev); + +#else + +static inline void hci_devcd_reset(struct hci_dev *hdev) {} +static inline void hci_devcd_rx(struct work_struct *work) {} +static inline void hci_devcd_timeout(struct work_struct *work) {} + +static inline int hci_devcd_register(struct hci_dev *hdev, coredump_t coredump, + dmp_hdr_t dmp_hdr, + notify_change_t notify_change) +{ + return -EOPNOTSUPP; +} + +static inline int hci_devcd_init(struct hci_dev *hdev, u32 dump_size) +{ + return -EOPNOTSUPP; +} + +static inline int hci_devcd_append(struct hci_dev *hdev, struct sk_buff *skb) +{ + return -EOPNOTSUPP; +} + +static inline int hci_devcd_append_pattern(struct hci_dev *hdev, + u8 pattern, u32 len) +{ + return -EOPNOTSUPP; +} + +static inline int hci_devcd_complete(struct hci_dev *hdev) +{ + return -EOPNOTSUPP; +} + +static inline int hci_devcd_abort(struct hci_dev *hdev) +{ + return -EOPNOTSUPP; +} + +#endif /* CONFIG_DEV_COREDUMP */ + +#endif /* __COREDUMP_H */ diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index e22e45fbe8db..a461335beacf 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -32,6 +32,7 @@ #include #include #include +#include /* HCI priority */ #define HCI_PRIO_MAX 7 @@ -590,6 +591,10 @@ struct hci_dev { const char *fw_info; struct dentry *debugfs; +#ifdef CONFIG_DEV_COREDUMP + struct hci_devcoredump dump; +#endif + struct device dev; struct rfkill *rfkill; @@ -1497,6 +1502,15 @@ static inline void hci_set_aosp_capable(struct hci_dev *hdev) #endif } +static inline void hci_devcd_setup(struct hci_dev *hdev) +{ +#ifdef CONFIG_DEV_COREDUMP + INIT_WORK(&hdev->dump.dump_rx, hci_devcd_rx); + INIT_DELAYED_WORK(&hdev->dump.dump_timeout, hci_devcd_timeout); + skb_queue_head_init(&hdev->dump.dump_q); +#endif +} + int hci_dev_open(__u16 dev); int hci_dev_close(__u16 dev); int hci_dev_do_close(struct hci_dev *hdev); diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile index 0e7b7db42750..141ac1fda0bf 100644 --- a/net/bluetooth/Makefile +++ b/net/bluetooth/Makefile @@ -17,6 +17,8 @@ bluetooth-y := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o \ ecdh_helper.o hci_request.o mgmt_util.o mgmt_config.o hci_codec.o \ eir.o hci_sync.o +bluetooth-$(CONFIG_DEV_COREDUMP) += coredump.o + bluetooth-$(CONFIG_BT_BREDR) += sco.o bluetooth-$(CONFIG_BT_LE) += iso.o bluetooth-$(CONFIG_BT_HS) += a2mp.o amp.o diff --git a/net/bluetooth/coredump.c b/net/bluetooth/coredump.c new file mode 100644 index 000000000000..08fa98505454 --- /dev/null +++ b/net/bluetooth/coredump.c @@ -0,0 +1,535 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2023 Google Corporation + */ + +#include + +#include +#include + +enum hci_devcoredump_pkt_type { + HCI_DEVCOREDUMP_PKT_INIT, + HCI_DEVCOREDUMP_PKT_SKB, + HCI_DEVCOREDUMP_PKT_PATTERN, + HCI_DEVCOREDUMP_PKT_COMPLETE, + HCI_DEVCOREDUMP_PKT_ABORT, +}; + +struct hci_devcoredump_skb_cb { + u16 pkt_type; +}; + +struct hci_devcoredump_skb_pattern { + u8 pattern; + u32 len; +} __packed; + +#define hci_dmp_cb(skb) ((struct hci_devcoredump_skb_cb *)((skb)->cb)) + +#define DBG_UNEXPECTED_STATE() \ + bt_dev_dbg(hdev, \ + "Unexpected packet (%d) for state (%d). ", \ + hci_dmp_cb(skb)->pkt_type, hdev->dump.state) + +#define MAX_DEVCOREDUMP_HDR_SIZE 512 /* bytes */ + +static int hci_devcd_update_hdr_state(char *buf, size_t size, int state) +{ + int len = 0; + + if (!buf) + return 0; + + len = scnprintf(buf, size, "Bluetooth devcoredump\nState: %d\n", state); + + return len + 1; /* scnprintf adds \0 at the end upon state rewrite */ +} + +/* Call with hci_dev_lock only. */ +static int hci_devcd_update_state(struct hci_dev *hdev, int state) +{ + bt_dev_dbg(hdev, "Updating devcoredump state from %d to %d.", + hdev->dump.state, state); + + hdev->dump.state = state; + + return hci_devcd_update_hdr_state(hdev->dump.head, + hdev->dump.alloc_size, state); +} + +static int hci_devcd_mkheader(struct hci_dev *hdev, struct sk_buff *skb) +{ + char dump_start[] = "--- Start dump ---\n"; + char hdr[80]; + int hdr_len; + + hdr_len = hci_devcd_update_hdr_state(hdr, sizeof(hdr), + HCI_DEVCOREDUMP_IDLE); + skb_put_data(skb, hdr, hdr_len); + + if (hdev->dump.dmp_hdr) + hdev->dump.dmp_hdr(hdev, skb); + + skb_put_data(skb, dump_start, strlen(dump_start)); + + return skb->len; +} + +/* Do not call with hci_dev_lock since this calls driver code. */ +static void hci_devcd_notify(struct hci_dev *hdev, int state) +{ + if (hdev->dump.notify_change) + hdev->dump.notify_change(hdev, state); +} + +/* Call with hci_dev_lock only. */ +void hci_devcd_reset(struct hci_dev *hdev) +{ + hdev->dump.head = NULL; + hdev->dump.tail = NULL; + hdev->dump.alloc_size = 0; + + hci_devcd_update_state(hdev, HCI_DEVCOREDUMP_IDLE); + + cancel_delayed_work(&hdev->dump.dump_timeout); + skb_queue_purge(&hdev->dump.dump_q); +} + +/* Call with hci_dev_lock only. */ +static void hci_devcd_free(struct hci_dev *hdev) +{ + if (hdev->dump.head) + vfree(hdev->dump.head); + + hci_devcd_reset(hdev); +} + +/* Call with hci_dev_lock only. */ +static int hci_devcd_alloc(struct hci_dev *hdev, u32 size) +{ + hdev->dump.head = vmalloc(size); + if (!hdev->dump.head) + return -ENOMEM; + + hdev->dump.alloc_size = size; + hdev->dump.tail = hdev->dump.head; + hdev->dump.end = hdev->dump.head + size; + + hci_devcd_update_state(hdev, HCI_DEVCOREDUMP_IDLE); + + return 0; +} + +/* Call with hci_dev_lock only. */ +static bool hci_devcd_copy(struct hci_dev *hdev, char *buf, u32 size) +{ + if (hdev->dump.tail + size > hdev->dump.end) + return false; + + memcpy(hdev->dump.tail, buf, size); + hdev->dump.tail += size; + + return true; +} + +/* Call with hci_dev_lock only. */ +static bool hci_devcd_memset(struct hci_dev *hdev, u8 pattern, u32 len) +{ + if (hdev->dump.tail + len > hdev->dump.end) + return false; + + memset(hdev->dump.tail, pattern, len); + hdev->dump.tail += len; + + return true; +} + +/* Call with hci_dev_lock only. */ +static int hci_devcd_prepare(struct hci_dev *hdev, u32 dump_size) +{ + struct sk_buff *skb; + int dump_hdr_size; + int err = 0; + + skb = alloc_skb(MAX_DEVCOREDUMP_HDR_SIZE, GFP_ATOMIC); + if (!skb) + return -ENOMEM; + + dump_hdr_size = hci_devcd_mkheader(hdev, skb); + + if (hci_devcd_alloc(hdev, dump_hdr_size + dump_size)) { + err = -ENOMEM; + goto hdr_free; + } + + /* Insert the device header */ + if (!hci_devcd_copy(hdev, skb->data, skb->len)) { + bt_dev_err(hdev, "Failed to insert header"); + hci_devcd_free(hdev); + + err = -ENOMEM; + goto hdr_free; + } + +hdr_free: + kfree_skb(skb); + + return err; +} + +static void hci_devcd_handle_pkt_init(struct hci_dev *hdev, struct sk_buff *skb) +{ + u32 *dump_size; + + if (hdev->dump.state != HCI_DEVCOREDUMP_IDLE) { + DBG_UNEXPECTED_STATE(); + return; + } + + if (skb->len != sizeof(*dump_size)) { + bt_dev_dbg(hdev, "Invalid dump init pkt"); + return; + } + + dump_size = skb_pull_data(skb, sizeof(*dump_size)); + if (!*dump_size) { + bt_dev_err(hdev, "Zero size dump init pkt"); + return; + } + + if (hci_devcd_prepare(hdev, *dump_size)) { + bt_dev_err(hdev, "Failed to prepare for dump"); + return; + } + + hci_devcd_update_state(hdev, HCI_DEVCOREDUMP_ACTIVE); + queue_delayed_work(hdev->workqueue, &hdev->dump.dump_timeout, + hdev->dump.timeout); +} + +static void hci_devcd_handle_pkt_skb(struct hci_dev *hdev, struct sk_buff *skb) +{ + if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) { + DBG_UNEXPECTED_STATE(); + return; + } + + if (!hci_devcd_copy(hdev, skb->data, skb->len)) + bt_dev_dbg(hdev, "Failed to insert skb"); +} + +static void hci_devcd_handle_pkt_pattern(struct hci_dev *hdev, + struct sk_buff *skb) +{ + struct hci_devcoredump_skb_pattern *pattern; + + if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) { + DBG_UNEXPECTED_STATE(); + return; + } + + if (skb->len != sizeof(*pattern)) { + bt_dev_dbg(hdev, "Invalid pattern skb"); + return; + } + + pattern = skb_pull_data(skb, sizeof(*pattern)); + + if (!hci_devcd_memset(hdev, pattern->pattern, pattern->len)) + bt_dev_dbg(hdev, "Failed to set pattern"); +} + +static void hci_devcd_handle_pkt_complete(struct hci_dev *hdev, + struct sk_buff *skb) +{ + u32 dump_size; + + if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) { + DBG_UNEXPECTED_STATE(); + return; + } + + hci_devcd_update_state(hdev, HCI_DEVCOREDUMP_DONE); + dump_size = hdev->dump.tail - hdev->dump.head; + + bt_dev_dbg(hdev, "complete with size %u (expect %zu)", dump_size, + hdev->dump.alloc_size); + + dev_coredumpv(&hdev->dev, hdev->dump.head, dump_size, GFP_KERNEL); +} + +static void hci_devcd_handle_pkt_abort(struct hci_dev *hdev, + struct sk_buff *skb) +{ + u32 dump_size; + + if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) { + DBG_UNEXPECTED_STATE(); + return; + } + + hci_devcd_update_state(hdev, HCI_DEVCOREDUMP_ABORT); + dump_size = hdev->dump.tail - hdev->dump.head; + + bt_dev_dbg(hdev, "aborted with size %u (expect %zu)", dump_size, + hdev->dump.alloc_size); + + /* Emit a devcoredump with the available data */ + dev_coredumpv(&hdev->dev, hdev->dump.head, dump_size, GFP_KERNEL); +} + +/* Bluetooth devcoredump state machine. + * + * Devcoredump states: + * + * HCI_DEVCOREDUMP_IDLE: The default state. + * + * HCI_DEVCOREDUMP_ACTIVE: A devcoredump will be in this state once it has + * been initialized using hci_devcd_init(). Once active, the driver + * can append data using hci_devcd_append() or insert a pattern + * using hci_devcd_append_pattern(). + * + * HCI_DEVCOREDUMP_DONE: Once the dump collection is complete, the drive + * can signal the completion using hci_devcd_complete(). A + * devcoredump is generated indicating the completion event and + * then the state machine is reset to the default state. + * + * HCI_DEVCOREDUMP_ABORT: The driver can cancel ongoing dump collection in + * case of any error using hci_devcd_abort(). A devcoredump is + * still generated with the available data indicating the abort + * event and then the state machine is reset to the default state. + * + * HCI_DEVCOREDUMP_TIMEOUT: A timeout timer for HCI_DEVCOREDUMP_TIMEOUT sec + * is started during devcoredump initialization. Once the timeout + * occurs, the driver is notified, a devcoredump is generated with + * the available data indicating the timeout event and then the + * state machine is reset to the default state. + * + * The driver must register using hci_devcd_register() before using the hci + * devcoredump APIs. + */ +void hci_devcd_rx(struct work_struct *work) +{ + struct hci_dev *hdev = container_of(work, struct hci_dev, dump.dump_rx); + struct sk_buff *skb; + int start_state; + + while ((skb = skb_dequeue(&hdev->dump.dump_q))) { + /* Return if timeout occurs. The timeout handler function + * hci_devcd_timeout() will report the available dump data. + */ + if (hdev->dump.state == HCI_DEVCOREDUMP_TIMEOUT) { + kfree_skb(skb); + return; + } + + hci_dev_lock(hdev); + start_state = hdev->dump.state; + + switch (hci_dmp_cb(skb)->pkt_type) { + case HCI_DEVCOREDUMP_PKT_INIT: + hci_devcd_handle_pkt_init(hdev, skb); + break; + + case HCI_DEVCOREDUMP_PKT_SKB: + hci_devcd_handle_pkt_skb(hdev, skb); + break; + + case HCI_DEVCOREDUMP_PKT_PATTERN: + hci_devcd_handle_pkt_pattern(hdev, skb); + break; + + case HCI_DEVCOREDUMP_PKT_COMPLETE: + hci_devcd_handle_pkt_complete(hdev, skb); + break; + + case HCI_DEVCOREDUMP_PKT_ABORT: + hci_devcd_handle_pkt_abort(hdev, skb); + break; + + default: + bt_dev_dbg(hdev, "Unknown packet (%d) for state (%d). ", + hci_dmp_cb(skb)->pkt_type, hdev->dump.state); + break; + } + + hci_dev_unlock(hdev); + kfree_skb(skb); + + /* Notify the driver about any state changes before resetting + * the state machine + */ + if (start_state != hdev->dump.state) + hci_devcd_notify(hdev, hdev->dump.state); + + /* Reset the state machine if the devcoredump is complete */ + hci_dev_lock(hdev); + if (hdev->dump.state == HCI_DEVCOREDUMP_DONE || + hdev->dump.state == HCI_DEVCOREDUMP_ABORT) + hci_devcd_reset(hdev); + hci_dev_unlock(hdev); + } +} +EXPORT_SYMBOL(hci_devcd_rx); + +void hci_devcd_timeout(struct work_struct *work) +{ + struct hci_dev *hdev = container_of(work, struct hci_dev, + dump.dump_timeout.work); + u32 dump_size; + + hci_devcd_notify(hdev, HCI_DEVCOREDUMP_TIMEOUT); + + hci_dev_lock(hdev); + + cancel_work(&hdev->dump.dump_rx); + + hci_devcd_update_state(hdev, HCI_DEVCOREDUMP_TIMEOUT); + + dump_size = hdev->dump.tail - hdev->dump.head; + bt_dev_dbg(hdev, "timeout with size %u (expect %zu)", dump_size, + hdev->dump.alloc_size); + + /* Emit a devcoredump with the available data */ + dev_coredumpv(&hdev->dev, hdev->dump.head, dump_size, GFP_KERNEL); + + hci_devcd_reset(hdev); + + hci_dev_unlock(hdev); +} +EXPORT_SYMBOL(hci_devcd_timeout); + +int hci_devcd_register(struct hci_dev *hdev, coredump_t coredump, + dmp_hdr_t dmp_hdr, notify_change_t notify_change) +{ + /* Driver must implement coredump() and dmp_hdr() functions for + * bluetooth devcoredump. The coredump() should trigger a coredump + * event on the controller when the device's coredump sysfs entry is + * written to. The dmp_hdr() should create a dump header to identify + * the controller/fw/driver info. + */ + if (!coredump || !dmp_hdr) + return -EINVAL; + + hci_dev_lock(hdev); + hdev->dump.coredump = coredump; + hdev->dump.dmp_hdr = dmp_hdr; + hdev->dump.notify_change = notify_change; + hdev->dump.supported = true; + hdev->dump.timeout = DEVCOREDUMP_TIMEOUT; + hci_dev_unlock(hdev); + + return 0; +} +EXPORT_SYMBOL(hci_devcd_register); + +static inline bool hci_devcd_enabled(struct hci_dev *hdev) +{ + return hdev->dump.supported; +} + +int hci_devcd_init(struct hci_dev *hdev, u32 dump_size) +{ + struct sk_buff *skb; + + if (!hci_devcd_enabled(hdev)) + return -EOPNOTSUPP; + + skb = alloc_skb(sizeof(dump_size), GFP_ATOMIC); + if (!skb) + return -ENOMEM; + + hci_dmp_cb(skb)->pkt_type = HCI_DEVCOREDUMP_PKT_INIT; + skb_put_data(skb, &dump_size, sizeof(dump_size)); + + skb_queue_tail(&hdev->dump.dump_q, skb); + queue_work(hdev->workqueue, &hdev->dump.dump_rx); + + return 0; +} +EXPORT_SYMBOL(hci_devcd_init); + +int hci_devcd_append(struct hci_dev *hdev, struct sk_buff *skb) +{ + if (!skb) + return -ENOMEM; + + if (!hci_devcd_enabled(hdev)) { + kfree_skb(skb); + return -EOPNOTSUPP; + } + + hci_dmp_cb(skb)->pkt_type = HCI_DEVCOREDUMP_PKT_SKB; + + skb_queue_tail(&hdev->dump.dump_q, skb); + queue_work(hdev->workqueue, &hdev->dump.dump_rx); + + return 0; +} +EXPORT_SYMBOL(hci_devcd_append); + +int hci_devcd_append_pattern(struct hci_dev *hdev, u8 pattern, u32 len) +{ + struct hci_devcoredump_skb_pattern p; + struct sk_buff *skb; + + if (!hci_devcd_enabled(hdev)) + return -EOPNOTSUPP; + + skb = alloc_skb(sizeof(p), GFP_ATOMIC); + if (!skb) + return -ENOMEM; + + p.pattern = pattern; + p.len = len; + + hci_dmp_cb(skb)->pkt_type = HCI_DEVCOREDUMP_PKT_PATTERN; + skb_put_data(skb, &p, sizeof(p)); + + skb_queue_tail(&hdev->dump.dump_q, skb); + queue_work(hdev->workqueue, &hdev->dump.dump_rx); + + return 0; +} +EXPORT_SYMBOL(hci_devcd_append_pattern); + +int hci_devcd_complete(struct hci_dev *hdev) +{ + struct sk_buff *skb; + + if (!hci_devcd_enabled(hdev)) + return -EOPNOTSUPP; + + skb = alloc_skb(0, GFP_ATOMIC); + if (!skb) + return -ENOMEM; + + hci_dmp_cb(skb)->pkt_type = HCI_DEVCOREDUMP_PKT_COMPLETE; + + skb_queue_tail(&hdev->dump.dump_q, skb); + queue_work(hdev->workqueue, &hdev->dump.dump_rx); + + return 0; +} +EXPORT_SYMBOL(hci_devcd_complete); + +int hci_devcd_abort(struct hci_dev *hdev) +{ + struct sk_buff *skb; + + if (!hci_devcd_enabled(hdev)) + return -EOPNOTSUPP; + + skb = alloc_skb(0, GFP_ATOMIC); + if (!skb) + return -ENOMEM; + + hci_dmp_cb(skb)->pkt_type = HCI_DEVCOREDUMP_PKT_ABORT; + + skb_queue_tail(&hdev->dump.dump_q, skb); + queue_work(hdev->workqueue, &hdev->dump.dump_rx); + + return 0; +} +EXPORT_SYMBOL(hci_devcd_abort); diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 334e308451f5..393b317ae68f 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -2544,6 +2544,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv) INIT_DELAYED_WORK(&hdev->cmd_timer, hci_cmd_timeout); INIT_DELAYED_WORK(&hdev->ncmd_timer, hci_ncmd_timeout); + hci_devcd_setup(hdev); hci_request_setup(hdev); hci_init_sysfs(hdev); diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 31231f0e4a28..f21497ebc814 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -4727,6 +4727,8 @@ int hci_dev_open_sync(struct hci_dev *hdev) goto done; } + hci_devcd_reset(hdev); + set_bit(HCI_RUNNING, &hdev->flags); hci_sock_dev_event(hdev, HCI_DEV_OPEN); -- cgit v1.2.3 From 288c90224eec55d13e786844b7954ef060752089 Mon Sep 17 00:00:00 2001 From: Luiz Augusto von Dentz Date: Mon, 19 Dec 2022 13:37:02 -0800 Subject: Bluetooth: Enable all supported LE PHY by default This enables 2M and Coded PHY by default if they are marked as supported in the LE features bits. Signed-off-by: Luiz Augusto von Dentz --- include/net/bluetooth/hci_core.h | 4 ++++ net/bluetooth/hci_sync.c | 28 ++++++++++++++++++++++++---- net/bluetooth/mgmt.c | 4 ++-- 3 files changed, 30 insertions(+), 6 deletions(-) (limited to 'net/bluetooth/hci_sync.c') diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index f11689284112..827e67159523 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1683,9 +1683,13 @@ void hci_conn_del_sysfs(struct hci_conn *conn); #define scan_1m(dev) (((dev)->le_tx_def_phys & HCI_LE_SET_PHY_1M) || \ ((dev)->le_rx_def_phys & HCI_LE_SET_PHY_1M)) +#define le_2m_capable(dev) (((dev)->le_features[1] & HCI_LE_PHY_2M)) + #define scan_2m(dev) (((dev)->le_tx_def_phys & HCI_LE_SET_PHY_2M) || \ ((dev)->le_rx_def_phys & HCI_LE_SET_PHY_2M)) +#define le_coded_capable(dev) (((dev)->le_features[1] & HCI_LE_PHY_CODED)) + #define scan_coded(dev) (((dev)->le_tx_def_phys & HCI_LE_SET_PHY_CODED) || \ ((dev)->le_rx_def_phys & HCI_LE_SET_PHY_CODED)) diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index f21497ebc814..00017f75cd41 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -4414,18 +4414,38 @@ static int hci_le_set_write_def_data_len_sync(struct hci_dev *hdev) sizeof(cp), &cp, HCI_CMD_TIMEOUT); } -/* Set Default PHY parameters if command is supported */ +/* Set Default PHY parameters if command is supported, enables all supported + * PHYs according to the LE Features bits. + */ static int hci_le_set_default_phy_sync(struct hci_dev *hdev) { struct hci_cp_le_set_default_phy cp; - if (!(hdev->commands[35] & 0x20)) + if (!(hdev->commands[35] & 0x20)) { + /* If the command is not supported it means only 1M PHY is + * supported. + */ + hdev->le_tx_def_phys = HCI_LE_SET_PHY_1M; + hdev->le_rx_def_phys = HCI_LE_SET_PHY_1M; return 0; + } memset(&cp, 0, sizeof(cp)); cp.all_phys = 0x00; - cp.tx_phys = hdev->le_tx_def_phys; - cp.rx_phys = hdev->le_rx_def_phys; + cp.tx_phys = HCI_LE_SET_PHY_1M; + cp.rx_phys = HCI_LE_SET_PHY_1M; + + /* Enables 2M PHY if supported */ + if (le_2m_capable(hdev)) { + cp.tx_phys |= HCI_LE_SET_PHY_2M; + cp.rx_phys |= HCI_LE_SET_PHY_2M; + } + + /* Enables Coded PHY if supported */ + if (le_coded_capable(hdev)) { + cp.tx_phys |= HCI_LE_SET_PHY_CODED; + cp.rx_phys |= HCI_LE_SET_PHY_CODED; + } return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_DEFAULT_PHY, sizeof(cp), &cp, HCI_CMD_TIMEOUT); diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 249dc6777fb4..5f8c144c84b8 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -8393,10 +8393,10 @@ static u32 get_supported_adv_flags(struct hci_dev *hdev) flags |= MGMT_ADV_FLAG_HW_OFFLOAD; flags |= MGMT_ADV_FLAG_CAN_SET_TX_POWER; - if (hdev->le_features[1] & HCI_LE_PHY_2M) + if (le_2m_capable(hdev)) flags |= MGMT_ADV_FLAG_SEC_2M; - if (hdev->le_features[1] & HCI_LE_PHY_CODED) + if (le_coded_capable(hdev)) flags |= MGMT_ADV_FLAG_SEC_CODED; } -- cgit v1.2.3 From c09b80be6ffc338634b2f5f8cfa12b6843410834 Mon Sep 17 00:00:00 2001 From: Luiz Augusto von Dentz Date: Wed, 12 Apr 2023 17:45:51 -0700 Subject: Bluetooth: hci_conn: Fix not waiting for HCI_EVT_LE_CIS_ESTABLISHED When submitting HCI_OP_LE_CREATE_CIS the code shall wait for HCI_EVT_LE_CIS_ESTABLISHED thus enforcing the serialization of HCI_OP_LE_CREATE_CIS as the Core spec does not allow to send them in parallel: BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E page 2566: If the Host issues this command before all the HCI_LE_CIS_Established events from the previous use of the command have been generated, the Controller shall return the error code Command Disallowed (0x0C). Signed-off-by: Luiz Augusto von Dentz --- include/net/bluetooth/hci_sync.h | 2 ++ net/bluetooth/hci_conn.c | 58 +---------------------------------- net/bluetooth/hci_sync.c | 65 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 57 deletions(-) (limited to 'net/bluetooth/hci_sync.c') diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h index 17f5a4c32f36..f61b249787fc 100644 --- a/include/net/bluetooth/hci_sync.h +++ b/include/net/bluetooth/hci_sync.h @@ -122,6 +122,8 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason); int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn); +int hci_le_create_cis_sync(struct hci_dev *hdev, struct hci_conn *conn); + int hci_le_remove_cig_sync(struct hci_dev *hdev, u8 handle); int hci_le_terminate_big_sync(struct hci_dev *hdev, u8 handle, u8 reason); diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index c215e983e287..640b951bf40a 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1932,63 +1932,7 @@ bool hci_iso_setup_path(struct hci_conn *conn) static int hci_create_cis_sync(struct hci_dev *hdev, void *data) { - struct { - struct hci_cp_le_create_cis cp; - struct hci_cis cis[0x1f]; - } cmd; - struct hci_conn *conn = data; - u8 cig; - - memset(&cmd, 0, sizeof(cmd)); - cmd.cis[0].acl_handle = cpu_to_le16(conn->parent->handle); - cmd.cis[0].cis_handle = cpu_to_le16(conn->handle); - cmd.cp.num_cis++; - cig = conn->iso_qos.ucast.cig; - - hci_dev_lock(hdev); - - rcu_read_lock(); - - list_for_each_entry_rcu(conn, &hdev->conn_hash.list, list) { - struct hci_cis *cis = &cmd.cis[cmd.cp.num_cis]; - - if (conn == data || conn->type != ISO_LINK || - conn->state == BT_CONNECTED || - conn->iso_qos.ucast.cig != cig) - continue; - - /* Check if all CIS(s) belonging to a CIG are ready */ - if (!conn->parent || conn->parent->state != BT_CONNECTED || - conn->state != BT_CONNECT) { - cmd.cp.num_cis = 0; - break; - } - - /* Group all CIS with state BT_CONNECT since the spec don't - * allow to send them individually: - * - * BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E - * page 2566: - * - * If the Host issues this command before all the - * HCI_LE_CIS_Established events from the previous use of the - * command have been generated, the Controller shall return the - * error code Command Disallowed (0x0C). - */ - cis->acl_handle = cpu_to_le16(conn->parent->handle); - cis->cis_handle = cpu_to_le16(conn->handle); - cmd.cp.num_cis++; - } - - rcu_read_unlock(); - - hci_dev_unlock(hdev); - - if (!cmd.cp.num_cis) - return 0; - - return hci_send_cmd(hdev, HCI_OP_LE_CREATE_CIS, sizeof(cmd.cp) + - sizeof(cmd.cis[0]) * cmd.cp.num_cis, &cmd); + return hci_le_create_cis_sync(hdev, data); } int hci_le_create_cis(struct hci_conn *conn) diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 00017f75cd41..6f060d00a70a 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -6137,6 +6137,71 @@ done: return err; } +int hci_le_create_cis_sync(struct hci_dev *hdev, struct hci_conn *conn) +{ + struct { + struct hci_cp_le_create_cis cp; + struct hci_cis cis[0x1f]; + } cmd; + u8 cig; + struct hci_conn *hcon = conn; + + memset(&cmd, 0, sizeof(cmd)); + cmd.cis[0].acl_handle = cpu_to_le16(conn->parent->handle); + cmd.cis[0].cis_handle = cpu_to_le16(conn->handle); + cmd.cp.num_cis++; + cig = conn->iso_qos.ucast.cig; + + hci_dev_lock(hdev); + + rcu_read_lock(); + + list_for_each_entry_rcu(conn, &hdev->conn_hash.list, list) { + struct hci_cis *cis = &cmd.cis[cmd.cp.num_cis]; + + if (conn == hcon || conn->type != ISO_LINK || + conn->state == BT_CONNECTED || + conn->iso_qos.ucast.cig != cig) + continue; + + /* Check if all CIS(s) belonging to a CIG are ready */ + if (!conn->parent || conn->parent->state != BT_CONNECTED || + conn->state != BT_CONNECT) { + cmd.cp.num_cis = 0; + break; + } + + /* Group all CIS with state BT_CONNECT since the spec don't + * allow to send them individually: + * + * BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E + * page 2566: + * + * If the Host issues this command before all the + * HCI_LE_CIS_Established events from the previous use of the + * command have been generated, the Controller shall return the + * error code Command Disallowed (0x0C). + */ + cis->acl_handle = cpu_to_le16(conn->parent->handle); + cis->cis_handle = cpu_to_le16(conn->handle); + cmd.cp.num_cis++; + } + + rcu_read_unlock(); + + hci_dev_unlock(hdev); + + if (!cmd.cp.num_cis) + return 0; + + /* Wait for HCI_LE_CIS_Established */ + return __hci_cmd_sync_status_sk(hdev, HCI_OP_LE_CREATE_CIS, + sizeof(cmd.cp) + sizeof(cmd.cis[0]) * + cmd.cp.num_cis, &cmd, + HCI_EVT_LE_CIS_ESTABLISHED, + conn->conn_timeout, NULL); +} + int hci_le_remove_cig_sync(struct hci_dev *hdev, u8 handle) { struct hci_cp_le_remove_cig cp; -- cgit v1.2.3 From 91b6d02ddcd113352bdd895990b252065c596de7 Mon Sep 17 00:00:00 2001 From: Raul Cheleguini Date: Thu, 23 Mar 2023 10:45:39 -0300 Subject: Bluetooth: Add new quirk for broken set random RPA timeout for ATS2851 The ATS2851 based controller advertises support for command "LE Set Random Private Address Timeout" but does not actually implement it, impeding the controller initialization. Add the quirk HCI_QUIRK_BROKEN_SET_RPA_TIMEOUT to unblock the controller initialization. < HCI Command: LE Set Resolvable Private... (0x08|0x002e) plen 2 Timeout: 900 seconds > HCI Event: Command Status (0x0f) plen 4 LE Set Resolvable Private Address Timeout (0x08|0x002e) ncmd 1 Status: Unknown HCI Command (0x01) Co-developed-by: imoc Signed-off-by: imoc Signed-off-by: Raul Cheleguini Signed-off-by: Luiz Augusto von Dentz --- drivers/bluetooth/btusb.c | 1 + include/net/bluetooth/hci.h | 8 ++++++++ net/bluetooth/hci_sync.c | 6 +++++- 3 files changed, 14 insertions(+), 1 deletion(-) (limited to 'net/bluetooth/hci_sync.c') diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index 2303b0a66323..3aa189b1986d 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -4149,6 +4149,7 @@ static int btusb_probe(struct usb_interface *intf, /* Support is advertised, but not implemented */ set_bit(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING, &hdev->quirks); set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks); + set_bit(HCI_QUIRK_BROKEN_SET_RPA_TIMEOUT, &hdev->quirks); set_bit(HCI_QUIRK_BROKEN_EXT_SCAN, &hdev->quirks); } diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index 997107bfc0b1..07df96c47ef4 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -301,6 +301,14 @@ enum { * don't actually support features declared there. */ HCI_QUIRK_BROKEN_LOCAL_EXT_FEATURES_PAGE_2, + + /* + * When this quirk is set, the HCI_OP_LE_SET_RPA_TIMEOUT command is + * skipped during initialization. This is required for the Actions + * Semiconductor ATS2851 based controllers, which erroneously claims + * to support it. + */ + HCI_QUIRK_BROKEN_SET_RPA_TIMEOUT, }; /* HCI device flags */ diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 6f060d00a70a..771aaa808967 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -4093,7 +4093,8 @@ static int hci_le_set_rpa_timeout_sync(struct hci_dev *hdev) { __le16 timeout = cpu_to_le16(hdev->rpa_timeout); - if (!(hdev->commands[35] & 0x04)) + if (!(hdev->commands[35] & 0x04) || + test_bit(HCI_QUIRK_BROKEN_SET_RPA_TIMEOUT, &hdev->quirks)) return 0; return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_RPA_TIMEOUT, @@ -4553,6 +4554,9 @@ static const struct { "HCI Set Event Filter command not supported."), HCI_QUIRK_BROKEN(ENHANCED_SETUP_SYNC_CONN, "HCI Enhanced Setup Synchronous Connection command is " + "advertised, but not supported."), + HCI_QUIRK_BROKEN(SET_RPA_TIMEOUT, + "HCI LE Set Random Private Address Timeout command is " "advertised, but not supported.") }; -- cgit v1.2.3 From d883a4669a1def6d121ccf5e64ad28260d1c9531 Mon Sep 17 00:00:00 2001 From: Luiz Augusto von Dentz Date: Fri, 21 Apr 2023 11:37:55 -0700 Subject: Bluetooth: hci_sync: Only allow hci_cmd_sync_queue if running This makes sure hci_cmd_sync_queue only queue new work if HCI_RUNNING has been set otherwise there is a risk of commands being sent while turning off. Because hci_cmd_sync_queue can no longer queue work while HCI_RUNNING is not set it cannot be used to power on adapters so instead hci_cmd_sync_submit is introduced which bypass the HCI_RUNNING check, so it behaves like the old implementation. Link: https://lore.kernel.org/all/CAB4PzUpDMvdc8j2MdeSAy1KkAE-D3woprCwAdYWeOc-3v3c9Sw@mail.gmail.com/ Signed-off-by: Luiz Augusto von Dentz --- include/net/bluetooth/hci_sync.h | 2 ++ net/bluetooth/hci_sync.c | 25 +++++++++++++++++++++++-- net/bluetooth/mgmt.c | 12 ++++++++---- 3 files changed, 33 insertions(+), 6 deletions(-) (limited to 'net/bluetooth/hci_sync.c') diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h index f61b249787fc..2495be4d8b82 100644 --- a/include/net/bluetooth/hci_sync.h +++ b/include/net/bluetooth/hci_sync.h @@ -41,6 +41,8 @@ void hci_cmd_sync_clear(struct hci_dev *hdev); void hci_cmd_sync_cancel(struct hci_dev *hdev, int err); void __hci_cmd_sync_cancel(struct hci_dev *hdev, int err); +int hci_cmd_sync_submit(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy); int hci_cmd_sync_queue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, void *data, hci_cmd_sync_work_destroy_t destroy); diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 771aaa808967..647a8ce54062 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -684,8 +684,12 @@ void hci_cmd_sync_cancel(struct hci_dev *hdev, int err) } EXPORT_SYMBOL(hci_cmd_sync_cancel); -int hci_cmd_sync_queue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, - void *data, hci_cmd_sync_work_destroy_t destroy) +/* Submit HCI command to be run in as cmd_sync_work: + * + * - hdev must _not_ be unregistered + */ +int hci_cmd_sync_submit(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy) { struct hci_cmd_sync_work_entry *entry; @@ -708,6 +712,23 @@ int hci_cmd_sync_queue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, return 0; } +EXPORT_SYMBOL(hci_cmd_sync_submit); + +/* Queue HCI command: + * + * - hdev must be running + */ +int hci_cmd_sync_queue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy) +{ + /* Only queue command if hdev is running which means it had been opened + * and is either on init phase or is already up. + */ + if (!test_bit(HCI_RUNNING, &hdev->flags)) + return -ENETDOWN; + + return hci_cmd_sync_submit(hdev, func, data, destroy); +} EXPORT_SYMBOL(hci_cmd_sync_queue); int hci_update_eir_sync(struct hci_dev *hdev) diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 13c745876b39..f7b2d0971f24 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -1400,11 +1400,15 @@ static int set_powered(struct sock *sk, struct hci_dev *hdev, void *data, } /* Cancel potentially blocking sync operation before power off */ - if (cp->val == 0x00) + if (cp->val == 0x00) { __hci_cmd_sync_cancel(hdev, -EHOSTDOWN); - - err = hci_cmd_sync_queue(hdev, set_powered_sync, cmd, - mgmt_set_powered_complete); + err = hci_cmd_sync_queue(hdev, set_powered_sync, cmd, + mgmt_set_powered_complete); + } else { + /* Use hci_cmd_sync_submit since hdev might not be running */ + err = hci_cmd_sync_submit(hdev, set_powered_sync, cmd, + mgmt_set_powered_complete); + } if (err < 0) mgmt_pending_remove(cmd); -- cgit v1.2.3 From 1857c19941c87eb36ad47f22a406be5dfe5eff9f Mon Sep 17 00:00:00 2001 From: Zhengping Jiang Date: Wed, 24 May 2023 17:11:58 -0700 Subject: Bluetooth: hci_sync: add lock to protect HCI_UNREGISTER When the HCI_UNREGISTER flag is set, no jobs should be scheduled. Fix potential race when HCI_UNREGISTER is set after the flag is tested in hci_cmd_sync_queue. Fixes: 0b94f2651f56 ("Bluetooth: hci_sync: Fix queuing commands when HCI_UNREGISTER is set") Signed-off-by: Zhengping Jiang Signed-off-by: Luiz Augusto von Dentz --- include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_core.c | 2 ++ net/bluetooth/hci_sync.c | 20 ++++++++++++++------ 3 files changed, 17 insertions(+), 6 deletions(-) (limited to 'net/bluetooth/hci_sync.c') diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 8baf34639939..fe3893855cd2 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -515,6 +515,7 @@ struct hci_dev { struct work_struct cmd_sync_work; struct list_head cmd_sync_work_list; struct mutex cmd_sync_work_lock; + struct mutex unregister_lock; struct work_struct cmd_sync_cancel_work; struct work_struct reenable_adv_work; diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 0164b56de12d..48917c68358d 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -2686,7 +2686,9 @@ void hci_unregister_dev(struct hci_dev *hdev) { BT_DBG("%p name %s bus %d", hdev, hdev->name, hdev->bus); + mutex_lock(&hdev->unregister_lock); hci_dev_set_flag(hdev, HCI_UNREGISTER); + mutex_unlock(&hdev->unregister_lock); write_lock(&hci_dev_list_lock); list_del(&hdev->list); diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 647a8ce54062..a59695f04c25 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -629,6 +629,7 @@ void hci_cmd_sync_init(struct hci_dev *hdev) INIT_WORK(&hdev->cmd_sync_work, hci_cmd_sync_work); INIT_LIST_HEAD(&hdev->cmd_sync_work_list); mutex_init(&hdev->cmd_sync_work_lock); + mutex_init(&hdev->unregister_lock); INIT_WORK(&hdev->cmd_sync_cancel_work, hci_cmd_sync_cancel_work); INIT_WORK(&hdev->reenable_adv_work, reenable_adv); @@ -692,14 +693,19 @@ int hci_cmd_sync_submit(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, void *data, hci_cmd_sync_work_destroy_t destroy) { struct hci_cmd_sync_work_entry *entry; + int err = 0; - if (hci_dev_test_flag(hdev, HCI_UNREGISTER)) - return -ENODEV; + mutex_lock(&hdev->unregister_lock); + if (hci_dev_test_flag(hdev, HCI_UNREGISTER)) { + err = -ENODEV; + goto unlock; + } entry = kmalloc(sizeof(*entry), GFP_KERNEL); - if (!entry) - return -ENOMEM; - + if (!entry) { + err = -ENOMEM; + goto unlock; + } entry->func = func; entry->data = data; entry->destroy = destroy; @@ -710,7 +716,9 @@ int hci_cmd_sync_submit(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, queue_work(hdev->req_workqueue, &hdev->cmd_sync_work); - return 0; +unlock: + mutex_unlock(&hdev->unregister_lock); + return err; } EXPORT_SYMBOL(hci_cmd_sync_submit); -- cgit v1.2.3 From fe2ccc6c29d53e14d3c8b3ddf8ad965a92e074ee Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 31 May 2023 10:57:58 +0200 Subject: Bluetooth: fix debugfs registration Since commit ec6cef9cd98d ("Bluetooth: Fix SMP channel registration for unconfigured controllers") the debugfs interface for unconfigured controllers will be created when the controller is configured. There is however currently nothing preventing a controller from being configured multiple time (e.g. setting the device address using btmgmt) which results in failed attempts to register the already registered debugfs entries: debugfs: File 'features' in directory 'hci0' already present! debugfs: File 'manufacturer' in directory 'hci0' already present! debugfs: File 'hci_version' in directory 'hci0' already present! ... debugfs: File 'quirk_simultaneous_discovery' in directory 'hci0' already present! Add a controller flag to avoid trying to register the debugfs interface more than once. Fixes: ec6cef9cd98d ("Bluetooth: Fix SMP channel registration for unconfigured controllers") Cc: stable@vger.kernel.org # 4.0 Signed-off-by: Johan Hovold Signed-off-by: Luiz Augusto von Dentz --- include/net/bluetooth/hci.h | 1 + net/bluetooth/hci_sync.c | 3 +++ 2 files changed, 4 insertions(+) (limited to 'net/bluetooth/hci_sync.c') diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index 07df96c47ef4..872dcb91a540 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -350,6 +350,7 @@ enum { enum { HCI_SETUP, HCI_CONFIG, + HCI_DEBUGFS_CREATED, HCI_AUTO_OFF, HCI_RFKILLED, HCI_MGMT, diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index a59695f04c25..804cde43b4e0 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -4551,6 +4551,9 @@ static int hci_init_sync(struct hci_dev *hdev) !hci_dev_test_flag(hdev, HCI_CONFIG)) return 0; + if (hci_dev_test_and_set_flag(hdev, HCI_DEBUGFS_CREATED)) + return 0; + hci_debugfs_create_common(hdev); if (lmp_bredr_capable(hdev)) -- cgit v1.2.3