Re: [PATCH v1] Bluetooth: mgmt: Verify cmd pending status before removing it

[Paul Menzel]

Dear Jiayang,


Thank you for your patch.

Am 09.09.24 um 05:09 schrieb Jiayang Mao:
> From: jiaymao <quic_jiaymao@xxxxxxxxxxx>

Please use your full name:

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

> 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@xxxxxxxxxxx>

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