Re: [PATCH v7 8/9] Bluetooth: qca: Set NVMEM BD address quirks when address is invalid
From: sashiko-bot@kernel.org
Date: 2026-07-02 16:01:29
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] NVMEM BD_ADDR retrieval does not reject the unprogrammed flash state (BDADDR_NONE / FF:FF:FF:FF:FF:FF).
- [Medium] is_configured() and get_missing_options() fail to consider HCI_QUIRK_USE_BDADDR_NVMEM when determining if a device is configured.
--
commit 9ecd7dcbdb800bbe76edbfcc8d48e97314e0cafb
Author: Loic Poulain [off-list ref]
Bluetooth: qca: Set NVMEM BD address quirks when address is invalid
When the controller BD address is invalid (zero or default),
set the NVMEM quirks to allow retrieving the address from a
'local-bd-address' NVMEM cell. The BD address is often stored
alongside the WiFi MAC address in big-endian format, so also
set the big-endian quirk.
quoted hunk ↗ jump to hunk
diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c index 04ebe290bc784..06cb787b0559f 100644 --- a/drivers/bluetooth/btqca.c +++ b/drivers/bluetooth/btqca.c@@ -723,8 +723,11 @@ static int qca_check_bdaddr(struct hci_dev *hdev, const struct qca_fw_config *co } bda = (struct hci_rp_read_bd_addr *)skb->data; - if (!bacmp(&bda->bdaddr, &config->bdaddr)) + if (!bacmp(&bda->bdaddr, &config->bdaddr)) { hci_set_quirk(hdev, HCI_QUIRK_USE_BDADDR_PROPERTY); + hci_set_quirk(hdev, HCI_QUIRK_USE_BDADDR_NVMEM);
[Severity: High]
Does this safely handle devices with an unprogrammed NVMEM cell?
If a device has an unprogrammed flash state (BDADDR_NONE or FF:FF:FF:FF:FF:FF),
of_get_nvmem_eui48() will retrieve it without rejecting it, because it only
checks for all-zeroes:
net/core/of_net.c:of_get_nvmem_eui48() {
if (len != ETH_ALEN || !memchr_inv(eui48, 0, ETH_ALEN)) {
kfree(eui48);
return -EINVAL;
}
...
Then hci_dev_get_bd_addr_from_nvmem() will assign this to hdev->public_addr
without checking against BDADDR_NONE:
net/bluetooth/hci_sync.c:hci_dev_get_bd_addr_from_nvmem() {
err = of_get_nvmem_eui48(np, "local-bd-address", ba);
if (err)
return err;
if (hci_test_quirk(hdev, HCI_QUIRK_BDADDR_NVMEM_BE))
baswap(&hdev->public_addr, (bdaddr_t *)ba);
else
bacpy(&hdev->public_addr, (bdaddr_t *)ba);
...
Could this result in multiple devices on the network being initialized with a
broadcast Bluetooth address instead of being marked as unconfigured?
[Severity: Medium]
Will this quirk be properly evaluated when determining if the device is
configured?
Currently, the core logic in net/bluetooth/mgmt.c:is_configured() only checks
for HCI_QUIRK_INVALID_BDADDR and HCI_QUIRK_USE_BDADDR_PROPERTY:
if ((hci_test_quirk(hdev, HCI_QUIRK_INVALID_BDADDR) ||
hci_test_quirk(hdev, HCI_QUIRK_USE_BDADDR_PROPERTY)) &&
!bacmp(&hdev->public_addr, BDADDR_ANY))
return false;
Similarly, net/bluetooth/mgmt.c:get_missing_options() omits
HCI_QUIRK_USE_BDADDR_NVMEM:
if ((hci_test_quirk(hdev, HCI_QUIRK_INVALID_BDADDR) ||
hci_test_quirk(hdev, HCI_QUIRK_USE_BDADDR_PROPERTY)) &&
!bacmp(&hdev->public_addr, BDADDR_ANY))
options |= MGMT_OPTION_PUBLIC_ADDRESS;
While the driver sets both HCI_QUIRK_USE_BDADDR_PROPERTY and
HCI_QUIRK_USE_BDADDR_NVMEM here, does the core logic need an update to handle
HCI_QUIRK_USE_BDADDR_NVMEM independently?
+ hci_set_quirk(hdev, HCI_QUIRK_BDADDR_NVMEM_BE); + } kfree_skb(skb);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260701-block-as-nvmem-v7-0-3fe8205ef0a8@oss.qualcomm.com?part=8