bluetooth: host: Fix uninitialized own_addr_type for legacy scan+adv

In 25c993e 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 <olivier.lesage@nordicsemi.no>

Author: olivier-le-sage

subsys/bluetooth/host/id.c

@@ -1789,7 +1789,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;
 
@@ -1805,7 +1805,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)
@@ -1838,20 +1858,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

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 <zephyr/bluetooth/hci.h>
@@ -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

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