| 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: Fri, 18 Jan 2019 13:38:44 -0800
From: Matthias Kaehlcke <mka@...omium.org>
To: Marcel Holtmann <marcel@...tmann.org>,
Johan Hedberg <johan.hedberg@...il.com>,
"David S . Miller" <davem@...emloft.net>,
Loic Poulain <loic.poulain@...aro.org>
Cc: linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org,
Balakrishna Godavarthi <bgodavar@...eaurora.org>,
Brian Norris <briannorris@...omium.org>,
Dmitry Grinberg <dmitrygr@...gle.com>
Subject: Re: [PATCH v3 1/3] Bluetooth: Add quirk for reading BD_ADDR from
fwnode property
On Fri, Dec 28, 2018 at 02:05:44PM -0800, Matthias Kaehlcke wrote:
> Add HCI_QUIRK_USE_BDADDR_PROPERTY to allow controllers to retrieve
> the public Bluetooth address from the firmware node property
> 'local-bd-address'. If quirk is set and the property does not exist
> or is invalid the controller is marked as unconfigured.
>
> Signed-off-by: Matthias Kaehlcke <mka@...omium.org>
> Reviewed-by: Balakrishna Godavarthi <bgodavar@...eaurora.org>
> Tested-by: Balakrishna Godavarthi <bgodavar@...eaurora.org>
> ---
> Changes in v3:
> - return no value from hci_dev_get_bd_addr_from_property() since
> currently nobody uses it anyway
> - use bacpy() in hci_dev_get_bd_addr_from_property()
> - return -EADDRNOTAVAIL if no BD_ADDR is configured or if the driver
> has no ->set_bdaddr
>
> Changes in v2:
> - added check for return value of ->setup()
> - only read BD_ADDR from the property if it isn't assigned yet. This
> is needed to support configuration from user space
> - refactored the branch of the new quirk to get rid of 'bd_addr_set'
> - added 'Reviewed-by: Balakrishna Godavarthi <bgodavar@...eaurora.org>' tag
> ---
> include/net/bluetooth/hci.h | 12 +++++++++++
> net/bluetooth/hci_core.c | 43 +++++++++++++++++++++++++++++++++++++
> net/bluetooth/mgmt.c | 6 ++++--
> 3 files changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index c36dc1e20556a..fbba43e9bef5b 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -158,6 +158,18 @@ enum {
> */
> HCI_QUIRK_INVALID_BDADDR,
>
> + /* When this quirk is set, the public Bluetooth address
> + * initially reported by HCI Read BD Address command
> + * is considered invalid. The public BD Address can be
> + * specified in the fwnode property 'local-bd-address'.
> + * If this property does not exist or is invalid controller
> + * configuration is required before this device can be used.
> + *
> + * This quirk can be set before hci_register_dev is called or
> + * during the hdev->setup vendor callback.
> + */
> + HCI_QUIRK_USE_BDADDR_PROPERTY,
> +
> /* When this quirk is set, the duplicate filtering during
> * scanning is based on Bluetooth devices addresses. To allow
> * RSSI based updates, restart scanning if needed.
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 7352fe85674be..3ccfa351b87bb 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -30,6 +30,7 @@
> #include <linux/rfkill.h>
> #include <linux/debugfs.h>
> #include <linux/crypto.h>
> +#include <linux/property.h>
> #include <asm/unaligned.h>
>
> #include <net/bluetooth/bluetooth.h>
> @@ -1355,6 +1356,32 @@ int hci_inquiry(void __user *arg)
> return err;
> }
>
> +/**
> + * hci_dev_get_bd_addr_from_property - Get the Bluetooth Device Address
> + * (BD_ADDR) for a HCI device from
> + * a firmware node property.
> + * @hdev: The HCI device
> + *
> + * Search the firmware node for 'local-bd-address'.
> + *
> + * All-zero BD addresses are rejected, because those could be properties
> + * that exist in the firmware tables, but were not updated by the firmware. For
> + * example, the DTS could define 'local-bd-address', with zero BD addresses.
> + */
> +static void hci_dev_get_bd_addr_from_property(struct hci_dev *hdev)
> +{
> + struct fwnode_handle *fwnode = dev_fwnode(hdev->dev.parent);
> + bdaddr_t ba;
> + int ret;
> +
> + ret = fwnode_property_read_u8_array(fwnode, "local-bd-address",
> + (u8 *)&ba, sizeof(ba));
> + if (ret < 0 || !bacmp(&ba, BDADDR_ANY))
> + return;
> +
> + bacpy(&hdev->public_addr, &ba);
> +}
> +
> static int hci_dev_do_open(struct hci_dev *hdev)
> {
> int ret = 0;
> @@ -1422,6 +1449,22 @@ static int hci_dev_do_open(struct hci_dev *hdev)
> if (hdev->setup)
> ret = hdev->setup(hdev);
>
> + if (ret)
> + goto setup_failed;
> +
> + if (test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks)) {
> + if (!bacmp(&hdev->public_addr, BDADDR_ANY))
> + hci_dev_get_bd_addr_from_property(hdev);
> +
> + if (bacmp(&hdev->public_addr, BDADDR_ANY) &&
> + hdev->set_bdaddr)
> + ret = hdev->set_bdaddr(hdev,
> + &hdev->public_addr);
> + else
> + ret = -EADDRNOTAVAIL;
> + }
> +
> +setup_failed:
> /* The transport driver can set these quirks before
> * creating the HCI device or in its setup callback.
> *
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index ccce954f81468..fae84353d030f 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -551,7 +551,8 @@ static bool is_configured(struct hci_dev *hdev)
> !hci_dev_test_flag(hdev, HCI_EXT_CONFIGURED))
> return false;
>
> - if (test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks) &&
> + if ((test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks) ||
> + test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks)) &&
> !bacmp(&hdev->public_addr, BDADDR_ANY))
> return false;
>
> @@ -566,7 +567,8 @@ static __le32 get_missing_options(struct hci_dev *hdev)
> !hci_dev_test_flag(hdev, HCI_EXT_CONFIGURED))
> options |= MGMT_OPTION_EXTERNAL_CONFIG;
>
> - if (test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks) &&
> + if ((test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks) ||
> + test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks)) &&
> !bacmp(&hdev->public_addr, BDADDR_ANY))
> options |= MGMT_OPTION_PUBLIC_ADDRESS;
>
Marcel (et al), do you have any more comments on this series or
is it ready to land?
Thanks
Matthias
Powered by blists - more mailing lists