| 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
| ||
|
Date: Mon, 15 Apr 2024 16:22:51 +0530
From: Janaki Ramaiah Thota <quic_janathot@...cinc.com>
To: Johan Hovold <johan@...nel.org>
CC: Johan Hovold <johan+linaro@...nel.org>, <luiz.dentz@...il.com>,
<marcel@...tmann.org>, <linux-bluetooth@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <stable@...r.kernel.org>,
<quic_mohamull@...cinc.com>, <quic_hbandi@...cinc.com>
Subject: Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode
exists in DT"
Hi Johan,
Are you planing to merge your below patch ?
On 4/5/2024 6:29 PM, Janaki Ramaiah Thota wrote:
>
>
> On 4/3/2024 5:54 PM, Johan Hovold wrote:
>> On Fri, Mar 29, 2024 at 12:55:40PM +0530, Janaki Ramaiah Thota wrote:
>>> On 3/28/2024 8:53 PM, Johan Hovold wrote:
>>>> On Thu, Mar 28, 2024 at 08:25:16PM +0530, Janaki Ramaiah Thota wrote:
>>
>>>>> We made this change to configure the device which supports persistent
>>>>> memory for the BD-Address
>>>>
>>>> Can you say something more about which devices support persistent
>>>> storage for the address? Is that all or just some of the chip variants?
>>
>>> Most of the devices support persistent storage, and bd-address storage
>>> is chosen based on the OEM and Target.
>>
>> Can you be more specific about which devices support it (or say which do
>> not)?
>>
>
> We know below BT chipsets supports persistent storage(OTP) for BDA
> WCN7850, WCN6855, WCN6750
>
>> Is the address stored in some external eeprom or similar which the OEM
>> can choose to populate?
>>
>
> This persistent storage is One Time Programmable (OTP) reserved memory
> which resides in the BT chip.
>
>>>>> So to make device functional in both scenarios we are adding a new
>>>>> property in dts file to distinguish persistent and non-persistent
>>>>> support of BD Address and set HCI_QUIRK_USE_BDADDR_PROPERTY bit
>>>>> accordingly
>>>>
>>>> Depending on the answer to my questions above, you may be able to infer
>>>> this from the compatible string and/or you can read out the address from
>>>> the device and only set the quirk if it's set to the default address.
>>>>
>>>> You should not need to add a new property for this.
>>
>>> As per my understanding, altering the compatible string may cause duplicate
>>> configuration, right ?
>>
>
> Yes, we are correct.
>
>> If it's the same device and just a different configuration then we can't
>> use the compatible string for this.
>>
>> It seems we need a patch like the below to address this. But please
>> provide some more details (e.g. answers to the questions above) so I can
>> determine what the end result should look like.
>>
>> Johan
>>
>>
>> From 9719effe80fcc17518131816fdfeb1824cfa08b6 Mon Sep 17 00:00:00 2001
>> From: Johan Hovold <johan+linaro@...nel.org>
>> Date: Thu, 20 Apr 2023 14:10:55 +0200
>> Subject: [PATCH] Bluetooth: btqca: add invalid device address check
>>
>> Some Qualcomm Bluetooth controllers lack persistent storage for the
>> device address and therefore end up using the default address
>> 00:00:00:00:5a:ad.
>>
>> Apparently this depends on how the controller has been integrated so we
>> can not use the device type alone to determine when the address is
>> valid.
>>
>> Instead read back the address during setup() and only set the
>> HCI_QUIRK_USE_BDADDR_PROPERTY flag when needed.
>>
>> Fixes: de79a9df1692 ("Bluetooth: btqcomsmd: use HCI_QUIRK_USE_BDADDR_PROPERTY")
>> Fixes: e668eb1e1578 ("Bluetooth: hci_core: Don't stop BT if the BD address missing in dts")
>> Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
>> Cc: stable@...r.kernel.org # 6.5
>> Signed-off-by: Johan Hovold <johan+linaro@...nel.org>
>> ---
>> drivers/bluetooth/btqca.c | 33 +++++++++++++++++++++++++++++++++
>> drivers/bluetooth/hci_qca.c | 2 --
>> 2 files changed, 33 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
>> index 19cfc342fc7b..15124157372c 100644
>> --- a/drivers/bluetooth/btqca.c
>> +++ b/drivers/bluetooth/btqca.c
>> @@ -15,6 +15,8 @@
>> #define VERSION "0.1"
>> +#define QCA_BDADDR_DEFAULT (&(bdaddr_t) {{ 0xad, 0x5a, 0x00, 0x00, 0x00, 0x00 }})
>> +
>> int qca_read_soc_version(struct hci_dev *hdev, struct qca_btsoc_version *ver,
>> enum qca_btsoc_type soc_type)
>> {
>> @@ -612,6 +614,35 @@ int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr)
>> }
>> EXPORT_SYMBOL_GPL(qca_set_bdaddr_rome);
>> +static void qca_check_bdaddr(struct hci_dev *hdev)
>> +{
>> + struct hci_rp_read_bd_addr *bda;
>> + struct sk_buff *skb;
>> + int err;
>> +
>> + if (bacmp(&hdev->public_addr, BDADDR_ANY))
>> + return;
>> +
>> + skb = __hci_cmd_sync(hdev, HCI_OP_READ_BD_ADDR, 0, NULL,
>> + HCI_INIT_TIMEOUT);
>> + if (IS_ERR(skb)) {
>> + err = PTR_ERR(skb);
>> + bt_dev_err(hdev, "Failed to read device address (%d)", err);
>> + return;
>> + }
>> +
>> + if (skb->len != sizeof(*bda)) {
>> + bt_dev_err(hdev, "Device address length mismatch");
>> + goto free_skb;
>> + }
>> +
>> + bda = (struct hci_rp_read_bd_addr *)skb->data;
>> + if (!bacmp(&bda->bdaddr, QCA_BDADDR_DEFAULT))
>> + set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
>> +free_skb:
>> + kfree_skb(skb);
>> +}
>> +
>> static void qca_generate_hsp_nvm_name(char *fwname, size_t max_size,
>> struct qca_btsoc_version ver, u8 rom_ver, u16 bid)
>> {
>> @@ -818,6 +849,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>> break;
>> }
>> + qca_check_bdaddr(hdev);
>> +
>> bt_dev_info(hdev, "QCA setup on UART is completed");
>> return 0;
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index b266db18c8cc..b621a0a40ea4 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -1908,8 +1908,6 @@ static int qca_setup(struct hci_uart *hu)
>> case QCA_WCN6750:
>> case QCA_WCN6855:
>> case QCA_WCN7850:
>> - set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
>> -
>> qcadev = serdev_device_get_drvdata(hu->serdev);
>> if (qcadev->bdaddr_property_broken)
>> set_bit(HCI_QUIRK_BDADDR_PROPERTY_BROKEN, &hdev->quirks);
>
> Thanks for the patch. This change looks fine and it will resolve the current OTP issue.
>
> --
> Thanks,
> JanakiRam
Regards,
Janaki Ram
Powered by blists - more mailing lists