linux-kernel - Re: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk

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: <CAMi1Hd3fe=wk02WG8J7K5Ud1GcWkuKKKrxFjkNguxDkzNz2WVQ@mail.gmail.com>
Date:   Fri, 7 Jul 2023 15:11:11 +0530
From:   Amit Pundir <amit.pundir@...aro.org>
To:     Johan Hovold <johan+linaro@...nel.org>
Cc:     Marcel Holtmann <marcel@...tmann.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Luiz Augusto von Dentz <luiz.dentz@...il.com>,
        Matthias Kaehlcke <mka@...omium.org>,
        linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org,
        Linux regressions mailing list <regressions@...ts.linux.dev>
Subject: Re: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk

Hi Johan,

On Wed, 31 May 2023 at 14:35, Johan Hovold <johan+linaro@...nel.org> wrote:
>
> Devices that lack persistent storage for the device address can indicate
> this by setting the HCI_QUIRK_INVALID_BDADDR which causes the controller
> to be marked as unconfigured until user space has set a valid address.
>
> The related HCI_QUIRK_USE_BDADDR_PROPERTY was later added to similarly
> indicate that the device lacks a valid address but that one may be
> specified in the devicetree.
>
> As is clear from commit 7a0e5b15ca45 ("Bluetooth: Add quirk for reading
> BD_ADDR from fwnode property") that added and documented this quirk and
> commits like de79a9df1692 ("Bluetooth: btqcomsmd: use
> HCI_QUIRK_USE_BDADDR_PROPERTY"), the device address of controllers with
> this flag should be treated as invalid until user space has had a chance
> to configure the controller in case the devicetree property is missing.
>
> As it does not make sense to allow controllers with invalid addresses,
> restore the original semantics, which also makes sure that the
> implementation is consistent (e.g. get_missing_options() indicates that
> the address must be set) and matches the documentation (including
> comments in the code, such as, "In case any of them is set, the
> controller has to start up as unconfigured.").
>

This patch broke Bluetooth on Dragonboard 845c (SDM845) devboard.
Reverting this patch fixes the BT breakage and I see the following
messages in dmesg:

Bluetooth: hci0: setting up wcn399x
Bluetooth: hci0: QCA Product ID   :0x0000000a
Bluetooth: hci0: QCA SOC Version  :0x40010214
Bluetooth: hci0: QCA ROM Version  :0x00000201
Bluetooth: hci0: QCA Patch Version:0x00000001
Bluetooth: hci0: QCA controller version 0x02140201
Bluetooth: hci0: QCA Downloading qca/crbtfw21.tlv
Bluetooth: hci0: QCA Downloading qca/crnv21.bin
Bluetooth: hci0: QCA setup on UART is completed

Regards,
Amit Pundir

> Fixes: e668eb1e1578 ("Bluetooth: hci_core: Don't stop BT if the BD address missing in dts")
> Signed-off-by: Johan Hovold <johan+linaro@...nel.org>
> ---
>  net/bluetooth/hci_sync.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index c2a805ee55cc..ce03038b3460 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -4615,16 +4615,14 @@ static int hci_dev_setup_sync(struct hci_dev *hdev)
>          * BD_ADDR invalid before creating the HCI device or in
>          * its setup callback.
>          */
> -       invalid_bdaddr = test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
> -
> +       invalid_bdaddr = test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks) ||
> +                        test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
>         if (!ret) {
>                 if (test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks) &&
>                     !bacmp(&hdev->public_addr, BDADDR_ANY))
>                         hci_dev_get_bd_addr_from_property(hdev);
>
> -               if ((invalid_bdaddr ||
> -                    test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks)) &&
> -                   bacmp(&hdev->public_addr, BDADDR_ANY) &&
> +               if (invalid_bdaddr && bacmp(&hdev->public_addr, BDADDR_ANY) &&
>                     hdev->set_bdaddr) {
>                         ret = hdev->set_bdaddr(hdev, &hdev->public_addr);
>                         if (!ret)
> --
> 2.39.3
>