Bluetooth: host: Remove cancel sync from database hash commit

Fix deadlock when db_hash_commit has to wait for the delayed work to
finish. This creates a deadlock if the delayed work for database hash
calculation needs to store the hash since the settings API is locked
when calling the commit callback.
Remove call to k_work_cancel_delayable_sync from db_hash_commit in order
to avoid the deadlock. Instead move comparing of the stored hash to the
delayed work and reschedule the work with no wait.

Signed-off-by: Joakim Andersson <[email protected]>

Author: joerchan

subsys/bluetooth/host/gatt.c

@@ -229,6 +229,7 @@ enum {
 
 #if defined(CONFIG_BT_GATT_CACHING)
 	DB_HASH_VALID,       /* Database hash needs to be calculated */
+	DB_HASH_LOAD,        /* Database hash loaded from settings. */
 #endif
 	/* Total number of flags - must be at the end of the enum */
 	SC_NUM_FLAGS,
@@ -248,6 +249,9 @@ static struct gatt_sc {
 #if defined(CONFIG_BT_GATT_CACHING)
 static struct db_hash {
 	uint8_t hash[16];
+#if defined(CONFIG_BT_SETTINGS)
+	 uint8_t stored_hash[16];
+#endif
 	struct k_work_delayable work;
 	struct k_work_sync sync;
 } db_hash;
@@ -719,8 +723,44 @@ static void db_hash_gen(bool store)
 	atomic_set_bit(gatt_sc.flags, DB_HASH_VALID);
 }
 
+#if defined(CONFIG_BT_SETTINGS)
+static void sc_indicate(uint16_t start, uint16_t end);
+#endif
+
 static void db_hash_process(struct k_work *work)
 {
+#if defined(CONFIG_BT_SETTINGS)
+	if (atomic_test_and_clear_bit(gatt_sc.flags, DB_HASH_LOAD)) {
+		if (!atomic_test_bit(gatt_sc.flags, DB_HASH_VALID)) {
+			db_hash_gen(false);
+		}
+
+		/* Check if hash matches then skip SC update */
+		if (!memcmp(db_hash.stored_hash, db_hash.hash,
+			    sizeof(db_hash.stored_hash))) {
+			BT_DBG("Database Hash matches");
+			k_work_cancel_delayable(&gatt_sc.work);
+			atomic_clear_bit(gatt_sc.flags, SC_RANGE_CHANGED);
+			return;
+		}
+
+		BT_HEXDUMP_DBG(db_hash.hash, sizeof(db_hash.hash),
+			       "New Hash: ");
+
+		/* GATT database has been modified since last boot, likely due
+		 * to a firmware update or a dynamic service that was not
+		 * re-registered on boot.
+		 * Indicate Service Changed to all bonded devices for the full
+		 * database range to invalidate client-side cache and force
+		 * discovery on reconnect.
+		 */
+		sc_indicate(0x0001, 0xffff);
+
+		/* Hash did not match, overwrite with current hash */
+		db_hash_store();
+		return;
+	}
+#endif /* defined(CONFIG_BT_SETTINGS) */
 	db_hash_gen(true);
 }
 
@@ -5147,58 +5187,30 @@ static int cf_set(const char *name, size_t len_rd, settings_read_cb read_cb,
 
 SETTINGS_STATIC_HANDLER_DEFINE(bt_cf, "bt/cf", NULL, cf_set, NULL, NULL);
 
-static uint8_t stored_hash[16];
-
 static int db_hash_set(const char *name, size_t len_rd,
 		       settings_read_cb read_cb, void *cb_arg)
 {
 	ssize_t len;
 
-	len = read_cb(cb_arg, stored_hash, sizeof(stored_hash));
+	len = read_cb(cb_arg, db_hash.stored_hash, sizeof(db_hash.stored_hash));
 	if (len < 0) {
 		BT_ERR("Failed to decode value (err %zd)", len);
 		return len;
 	}
 
-	BT_HEXDUMP_DBG(stored_hash, sizeof(stored_hash), "Stored Hash: ");
+	BT_HEXDUMP_DBG(db_hash.stored_hash, sizeof(db_hash.stored_hash),
+		       "Stored Hash: ");
 
 	return 0;
 }
 
 static int db_hash_commit(void)
 {
-
-	k_sched_lock();
-
-	/* Stop work and generate the hash */
-	(void)k_work_cancel_delayable_sync(&db_hash.work, &db_hash.sync);
-	if (!atomic_test_bit(gatt_sc.flags, DB_HASH_VALID)) {
-		db_hash_gen(false);
-	}
-
-	k_sched_unlock();
-
-	/* Check if hash matches then skip SC update */
-	if (!memcmp(stored_hash, db_hash.hash, sizeof(stored_hash))) {
-		BT_DBG("Database Hash matches");
-		k_work_cancel_delayable(&gatt_sc.work);
-		atomic_clear_bit(gatt_sc.flags, SC_RANGE_CHANGED);
-		return 0;
-	}
-
-	BT_HEXDUMP_DBG(db_hash.hash, sizeof(db_hash.hash), "New Hash: ");
-
-	/**
-	 * GATT database has been modified since last boot, likely due to
-	 * a firmware update or a dynamic service that was not re-registered on
-	 * boot. Indicate Service Changed to all bonded devices for the full
-	 * database range to invalidate client-side cache and force discovery on
-	 * reconnect.
+	atomic_set_bit(gatt_sc.flags, DB_HASH_LOAD);
+	/* Reschedule work to calculate and compare against the Hash value
+	 * loaded from flash.
 	 */
-	sc_indicate(0x0001, 0xffff);
-
-	/* Hash did not match overwrite with current hash */
-	db_hash_store();
+	k_work_reschedule(&db_hash.work, K_NO_WAIT);
 
 	return 0;
 }