Bluetooth: Fix callback handling in ECC Public Key generation

Commit d6c34c4 changed the behavior
slightly but didn't update the documentation. The callback will now be
reset to NULL once the key is generated. Calling bt_pub_key_gen()
multiple times before the key is finished would result in creation of an
infinite loop. This could happen when an application calls mesh_init()
and mesh_reset() in quick succession. Clarify the behavior of the API in
the documentation.

Also passing a NULL argument would result in an undefined behavior, so
add a check to match the behavior described in documentation.

Signed-off-by: Michał Narajowski <michal.narajowski@codecoup.pl>

Author: Michał Narajowski

subsys/bluetooth/host/ecc.c

@@ -17,7 +17,7 @@
 #include "common/log.h"
 
 static uint8_t pub_key[64];
-static struct bt_pub_key_cb *pub_key_cb;
+static sys_slist_t pub_key_cb_slist;
 static bt_dh_key_cb_t dh_key_cb;
 
 static const uint8_t debug_public_key[64] = {
@@ -40,6 +40,7 @@ bool bt_pub_key_is_debug(uint8_t *pub_key)
 
 int bt_pub_key_gen(struct bt_pub_key_cb *new_cb)
 {
+	struct bt_pub_key_cb *cb;
 	int err;
 
 	/*
@@ -64,8 +65,18 @@ int bt_pub_key_gen(struct bt_pub_key_cb *new_cb)
 		}
 	}
 
-	new_cb->_next = pub_key_cb;
-	pub_key_cb = new_cb;
+	if (!new_cb) {
+		return -EINVAL;
+	}
+
+	SYS_SLIST_FOR_EACH_CONTAINER(&pub_key_cb_slist, cb, node) {
+		if (cb == new_cb) {
+			BT_WARN("Callback already registered");
+			return -EALREADY;
+		}
+	}
+
+	sys_slist_prepend(&pub_key_cb_slist, &new_cb->node);
 
 	if (atomic_test_and_set_bit(bt_dev.flags, BT_DEV_PUB_KEY_BUSY)) {
 		return 0;
@@ -75,9 +86,17 @@ int bt_pub_key_gen(struct bt_pub_key_cb *new_cb)
 
 	err = bt_hci_cmd_send_sync(BT_HCI_OP_LE_P256_PUBLIC_KEY, NULL, NULL);
 	if (err) {
+
 		BT_ERR("Sending LE P256 Public Key command failed");
 		atomic_clear_bit(bt_dev.flags, BT_DEV_PUB_KEY_BUSY);
-		pub_key_cb = NULL;
+
+		SYS_SLIST_FOR_EACH_CONTAINER(&pub_key_cb_slist, cb, node) {
+			if (cb->func) {
+				cb->func(NULL);
+			}
+		}
+
+		sys_slist_init(&pub_key_cb_slist);
 		return err;
 	}
 
@@ -180,11 +199,13 @@ void bt_hci_evt_le_pkey_complete(struct net_buf *buf)
 		atomic_set_bit(bt_dev.flags, BT_DEV_HAS_PUB_KEY);
 	}
 
-	for (cb = pub_key_cb; cb; cb = cb->_next) {
-		cb->func(evt->status ? NULL : pub_key);
+	SYS_SLIST_FOR_EACH_CONTAINER(&pub_key_cb_slist, cb, node) {
+		if (cb->func) {
+			cb->func(evt->status ? NULL : pub_key);
+		}
 	}
 
-	pub_key_cb = NULL;
+	sys_slist_init(&pub_key_cb_slist);
 }
 
 void bt_hci_evt_le_dhkey_complete(struct net_buf *buf)

subsys/bluetooth/host/ecc.h

@@ -18,7 +18,8 @@ struct bt_pub_key_cb {
 	 */
 	void (*func)(const uint8_t key[64]);
 
-	struct bt_pub_key_cb *_next;
+	/* Internal */
+	sys_snode_t node;
 };
 
 /*  @brief Check if public key is equal to the debug public key.
@@ -34,12 +35,14 @@ bool bt_pub_key_is_debug(uint8_t *pub_key);
 
 /*  @brief Generate a new Public Key.
  *
- *  Generate a new ECC Public Key. The callback will persist even after the
- *  key has been generated, and will be used to notify of new generation
- *  processes (NULL as key).
+ *  Generate a new ECC Public Key. Provided cb must persists until callback
+ *  is called. Callee adds the callback structure to a linked list. Registering
+ *  multiple callbacks requires multiple calls to bt_pub_key_gen() and separate
+ *  callback structures. This method cannot be called directly from result
+ *  callback. After calling all the registered callbacks the linked list
+ *  is cleared.
  *
- *  @param cb Callback to notify the new key, or NULL to request an update
- *            without registering any new callback.
+ *  @param cb Callback to notify the new key.
  *
  *  @return Zero on success or negative error code otherwise
  */