Re: [PATCH v1] Bluetooth: qca: Fix BT enable failure again for QCA6390 after warm reboot

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Zijun,

On Thu, May 2, 2024 at 10:06 AM Zijun Hu <quic_zijuhu@xxxxxxxxxxx> wrote:
>
> Commit 272970be3dab ("Bluetooth: hci_qca: Fix driver shutdown on closed
> serdev") will cause below regression issue:
>
> BT can't be enabled after below steps:
> cold boot -> enable BT -> disable BT -> warm reboot -> BT enable failure
> if property enable-gpios is not configured within DT|ACPI for QCA6390.
>
> The commit is to fix a use-after-free issue within qca_serdev_shutdown()
> during reboot, but also introduces this regression issue regarding above
> steps since the VSC is not sent to reset controller during warm reboot.
>
> Fixed by sending the VSC to reset controller within qca_serdev_shutdown()
> once BT was ever enabled, and the use-after-free issue is also be fixed
> by this change since serdev is still opened when send to serdev.
>
> Fixes: 272970be3dab ("Bluetooth: hci_qca: Fix driver shutdown on closed serdev")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Wren Turkal <wt@xxxxxxxxxxxxxxxx>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218726
> Signed-off-by: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>
> Tested-by: Wren Turkal <wt@xxxxxxxxxxxxxxxx>
> ---
>  drivers/bluetooth/hci_qca.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 0c9c9ee56592..8e35c9091486 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -2450,13 +2450,12 @@ static void qca_serdev_shutdown(struct device *dev)
>         struct qca_serdev *qcadev = serdev_device_get_drvdata(serdev);
>         struct hci_uart *hu = &qcadev->serdev_hu;
>         struct hci_dev *hdev = hu->hdev;
> -       struct qca_data *qca = hu->priv;
>         const u8 ibs_wake_cmd[] = { 0xFD };
>         const u8 edl_reset_soc_cmd[] = { 0x01, 0x00, 0xFC, 0x01, 0x05 };
>
>         if (qcadev->btsoc_type == QCA_QCA6390) {
> -               if (test_bit(QCA_BT_OFF, &qca->flags) ||
> -                   !test_bit(HCI_RUNNING, &hdev->flags))

This probably deserves a comment on why you end up with
HCI_QUIRK_NON_PERSISTENT_SETUP and HCI_SETUP flags here, also why you
are removing the flags above since that was introduce to prevent
use-after-free this sort of revert it so I do wonder how serdev can
still be open if you haven't tested for QCA_BT_OFF for example?

> +               if (test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks) ||
> +                   hci_dev_test_flag(hdev, HCI_SETUP))
>                         return;
>
>                 serdev_device_write_flush(serdev);
> --
> 2.7.4
>


-- 
Luiz Augusto von Dentz





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux