Skip to content

Commit

Permalink
Bluetooth: Fix not generating RPA when required
Browse files Browse the repository at this point in the history 
[ Upstream commit c45074d ]

Code was checking if random_addr and hdev->rpa match without first
checking if the RPA has not been set (BDADDR_ANY), furthermore it was
clearing HCI_RPA_EXPIRED before the command completes and the RPA is
actually programmed which in case of failure would leave the expired
RPA still set.

Since advertising instance have a similar problem the clearing of
HCI_RPA_EXPIRED has been moved to hci_event.c after checking the random
address is in fact the hdev->rap and then proceed to set the expire
timeout.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
@Vudentz @gregkh
Vudentz authored and gregkh committed Sep 18, 2021
1 parent f38dd8e commit 20b3c26
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 56 deletions.
4 changes: 4 additions & 0 deletions include/net/bluetooth/hci_core.h
View file
Expand Up @@ -1411,6 +1411,10 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
!hci_dev_test_flag(dev, HCI_AUTO_OFF))
#define bredr_sc_enabled(dev) (lmp_sc_capable(dev) && \
hci_dev_test_flag(dev, HCI_SC_ENABLED))
#define rpa_valid(dev) (bacmp(&dev->rpa, BDADDR_ANY) && \
!hci_dev_test_flag(dev, HCI_RPA_EXPIRED))
#define adv_rpa_valid(adv) (bacmp(&adv->random_addr, BDADDR_ANY) && \
!adv->rpa_expired)

#define scan_1m(dev) (((dev)->le_tx_def_phys & HCI_LE_SET_PHY_1M) || \
((dev)->le_rx_def_phys & HCI_LE_SET_PHY_1M))
Expand Down
32 changes: 23 additions & 9 deletions net/bluetooth/hci_event.c
View file
Expand Up @@ -40,6 +40,8 @@
#define ZERO_KEY "\x00\x00\x00\x00\x00\x00\x00\x00" \
"\x00\x00\x00\x00\x00\x00\x00\x00"

#define secs_to_jiffies(_secs) msecs_to_jiffies((_secs) * 1000)

/* Handle HCI Event packets */

static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb,
Expand Down Expand Up @@ -1171,6 +1173,12 @@ static void hci_cc_le_set_random_addr(struct hci_dev *hdev, struct sk_buff *skb)

bacpy(&hdev->random_addr, sent);

if (!bacmp(&hdev->rpa, sent)) {
hci_dev_clear_flag(hdev, HCI_RPA_EXPIRED);
queue_delayed_work(hdev->workqueue, &hdev->rpa_expired,
secs_to_jiffies(hdev->rpa_timeout));
}

hci_dev_unlock(hdev);
}

