From e6f8080e1cd4968ffa728d9cab8fc466abca2e4d Mon Sep 17 00:00:00 2001 From: Olivier Lesage Date: Tue, 8 Jul 2025 10:43:32 +0200 Subject: [PATCH 1/4] [nrf fromtree] bluetooth: host: Fix uninitialized own_addr_type for legacy scan+adv In 25c993e5b74562137c2951db01c8a8d200e98d3d a new case was introduced where own_addr_type is not set by bt_id_set_scan_own_addr properly. This led to issues for users where increasing their zephyr version led to failures to start scanning after advertising in the case where CONFIG_BT_SCAN_WITH_IDENTITY=n and legacy advertising commands are used. Signed-off-by: Olivier Lesage (cherry picked from commit 43223a166118007f9658e1763502e9abe35e5928) --- subsys/bluetooth/host/id.c | 54 ++++++++++--- .../id/bt_id_set_scan_own_addr/src/main.c | 77 +++++++++++++++++++ .../id/bt_id_set_scan_own_addr/testcase.yaml | 4 + 3 files changed, 123 insertions(+), 12 deletions(-) diff --git a/subsys/bluetooth/host/id.c b/subsys/bluetooth/host/id.c index 7ecd9f98f74..475803e7f18 100644 --- a/subsys/bluetooth/host/id.c +++ b/subsys/bluetooth/host/id.c @@ -1965,7 +1965,7 @@ int bt_id_set_create_conn_own_addr(bool use_filter, uint8_t *own_addr_type) #endif /* defined(CONFIG_BT_CENTRAL) */ #if defined(CONFIG_BT_OBSERVER) -static bool is_adv_using_rand_addr(void) +static bool is_legacy_adv_enabled(void) { struct bt_le_ext_adv *adv; @@ -1981,7 +1981,27 @@ static bool is_adv_using_rand_addr(void) adv = bt_le_adv_lookup_legacy(); - return adv && atomic_test_bit(adv->flags, BT_ADV_ENABLED); + return adv != NULL && atomic_test_bit(adv->flags, BT_ADV_ENABLED); +} + +static bool is_legacy_adv_using_id_addr(void) +{ + struct bt_le_ext_adv *adv; + + if (!IS_ENABLED(CONFIG_BT_BROADCASTER) || + (IS_ENABLED(CONFIG_BT_EXT_ADV) && + BT_DEV_FEAT_LE_EXT_ADV(bt_dev.le.features))) { + /* When advertising is not enabled or is using extended + * advertising HCI commands then only the scanner uses the set + * random address command. + */ + return false; + } + + adv = bt_le_adv_lookup_legacy(); + + return adv != NULL && atomic_test_bit(adv->flags, BT_ADV_ENABLED) + && atomic_test_bit(adv->flags, BT_ADV_USE_IDENTITY); } int bt_id_set_scan_own_addr(bool active_scan, uint8_t *own_addr_type) @@ -2014,20 +2034,30 @@ int bt_id_set_scan_own_addr(bool active_scan, uint8_t *own_addr_type) * (through Kconfig). * Use same RPA as legacy advertiser if advertising. */ - if (!IS_ENABLED(CONFIG_BT_SCAN_WITH_IDENTITY) && - !is_adv_using_rand_addr()) { - err = bt_id_set_private_addr(BT_ID_DEFAULT); - if (err) { - if (active_scan || !is_adv_using_rand_addr()) { + if (!IS_ENABLED(CONFIG_BT_SCAN_WITH_IDENTITY)) { + /* When using legacy advertising commands, the scanner and advertiser + * share the same address, so we cannot change it. + * When using extended advertising commands, however, the advertising + * sets have their own addresses, so we can always change the scanner + * address here. + */ + if (is_legacy_adv_using_id_addr()) { + if (bt_dev.id_addr[BT_ID_DEFAULT].type == BT_ADDR_LE_RANDOM) { + *own_addr_type = BT_HCI_OWN_ADDR_RANDOM; + } else { + *own_addr_type = BT_HCI_OWN_ADDR_PUBLIC; + } + } else if (is_legacy_adv_enabled()) { + *own_addr_type = BT_HCI_OWN_ADDR_RANDOM; + } else { + err = bt_id_set_private_addr(BT_ID_DEFAULT); + if (err) { return err; } - LOG_WRN("Ignoring failure to set address for passive scan (%d)", - err); + *own_addr_type = BT_HCI_OWN_ADDR_RANDOM; } - - *own_addr_type = BT_HCI_OWN_ADDR_RANDOM; - } else if (IS_ENABLED(CONFIG_BT_SCAN_WITH_IDENTITY)) { + } else { if (bt_dev.id_addr[BT_ID_DEFAULT].type == BT_ADDR_LE_RANDOM) { /* If scanning with Identity Address we must set the * random identity address for both active and passive diff --git a/tests/bluetooth/host/id/bt_id_set_scan_own_addr/src/main.c b/tests/bluetooth/host/id/bt_id_set_scan_own_addr/src/main.c index c39411897cd..2c1fbc675d7 100644 --- a/tests/bluetooth/host/id/bt_id_set_scan_own_addr/src/main.c +++ b/tests/bluetooth/host/id/bt_id_set_scan_own_addr/src/main.c @@ -7,6 +7,8 @@ #include "mocks/crypto.h" #include "mocks/hci_core.h" #include "mocks/rpa.h" +#include "mocks/adv.h" +#include "mocks/adv_expects.h" #include "testing_common_defs.h" #include @@ -75,6 +77,81 @@ ZTEST(bt_id_set_scan_own_addr, test_set_nrpa_scan_address_no_privacy) "Address type reference was incorrectly set"); } +/* + * Test setting scan own address while 'CONFIG_BT_PRIVACY' isn't enabled. + * Advertising is ongoing and uses a random device address. + * + * Constraints: + * - bt_id_set_private_addr() succeeds and returns 0 + * - 'CONFIG_BT_SCAN_WITH_IDENTITY' isn't enabled + * - 'CONFIG_BT_PRIVACY' isn't enabled + * + * Expected behaviour: + * - bt_id_set_scan_own_addr() returns 0 + * - Address type reference is updated + */ +ZTEST(bt_id_set_scan_own_addr, test_set_nrpa_scan_address_no_privacy_adv_ongoing_random_identity) +{ + int err; + struct bt_le_ext_adv *adv = &bt_dev.adv; + uint8_t own_addr_type = BT_ADDR_LE_ANONYMOUS; + + Z_TEST_SKIP_IFDEF(CONFIG_BT_PRIVACY); + Z_TEST_SKIP_IFDEF(CONFIG_BT_SCAN_WITH_IDENTITY); + Z_TEST_SKIP_IFNDEF(CONFIG_BT_BROADCASTER); + + bt_rand_fake.custom_fake = bt_rand_custom_fake; + bt_le_adv_lookup_legacy_fake.return_val = adv; + + bt_addr_le_copy(&bt_dev.id_addr[BT_ID_DEFAULT], BT_STATIC_RANDOM_LE_ADDR_1); + + atomic_set_bit(adv->flags, BT_ADV_ENABLED); + + err = bt_id_set_scan_own_addr(false, &own_addr_type); + + zassert_ok(err, "Unexpected error code '%d' was returned", err); + zassert_true(own_addr_type == BT_HCI_OWN_ADDR_RANDOM, + "Address type reference was incorrectly set"); +} + +/* + * Test setting scan own address while 'CONFIG_BT_PRIVACY' isn't enabled. + * Advertising is ongoing and uses a public device address. + * + * Constraints: + * - bt_id_set_private_addr() succeeds and returns 0 + * - 'CONFIG_BT_SCAN_WITH_IDENTITY' isn't enabled + * - 'CONFIG_BT_PRIVACY' isn't enabled + * + * Expected behaviour: + * - bt_id_set_scan_own_addr() returns 0 + * - Address type reference is updated + */ +ZTEST(bt_id_set_scan_own_addr, test_set_nrpa_scan_address_no_privacy_adv_ongoing_public_identity) +{ + int err; + struct bt_le_ext_adv *adv = &bt_dev.adv; + uint8_t own_addr_type = BT_ADDR_LE_ANONYMOUS; + + Z_TEST_SKIP_IFDEF(CONFIG_BT_PRIVACY); + Z_TEST_SKIP_IFDEF(CONFIG_BT_SCAN_WITH_IDENTITY); + Z_TEST_SKIP_IFNDEF(CONFIG_BT_BROADCASTER); + + bt_rand_fake.custom_fake = bt_rand_custom_fake; + bt_le_adv_lookup_legacy_fake.return_val = adv; + + bt_addr_le_copy(&bt_dev.id_addr[BT_ID_DEFAULT], BT_LE_ADDR); + + atomic_set_bit(adv->flags, BT_ADV_ENABLED); + atomic_set_bit(adv->flags, BT_ADV_USE_IDENTITY); + + err = bt_id_set_scan_own_addr(false, &own_addr_type); + + zassert_ok(err, "Unexpected error code '%d' was returned", err); + zassert_true(own_addr_type == BT_HCI_OWN_ADDR_PUBLIC, + "Address type reference was incorrectly set"); +} + /* * Test setting scan own address while 'CONFIG_BT_PRIVACY' isn't enabled. * If 'CONFIG_BT_SCAN_WITH_IDENTITY' is enabled and the default identity has an RPA address of type diff --git a/tests/bluetooth/host/id/bt_id_set_scan_own_addr/testcase.yaml b/tests/bluetooth/host/id/bt_id_set_scan_own_addr/testcase.yaml index 4198d2bf91b..7e3cab5ce37 100644 --- a/tests/bluetooth/host/id/bt_id_set_scan_own_addr/testcase.yaml +++ b/tests/bluetooth/host/id/bt_id_set_scan_own_addr/testcase.yaml @@ -20,3 +20,7 @@ tests: - CONFIG_BT_SCAN_WITH_IDENTITY=y - CONFIG_BT_SMP=y - CONFIG_BT_PRIVACY=y + bluetooth.host.bt_id_set_scan_own_addr.scan_while_advertising: + type: unit + extra_configs: + - CONFIG_BT_BROADCASTER=y From a110b60ed830067d9acf728b8afd865ccb2b2b58 Mon Sep 17 00:00:00 2001 From: Olivier Lesage Date: Tue, 17 Jun 2025 13:08:46 +0200 Subject: [PATCH 2/4] [nrf fromtree] bluetooth: host: rename bool scan_enabled -> scan_disabled It represents whether scanning was disabled as part of this flow. Signed-off-by: Olivier Lesage (cherry picked from commit 11782dbbc032a7516125c5a797e396b9765689e6) --- subsys/bluetooth/host/id.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/subsys/bluetooth/host/id.c b/subsys/bluetooth/host/id.c index 475803e7f18..abbe693b25e 100644 --- a/subsys/bluetooth/host/id.c +++ b/subsys/bluetooth/host/id.c @@ -2173,13 +2173,13 @@ int bt_id_set_adv_own_addr(struct bt_le_ext_adv *adv, uint32_t options, * problem. */ #if defined(CONFIG_BT_OBSERVER) - bool scan_enabled = false; + bool scan_disabled = false; /* If active scan with NRPA is ongoing refresh NRPA */ if (!IS_ENABLED(CONFIG_BT_PRIVACY) && !IS_ENABLED(CONFIG_BT_SCAN_WITH_IDENTITY) && atomic_test_bit(bt_dev.flags, BT_DEV_SCANNING)) { - scan_enabled = true; + scan_disabled = true; bt_le_scan_set_enable(BT_HCI_LE_SCAN_DISABLE); } #endif /* defined(CONFIG_BT_OBSERVER) */ @@ -2187,7 +2187,7 @@ int bt_id_set_adv_own_addr(struct bt_le_ext_adv *adv, uint32_t options, *own_addr_type = BT_HCI_OWN_ADDR_RANDOM; #if defined(CONFIG_BT_OBSERVER) - if (scan_enabled) { + if (scan_disabled) { bt_le_scan_set_enable(BT_HCI_LE_SCAN_ENABLE); } #endif /* defined(CONFIG_BT_OBSERVER) */ From 3883759864cd051250bd3210e34e7f5394298e2a Mon Sep 17 00:00:00 2001 From: Olivier Lesage Date: Wed, 2 Jul 2025 08:52:33 +0200 Subject: [PATCH 3/4] [nrf fromtree] bluetooth: host: Do not try to set NRPA when scanning with identity Attempting this would fail (assuming the controller is implemented correctly) because when using legacy commands it is not allowed to change the device address while scanning. It also did not make sense. If we have configured the scanner to use the identity address as own_addr, because the advertiser and scanner addresses are shared when using legacy commands, setting the adv NRPA here would overwrite the identity address used by the scanner, which I assume is not the intention. Signed-off-by: Olivier Lesage (cherry picked from commit ef7ede64cce712b7cae1e8dde0d078b9cf55c3ca) --- subsys/bluetooth/host/id.c | 26 ++++- .../host/id/bt_id_set_adv_own_addr/src/main.c | 102 ++++++++++++++++++ .../id/bt_id_set_adv_own_addr/testcase.yaml | 4 + 3 files changed, 127 insertions(+), 5 deletions(-) diff --git a/subsys/bluetooth/host/id.c b/subsys/bluetooth/host/id.c index abbe693b25e..bf79215c450 100644 --- a/subsys/bluetooth/host/id.c +++ b/subsys/bluetooth/host/id.c @@ -2174,22 +2174,38 @@ int bt_id_set_adv_own_addr(struct bt_le_ext_adv *adv, uint32_t options, */ #if defined(CONFIG_BT_OBSERVER) bool scan_disabled = false; + bool dev_scanning = atomic_test_bit(bt_dev.flags, + BT_DEV_SCANNING); /* If active scan with NRPA is ongoing refresh NRPA */ if (!IS_ENABLED(CONFIG_BT_PRIVACY) && !IS_ENABLED(CONFIG_BT_SCAN_WITH_IDENTITY) && - atomic_test_bit(bt_dev.flags, BT_DEV_SCANNING)) { + dev_scanning) { scan_disabled = true; bt_le_scan_set_enable(BT_HCI_LE_SCAN_DISABLE); } -#endif /* defined(CONFIG_BT_OBSERVER) */ - err = bt_id_set_adv_private_addr(adv); - *own_addr_type = BT_HCI_OWN_ADDR_RANDOM; -#if defined(CONFIG_BT_OBSERVER) + /* If we are scanning with the identity address, it does + * not make sense to set an NRPA. + */ + if (!IS_ENABLED(CONFIG_BT_SCAN_WITH_IDENTITY) || + !dev_scanning) { + err = bt_id_set_adv_private_addr(adv); + *own_addr_type = BT_HCI_OWN_ADDR_RANDOM; + } else { + if (id_addr->type == BT_ADDR_LE_RANDOM) { + *own_addr_type = BT_HCI_OWN_ADDR_RANDOM; + } else if (id_addr->type == BT_ADDR_LE_PUBLIC) { + *own_addr_type = BT_HCI_OWN_ADDR_PUBLIC; + } + } + if (scan_disabled) { bt_le_scan_set_enable(BT_HCI_LE_SCAN_ENABLE); } +#else + err = bt_id_set_adv_private_addr(adv); + *own_addr_type = BT_HCI_OWN_ADDR_RANDOM; #endif /* defined(CONFIG_BT_OBSERVER) */ } else { err = bt_id_set_adv_private_addr(adv); diff --git a/tests/bluetooth/host/id/bt_id_set_adv_own_addr/src/main.c b/tests/bluetooth/host/id/bt_id_set_adv_own_addr/src/main.c index 29c500e9840..8304906d37e 100644 --- a/tests/bluetooth/host/id/bt_id_set_adv_own_addr/src/main.c +++ b/tests/bluetooth/host/id/bt_id_set_adv_own_addr/src/main.c @@ -7,6 +7,10 @@ #include "mocks/crypto.h" #include "mocks/scan.h" #include "mocks/scan_expects.h" +#include "mocks/hci_core.h" +#include "mocks/hci_core_expects.h" +#include "mocks/net_buf.h" +#include "mocks/net_buf_expects.h" #include "testing_common_defs.h" #include @@ -24,6 +28,7 @@ static void fff_reset_rule_before(const struct ztest_unit_test *test, void *fixt memset(&bt_dev, 0x00, sizeof(struct bt_dev)); CRYPTO_FFF_FAKES_LIST(RESET_FAKE); + HCI_CORE_FFF_FAKES_LIST(RESET_FAKE); } ZTEST_RULE(fff_reset_rule, fff_reset_rule_before, NULL); @@ -232,6 +237,7 @@ ZTEST(bt_id_set_adv_own_addr, test_observer_scanning_re_enabled_after_updating_a Z_TEST_SKIP_IFDEF(CONFIG_BT_PRIVACY); Z_TEST_SKIP_IFDEF(CONFIG_BT_EXT_ADV); + Z_TEST_SKIP_IFDEF(CONFIG_BT_SCAN_WITH_IDENTITY); Z_TEST_SKIP_IFNDEF(CONFIG_BT_OBSERVER); options &= ~BT_LE_ADV_OPT_CONN; @@ -241,6 +247,102 @@ ZTEST(bt_id_set_adv_own_addr, test_observer_scanning_re_enabled_after_updating_a atomic_set_bit(bt_dev.flags, BT_DEV_SCANNING); bt_id_set_adv_own_addr(&adv, options, true, &own_addr_type); + zassert_true(own_addr_type == BT_HCI_OWN_ADDR_RANDOM, + "Address type reference was incorrectly set"); expect_call_count_bt_le_scan_set_enable(2, expected_args_history); } + +/* + * Test setting the advertiser address while 'CONFIG_BT_SCAN_WITH_IDENTITY' is enabled + * and scanning is ongoing. The scanner is using a random identity address. + * + * Constraints: + * - Options 'BT_LE_ADV_OPT_CONN' bit isn't set + * + * Expected behaviour: + * - Scanning is not disabled. + * - The advertiser doesn't attempt to change the identity addr with bt_id_set_adv_private_addr() + * - The advertiser uses the same identity address as the scanner. + */ +ZTEST(bt_id_set_adv_own_addr, test_set_adv_own_addr_while_scanning_with_identity_random) +{ + uint32_t options = 0; + struct bt_le_ext_adv adv = {0}; + struct net_buf net_buff; + int err; + uint8_t scan_own_addr_type = BT_ADDR_LE_ANONYMOUS; + uint8_t adv_own_addr_type = BT_ADDR_LE_ANONYMOUS; + + Z_TEST_SKIP_IFDEF(CONFIG_BT_PRIVACY); + Z_TEST_SKIP_IFDEF(CONFIG_BT_EXT_ADV); + Z_TEST_SKIP_IFNDEF(CONFIG_BT_OBSERVER); + Z_TEST_SKIP_IFNDEF(CONFIG_BT_SCAN_WITH_IDENTITY); + + bt_hci_cmd_alloc_fake.return_val = &net_buff; + bt_hci_cmd_send_sync_fake.return_val = 0; + + options &= ~BT_LE_ADV_OPT_CONN; + bt_addr_le_copy(&bt_dev.id_addr[BT_ID_DEFAULT], BT_STATIC_RANDOM_LE_ADDR_1); + + err = bt_id_set_scan_own_addr(false, &scan_own_addr_type); + + expect_single_call_bt_hci_cmd_alloc(); + expect_single_call_bt_hci_cmd_send_sync(BT_HCI_OP_LE_SET_RANDOM_ADDRESS); + + zassert_ok(err, "Unexpected error code '%d' was returned", err); + zassert_true(scan_own_addr_type == BT_HCI_OWN_ADDR_RANDOM, + "Address type reference was incorrectly set"); + + atomic_set_bit(bt_dev.flags, BT_DEV_SCANNING); + + bt_id_set_adv_own_addr(&adv, options, true, &adv_own_addr_type); + zassert_true(adv_own_addr_type == BT_HCI_OWN_ADDR_RANDOM, + "Address type reference was incorrectly set"); + + expect_call_count_bt_le_scan_set_enable(0, NULL); +} + +/* + * Test setting the advertiser address while 'CONFIG_BT_SCAN_WITH_IDENTITY' is enabled + * and scanning is ongoing. The scanner is using a public identity address. + * + * Constraints: + * - Options 'BT_LE_ADV_OPT_CONN' bit isn't set + * + * Expected behaviour: + * - Scanning is not disabled. + * - The advertiser doesn't attempt to change the identity addr with bt_id_set_adv_private_addr() + * - The advertiser uses the same identity address as the scanner. + */ +ZTEST(bt_id_set_adv_own_addr, test_set_adv_own_addr_while_scanning_with_identity_public) +{ + uint32_t options = 0; + struct bt_le_ext_adv adv = {0}; + int err; + uint8_t scan_own_addr_type = BT_ADDR_LE_ANONYMOUS; + uint8_t adv_own_addr_type = BT_ADDR_LE_ANONYMOUS; + + Z_TEST_SKIP_IFDEF(CONFIG_BT_PRIVACY); + Z_TEST_SKIP_IFDEF(CONFIG_BT_EXT_ADV); + Z_TEST_SKIP_IFNDEF(CONFIG_BT_OBSERVER); + Z_TEST_SKIP_IFNDEF(CONFIG_BT_SCAN_WITH_IDENTITY); + + options &= ~BT_LE_ADV_OPT_CONN; + + bt_addr_le_copy(&bt_dev.id_addr[BT_ID_DEFAULT], BT_LE_ADDR); + + err = bt_id_set_scan_own_addr(false, &scan_own_addr_type); + + zassert_ok(err, "Unexpected error code '%d' was returned", err); + zassert_true(scan_own_addr_type == BT_HCI_OWN_ADDR_PUBLIC, + "Address type reference was incorrectly set"); + + atomic_set_bit(bt_dev.flags, BT_DEV_SCANNING); + + bt_id_set_adv_own_addr(&adv, options, true, &adv_own_addr_type); + zassert_true(adv_own_addr_type == BT_HCI_OWN_ADDR_PUBLIC, + "Address type reference was incorrectly set"); + + expect_call_count_bt_le_scan_set_enable(0, NULL); +} diff --git a/tests/bluetooth/host/id/bt_id_set_adv_own_addr/testcase.yaml b/tests/bluetooth/host/id/bt_id_set_adv_own_addr/testcase.yaml index 145096d0ce7..d5ab3e4ef85 100644 --- a/tests/bluetooth/host/id/bt_id_set_adv_own_addr/testcase.yaml +++ b/tests/bluetooth/host/id/bt_id_set_adv_own_addr/testcase.yaml @@ -14,3 +14,7 @@ tests: extra_configs: - CONFIG_BT_SMP=y - CONFIG_BT_PRIVACY=y + bluetooth.host.bt_id_set_adv_own_addr.scan_with_identity: + type: unit + extra_configs: + - CONFIG_BT_SCAN_WITH_IDENTITY=y From d6e86f4c23a41de48830b73681bda21331f8b713 Mon Sep 17 00:00:00 2001 From: Olivier Lesage Date: Wed, 2 Jul 2025 08:03:14 +0200 Subject: [PATCH 4/4] [nrf fromtree] bluetooth: host: Handle failure to disable scan when updating own_addr It wasn't taken into account that bt_le_scan_set_enable() has a return value. It's not likely that the controller rejects the command when BT_DEV_SCANNING is set, however. Signed-off-by: Olivier Lesage (cherry picked from commit aa4e6ac0ddedd3a1a3a30f0481ffec234fc9cd7c) --- subsys/bluetooth/host/id.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/subsys/bluetooth/host/id.c b/subsys/bluetooth/host/id.c index bf79215c450..436feb32c1b 100644 --- a/subsys/bluetooth/host/id.c +++ b/subsys/bluetooth/host/id.c @@ -2181,8 +2181,8 @@ int bt_id_set_adv_own_addr(struct bt_le_ext_adv *adv, uint32_t options, if (!IS_ENABLED(CONFIG_BT_PRIVACY) && !IS_ENABLED(CONFIG_BT_SCAN_WITH_IDENTITY) && dev_scanning) { - scan_disabled = true; - bt_le_scan_set_enable(BT_HCI_LE_SCAN_DISABLE); + err = bt_le_scan_set_enable(BT_HCI_LE_SCAN_DISABLE); + scan_disabled = err == 0; } /* If we are scanning with the identity address, it does