Hi Luiz,
>-----Original Message-----
>From: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>
>Sent: Thursday, November 21, 2024 10:14 PM
>To: linux-bluetooth@xxxxxxxxxxxxxxx
>Subject: [PATCH v1] Bluetooth: MGMT: Fix possible deadlocks
>
>From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>
>This fixes possible deadlocks like the following caused by
>hci_cmd_sync_dequeue causing the destroy function to run:
>
> INFO: task kworker/u19:0:143 blocked for more than 120 seconds.
> Tainted: G W O 6.8.0-2024-03-19-intel-next-iLS-24ww14 #1
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> task:kworker/u19:0 state:D stack:0 pid:143 tgid:143 ppid:2
>flags:0x00004000
> Workqueue: hci0 hci_cmd_sync_work [bluetooth] Call Trace:
> <TASK>
> __schedule+0x374/0xaf0
> schedule+0x3c/0xf0
> schedule_preempt_disabled+0x1c/0x30
> __mutex_lock.constprop.0+0x3ef/0x7a0
> __mutex_lock_slowpath+0x13/0x20
> mutex_lock+0x3c/0x50
> mgmt_set_connectable_complete+0xa4/0x150 [bluetooth]
> ? kfree+0x211/0x2a0
> hci_cmd_sync_dequeue+0xae/0x130 [bluetooth]
> ? __pfx_cmd_complete_rsp+0x10/0x10 [bluetooth]
> cmd_complete_rsp+0x26/0x80 [bluetooth]
> mgmt_pending_foreach+0x4d/0x70 [bluetooth]
> __mgmt_power_off+0x8d/0x180 [bluetooth]
> ? _raw_spin_unlock_irq+0x23/0x40
> hci_dev_close_sync+0x445/0x5b0 [bluetooth]
> hci_set_powered_sync+0x149/0x250 [bluetooth]
> set_powered_sync+0x24/0x60 [bluetooth]
> hci_cmd_sync_work+0x90/0x150 [bluetooth]
> process_one_work+0x13e/0x300
> worker_thread+0x2f7/0x420
> ? __pfx_worker_thread+0x10/0x10
> kthread+0x107/0x140
> ? __pfx_kthread+0x10/0x10
> ret_from_fork+0x3d/0x60
> ? __pfx_kthread+0x10/0x10
> ret_from_fork_asm+0x1b/0x30
> </TASK>
>
>Fixes: f53e1c9c726d ("Bluetooth: MGMT: Fix possible crash on
>mgmt_index_removed")
>Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
Tested-by: Kiran K <kiran.k@xxxxxxxxx>
>---
> net/bluetooth/mgmt.c | 27 ++++++++++++++++++---------
> 1 file changed, 18 insertions(+), 9 deletions(-)
>
>diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index
>e406eb8e4327..b31192d473d0 100644
>--- a/net/bluetooth/mgmt.c
>+++ b/net/bluetooth/mgmt.c
>@@ -1518,7 +1518,8 @@ static void mgmt_set_discoverable_complete(struct
>hci_dev *hdev, void *data,
> bt_dev_dbg(hdev, "err %d", err);
>
> /* Make sure cmd still outstanding. */
>- if (cmd != pending_find(MGMT_OP_SET_DISCOVERABLE, hdev))
>+ if (err == -ECANCELED ||
>+ cmd != pending_find(MGMT_OP_SET_DISCOVERABLE, hdev))
> return;
>
> hci_dev_lock(hdev);
>@@ -1692,7 +1693,8 @@ static void mgmt_set_connectable_complete(struct
>hci_dev *hdev, void *data,
> bt_dev_dbg(hdev, "err %d", err);
>
> /* Make sure cmd still outstanding. */
>- if (cmd != pending_find(MGMT_OP_SET_CONNECTABLE, hdev))
>+ if (err == -ECANCELED ||
>+ cmd != pending_find(MGMT_OP_SET_CONNECTABLE, hdev))
> return;
>
> hci_dev_lock(hdev);
>@@ -1924,7 +1926,7 @@ static void set_ssp_complete(struct hci_dev *hdev,
>void *data, int err)
> bool changed;
>
> /* Make sure cmd still outstanding. */
>- if (cmd != pending_find(MGMT_OP_SET_SSP, hdev))
>+ if (err == -ECANCELED || cmd != pending_find(MGMT_OP_SET_SSP,
>hdev))
> return;
>
> if (err) {
>@@ -3848,7 +3850,8 @@ static void set_name_complete(struct hci_dev
>*hdev, void *data, int err)
>
> bt_dev_dbg(hdev, "err %d", err);
>
>- if (cmd != pending_find(MGMT_OP_SET_LOCAL_NAME, hdev))
>+ if (err == -ECANCELED ||
>+ cmd != pending_find(MGMT_OP_SET_LOCAL_NAME, hdev))
> return;
>
> if (status) {
>@@ -4023,7 +4026,8 @@ static void set_default_phy_complete(struct hci_dev
>*hdev, void *data, int err)
> struct sk_buff *skb = cmd->skb;
> u8 status = mgmt_status(err);
>
>- if (cmd != pending_find(MGMT_OP_SET_PHY_CONFIGURATION, hdev))
>+ if (err == -ECANCELED ||
>+ cmd != pending_find(MGMT_OP_SET_PHY_CONFIGURATION, hdev))
> return;
>
> if (!status) {
>@@ -5914,13 +5918,16 @@ static void start_discovery_complete(struct
>hci_dev *hdev, void *data, int err) {
> struct mgmt_pending_cmd *cmd = data;
>
>+ bt_dev_dbg(hdev, "err %d", err);
>+
>+ if (err == -ECANCELED)
>+ return;
>+
> if (cmd != pending_find(MGMT_OP_START_DISCOVERY, hdev) &&
> cmd != pending_find(MGMT_OP_START_LIMITED_DISCOVERY, hdev)
>&&
> cmd != pending_find(MGMT_OP_START_SERVICE_DISCOVERY,
>hdev))
> return;
>
>- bt_dev_dbg(hdev, "err %d", err);
>-
> mgmt_cmd_complete(cmd->sk, cmd->index, cmd->opcode,
>mgmt_status(err),
> cmd->param, 1);
> mgmt_pending_remove(cmd);
>@@ -6153,7 +6160,8 @@ static void stop_discovery_complete(struct hci_dev
>*hdev, void *data, int err) {
> struct mgmt_pending_cmd *cmd = data;
>
>- if (cmd != pending_find(MGMT_OP_STOP_DISCOVERY, hdev))
>+ if (err == -ECANCELED ||
>+ cmd != pending_find(MGMT_OP_STOP_DISCOVERY, hdev))
> return;
>
> bt_dev_dbg(hdev, "err %d", err);
>@@ -8144,7 +8152,8 @@ static void
>read_local_oob_ext_data_complete(struct hci_dev *hdev, void *data,
> u8 status = mgmt_status(err);
> u16 eir_len;
>
>- if (cmd != pending_find(MGMT_OP_READ_LOCAL_OOB_EXT_DATA,
>hdev))
>+ if (err == -ECANCELED ||
>+ cmd != pending_find(MGMT_OP_READ_LOCAL_OOB_EXT_DATA,
>hdev))
> return;
>
> if (!status) {
>--
>2.47.0
>
Thanks,
Kiran
