bluetooth: host: smp: fix deadlock when public key generation fails

When `bt_le_oob_get_local` or `bt_le_ext_adv_oob_get_local` is called
and SMP is enabled, `bt_smp_le_oob_generate_sc_data` is called to
generate a Random Number and a Confirmation Value needed for OOB data.
These values are based on the device's public key.

The public key is generated only once when `bt_smp_init` is called.
If public key generation fails, the callback passed to `bt_pub_key_get`
is called with `pkey` set to NULL. The `bt_smp_pkey_ready` callback
gets called, but it doesn't release the `sc_local_pkey_ready` semaphore
thus leaving `bt_smp_le_oob_generate_sc_data` wait for semaphore with
`K_FOREVER`.

This commit replaces the semaphore with a conditional variable and
requests a public key again if the key is NULL thus solving 2 issues:
- handling the case where the callback was triggered notifying about the
  completion of the public key request, but the key was not generated,
- handling the case where multiple threads trying to acquire the same
  sempahore.

The timeout is used instead of K_FOREVER to avoid cases when callback
has never been triggered.

Signed-off-by: Pavel Vasilyev <[email protected]>

Author: PavelVPV

subsys/bluetooth/host/smp.c

@@ -284,7 +284,19 @@ static bool sc_oobd_present;
 static bool legacy_oobd_present;
 static bool sc_supported;
 static const uint8_t *sc_public_key;
-static K_SEM_DEFINE(sc_local_pkey_ready, 0, 1);
+
+static void bt_smp_pkey_ready(const uint8_t *pkey);
+static struct {
+	struct k_mutex lock;
+	struct k_condvar condvar;
+	struct bt_pub_key_cb cb;
+} pub_key_gen = {
+	.lock = Z_MUTEX_INITIALIZER(pub_key_gen.lock),
+	.condvar = Z_CONDVAR_INITIALIZER(pub_key_gen.condvar),
+	.cb = {
+		.func = bt_smp_pkey_ready,
+	},
+};
 
 /* Pointer to internal data is used to mark that callbacks of given SMP channel are not initialized.
  * Value of NULL represents no authentication capabilities and cannot be used for that purpose.
@@ -4627,6 +4639,21 @@ static int bt_smp_recv(struct bt_l2cap_chan *chan, struct net_buf *buf)
 	return 0;
 }
 
+static void pub_key_ready_notify(void)
+{
+	int err;
+
+	ARG_UNUSED(err);
+
+	err = k_mutex_lock(&pub_key_gen.lock, K_FOREVER);
+	__ASSERT_NO_MSG(err == 0);
+
+	(void)k_condvar_broadcast(&pub_key_gen.condvar);
+
+	err = k_mutex_unlock(&pub_key_gen.lock);
+	__ASSERT_NO_MSG(err == 0);
+}
+
 static void bt_smp_pkey_ready(const uint8_t *pkey)
 {
 	int i;
@@ -4635,13 +4662,13 @@ static void bt_smp_pkey_ready(const uint8_t *pkey)
 
 	sc_public_key = pkey;
 
+	pub_key_ready_notify();
+
 	if (!pkey) {
 		LOG_WRN("Public key not available");
 		return;
 	}
 
-	k_sem_give(&sc_local_pkey_ready);
-
 	for (i = 0; i < ARRAY_SIZE(bt_smp_pool); i++) {
 		struct bt_smp *smp = &bt_smp_pool[i];
 		uint8_t err;
@@ -5668,10 +5695,42 @@ int bt_smp_le_oob_generate_sc_data(struct bt_le_oob_sc_data *le_sc_oob)
 	}
 
 	if (!sc_public_key) {
-		err = k_sem_take(&sc_local_pkey_ready, K_FOREVER);
+		/* Public key request has not been finished yet, or finished, but generation failed.
+		 * Retrying.
+		 */
+		err = k_mutex_lock(&pub_key_gen.lock, K_FOREVER);
+		__ASSERT_NO_MSG(err == 0);
+
+		err = bt_pub_key_gen(&pub_key_gen.cb);
+		if (err && err != -EALREADY) {
+			LOG_WRN("Public key re-generation request failed (%d)", err);
+
+			int mutex_err;
+
+			mutex_err = k_mutex_unlock(&pub_key_gen.lock);
+			__ASSERT_NO_MSG(mutex_err == 0);
+
+			return err;
+		}
+
+		/* 30 seconds is an arbitrary number. Increase if fails earlier on certain
+		 * platforms.
+		 */
+		err = k_condvar_wait(&pub_key_gen.condvar,
+				     &pub_key_gen.lock,
+				     K_SECONDS(30));
 		if (err) {
+			LOG_WRN("Public key generation timeout");
 			return err;
 		}
+
+		err = k_mutex_unlock(&pub_key_gen.lock);
+		__ASSERT_NO_MSG(err == 0);
+
+		/* Public key has been re-requested but generation failed. */
+		if (!sc_public_key) {
+			return -EAGAIN;
+		}
 	}
 
 	if (IS_ENABLED(CONFIG_BT_OOB_DATA_FIXED)) {
@@ -6082,10 +6141,6 @@ BT_L2CAP_BR_CHANNEL_DEFINE(smp_br_fixed_chan, BT_L2CAP_CID_BR_SMP,
 
 int bt_smp_init(void)
 {
-	static struct bt_pub_key_cb pub_key_cb = {
-		.func           = bt_smp_pkey_ready,
-	};
-
 	sc_supported = le_sc_supported();
 	if (IS_ENABLED(CONFIG_BT_SMP_SC_PAIR_ONLY) && !sc_supported) {
 		LOG_ERR("SC Pair Only Mode selected but LE SC not supported");
@@ -6100,7 +6155,7 @@ int bt_smp_init(void)
 	LOG_DBG("LE SC %s", sc_supported ? "enabled" : "disabled");
 
 	if (!IS_ENABLED(CONFIG_BT_SMP_OOB_LEGACY_PAIR_ONLY)) {
-		bt_pub_key_gen(&pub_key_cb);
+		bt_pub_key_gen(&pub_key_gen.cb);
 	}
 
 	return smp_self_test();