Expand Down Expand Up @@ -1201,24 +1209,30 @@ static void hci_cc_le_set_adv_set_random_addr(struct hci_dev *hdev,
{
__u8 status = *((__u8 *) skb->data);
struct hci_cp_le_set_adv_set_rand_addr *cp;
struct adv_info *adv_instance;
struct adv_info *adv;

if (status)
return;

cp = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_ADV_SET_RAND_ADDR);
if (!cp)
/* Update only in case the adv instance since handle 0x00 shall be using
* HCI_OP_LE_SET_RANDOM_ADDR since that allows both extended and
* non-extended adverting.
*/
if (!cp || !cp->handle)
return;

hci_dev_lock(hdev);

if (!cp->handle) {
/* Store in hdev for instance 0 (Set adv and Directed advs) */
bacpy(&hdev->random_addr, &cp->bdaddr);
} else {
adv_instance = hci_find_adv_instance(hdev, cp->handle);
if (adv_instance)
bacpy(&adv_instance->random_addr, &cp->bdaddr);
adv = hci_find_adv_instance(hdev, cp->handle);
if (adv) {
bacpy(&adv->random_addr, &cp->bdaddr);
if (!bacmp(&hdev->rpa, &cp->bdaddr)) {
adv->rpa_expired = false;
queue_delayed_work(hdev->workqueue,
&adv->rpa_expired_cb,
secs_to_jiffies(hdev->rpa_timeout));
}
}

hci_dev_unlock(hdev);
Expand Down
81 changes: 34 additions & 47 deletions net/bluetooth/hci_request.c
View file
Expand Up @@ -2053,8 +2053,6 @@ int hci_get_random_address(struct hci_dev *hdev, bool require_privacy,
* current RPA has expired then generate a new one.
*/
if (use_rpa) {
int to;

/* If Controller supports LL Privacy use own address type is
* 0x03
*/
Expand All @@ -2065,14 +2063,10 @@ int hci_get_random_address(struct hci_dev *hdev, bool require_privacy,
*own_addr_type = ADDR_LE_DEV_RANDOM;

if (adv_instance) {
if (!adv_instance->rpa_expired &&
!bacmp(&adv_instance->random_addr, &hdev->rpa))
if (adv_rpa_valid(adv_instance))
return 0;

adv_instance->rpa_expired = false;
} else {
if (!hci_dev_test_and_clear_flag(hdev, HCI_RPA_EXPIRED) &&
!bacmp(&hdev->random_addr, &hdev->rpa))
if (rpa_valid(hdev))
return 0;
}

Expand All @@ -2084,14 +2078,6 @@ int hci_get_random_address(struct hci_dev *hdev, bool require_privacy,

bacpy(rand_addr, &hdev->rpa);

to = msecs_to_jiffies(hdev->rpa_timeout * 1000);
if (adv_instance)
queue_delayed_work(hdev->workqueue,
&adv_instance->rpa_expired_cb, to);
else
queue_delayed_work(hdev->workqueue,
&hdev->rpa_expired, to);

return 0;
}

Expand Down Expand Up @@ -2134,6 +2120,30 @@ void __hci_req_clear_ext_adv_sets(struct hci_request *req)
hci_req_add(req, HCI_OP_LE_CLEAR_ADV_SETS, 0, NULL);
}

static void set_random_addr(struct hci_request *req, bdaddr_t *rpa)
{
struct hci_dev *hdev = req->hdev;

/* If we're advertising or initiating an LE connection we can't
* go ahead and change the random address at this time. This is
* because the eventual initiator address used for the
* subsequently created connection will be undefined (some
* controllers use the new address and others the one we had
* when the operation started).
*
* In this kind of scenario skip the update and let the random
* address be updated at the next cycle.
*/
if (hci_dev_test_flag(hdev, HCI_LE_ADV) ||
hci_lookup_le_connect(hdev)) {
bt_dev_dbg(hdev, "Deferring random address update");
hci_dev_set_flag(hdev, HCI_RPA_EXPIRED);
return;
}

hci_req_add(req, HCI_OP_LE_SET_RANDOM_ADDR, 6, rpa);
}

int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)
{
struct hci_cp_le_set_ext_adv_params cp;
Expand Down Expand Up @@ -2236,6 +2246,13 @@ int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)
} else {
if (!bacmp(&random_addr, &hdev->random_addr))
return 0;
/* Instance 0x00 doesn't have an adv_info, instead it
* uses hdev->random_addr to track its address so
* whenever it needs to be updated this also set the
* random address since hdev->random_addr is shared with
* scan state machine.
*/
set_random_addr(req, &random_addr);
}

memset(&cp, 0, sizeof(cp));
Expand Down Expand Up @@ -2493,30 +2510,6 @@ void hci_req_clear_adv_instance(struct hci_dev *hdev, struct sock *sk,
false);
}

static void set_random_addr(struct hci_request *req, bdaddr_t *rpa)
{
struct hci_dev *hdev = req->hdev;

/* If we're advertising or initiating an LE connection we can't
* go ahead and change the random address at this time. This is
* because the eventual initiator address used for the
* subsequently created connection will be undefined (some
* controllers use the new address and others the one we had
* when the operation started).
*
* In this kind of scenario skip the update and let the random
* address be updated at the next cycle.
*/
if (hci_dev_test_flag(hdev, HCI_LE_ADV) ||
hci_lookup_le_connect(hdev)) {
bt_dev_dbg(hdev, "Deferring random address update");
hci_dev_set_flag(hdev, HCI_RPA_EXPIRED);
return;
}

hci_req_add(req, HCI_OP_LE_SET_RANDOM_ADDR, 6, rpa);
}

int hci_update_random_address(struct hci_request *req, bool require_privacy,
bool use_rpa, u8 *own_addr_type)
{
Expand All @@ -2528,8 +2521,6 @@ int hci_update_random_address(struct hci_request *req, bool require_privacy,
* the current RPA in use, then generate a new one.
*/
if (use_rpa) {
int to;

/* If Controller supports LL Privacy use own address type is
* 0x03
*/
Expand All @@ -2539,8 +2530,7 @@ int hci_update_random_address(struct hci_request *req, bool require_privacy,
else
*own_addr_type = ADDR_LE_DEV_RANDOM;

if (!hci_dev_test_and_clear_flag(hdev, HCI_RPA_EXPIRED) &&
!bacmp(&hdev->random_addr, &hdev->rpa))
if (rpa_valid(hdev))
return 0;

err = smp_generate_rpa(hdev, hdev->irk, &hdev->rpa);
Expand All @@ -2551,9 +2541,6 @@ int hci_update_random_address(struct hci_request *req, bool require_privacy,

set_random_addr(req, &hdev->rpa);

to = msecs_to_jiffies(hdev->rpa_timeout * 1000);
queue_delayed_work(hdev->workqueue, &hdev->rpa_expired, to);

return 0;
}

Expand Down

0 comments on commit 20b3c26

Please sign in to comment.