From: jiaymao <quic_jiaymao@quicinc.com>

Add a verification step to ensure that a command is still in the pending
list before attempting to remove it. A crash may occur during the boot
process when Bluetooth is enabled and then immediately disabled. In a
race condition, mgmt_index_removed() might free the pending command
before mgmt_add_adv_patterns_monitor_complete() is called, leading to a
double free scenario.

Part of the crash call trace:
0x0000053D: __list_del_entry_valid_or_report+0x98/0xdc
0x0000053D: mgmt_pending_remove+0x18/0x58 [bluetooth]
0x0000053E: mgmt_remove_adv_monitor_complete+0x80/0x108 [bluetooth]
0x0000053E: hci_cmd_sync_work+0xbc/0x164 [bluetooth]

Signed-off-by: jiaymao <quic_jiaymao@quicinc.com>
---
 net/bluetooth/mgmt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 25979f4283a6..9d019db92043 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5428,6 +5428,9 @@ static void mgmt_remove_adv_monitor_complete(struct hci_dev *hdev,
 
 	hci_dev_lock(hdev);
 
+	if (cmd != pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev))
+		goto done;
+
 	rp.monitor_handle = cp->monitor_handle;
 
 	if (!status)
@@ -5437,6 +5440,7 @@ static void mgmt_remove_adv_monitor_complete(struct hci_dev *hdev,
 			  mgmt_status(status), &rp, sizeof(rp));
 	mgmt_pending_remove(cmd);
 
+done:
 	hci_dev_unlock(hdev);
 	bt_dev_dbg(hdev, "remove monitor %d complete, status %d",
 		   rp.monitor_handle, status);
-- 
2.46.0

Dear Jiayang,


Thank you for your patch.

Am 09.09.24 um 05:09 schrieb Jiayang Mao:
> From: jiaymao <quic_jiaymao@quicinc.com>

Please use your full name:

     $ git config --global user.name "Jiayang Mao"
     $ git commit -s --amend --author="Jiayang Mao 
<quic_jiaymao@quicinc.com>"

> Add a verification step to ensure that a command is still in the pending
> list before attempting to remove it. A crash may occur during the boot
> process when Bluetooth is enabled and then immediately disabled. In a
> race condition, mgmt_index_removed() might free the pending command
> before mgmt_add_adv_patterns_monitor_complete() is called, leading to a
> double free scenario.
> 
> Part of the crash call trace:
> 0x0000053D: __list_del_entry_valid_or_report+0x98/0xdc
> 0x0000053D: mgmt_pending_remove+0x18/0x58 [bluetooth]
> 0x0000053E: mgmt_remove_adv_monitor_complete+0x80/0x108 [bluetooth]
> 0x0000053E: hci_cmd_sync_work+0xbc/0x164 [bluetooth]

What is your test environment? Please document it.

> Signed-off-by: jiaymao <quic_jiaymao@quicinc.com>

Ditto.

> ---
>   net/bluetooth/mgmt.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 25979f4283a6..9d019db92043 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -5428,6 +5428,9 @@ static void mgmt_remove_adv_monitor_complete(struct hci_dev *hdev,
>   
>   	hci_dev_lock(hdev);
>   
> +	if (cmd != pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev))
> +		goto done;
> +
>   	rp.monitor_handle = cp->monitor_handle;
>   
>   	if (!status)
> @@ -5437,6 +5440,7 @@ static void mgmt_remove_adv_monitor_complete(struct hci_dev *hdev,
>   			  mgmt_status(status), &rp, sizeof(rp));
>   	mgmt_pending_remove(cmd);
>   
> +done:
>   	hci_dev_unlock(hdev);
>   	bt_dev_dbg(hdev, "remove monitor %d complete, status %d",
>   		   rp.monitor_handle, status);


Kind regards,

Paul