linux-kernel - Re: [PATCH v1] Bluetooth: hci_sync: clear cmd_sync_work

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABBYNZLY_PAA0jPiHwGKUmdd3SKqwViLSHAkNHH0=trdqrDRnQ@mail.gmail.com>
Date: Mon, 2 Dec 2024 15:41:28 -0500
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: Jiayang Mao <quic_jiaymao@...cinc.com>
Cc: Marcel Holtmann <marcel@...tmann.org>, Johan Hedberg <johan.hedberg@...il.com>, 
	linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org, 
	quic_chejiang@...cinc.com
Subject: Re: [PATCH v1] Bluetooth: hci_sync: clear cmd_sync_work_list when
 power off

Hi Jiayang,

On Mon, Nov 25, 2024 at 12:51 PM Jiayang Mao <quic_jiaymao@...cinc.com> wrote:
>
> Clear the remaining command in cmd_sync_work_list when BT is
> performing power off. In some cases, this list is not empty after
> power off. BT host will try to send more HCI commands.
> This can cause unexpected results.

What commands are in the queue?

> Signed-off-by: Jiayang Mao <quic_jiaymao@...cinc.com>
> ---
>  net/bluetooth/hci_sync.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index c86f4e42e..bc622d074 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -5139,6 +5139,7 @@ int hci_dev_close_sync(struct hci_dev *hdev)
>  {
>         bool auto_off;
>         int err = 0;
> +       struct hci_cmd_sync_work_entry *entry, *tmp;
>
>         bt_dev_dbg(hdev, "");
>
> @@ -5258,6 +5259,11 @@ int hci_dev_close_sync(struct hci_dev *hdev)
>         clear_bit(HCI_RUNNING, &hdev->flags);
>         hci_sock_dev_event(hdev, HCI_DEV_CLOSE);
>
> +       mutex_lock(&hdev->cmd_sync_work_lock);
> +       list_for_each_entry_safe(entry, tmp, &hdev->cmd_sync_work_list, list)
> +               _hci_cmd_sync_cancel_entry(hdev, entry, -ECANCELED);
> +       mutex_unlock(&hdev->cmd_sync_work_lock);

Seems equivalent to hci_cmd_sync_clear, that said we should have been
running with that lock already, also if there is a sequence like
close/open the close may cancel the subsequent open, so I don't think
we should be canceling every subsequent callback like this.

>         /* After this point our queues are empty and no tasks are scheduled. */
>         hdev->close(hdev);
>
> --
> 2.25.1
>


-- 
Luiz Augusto von Dentz