Bluetooth: BAP: Add validation of qos_pref

The QoS preference defined by ASCS has some specified
limits and values that we should enforce.

Given the current API we cannot return an error to the
unicast server if it supplies invalid values, so we have
to resort to a LOG_ERR.

For the unicast client we treat invalid QoS preferences
similar to other invalid data in the notifications.

This also adds additional documentation in the
bt_audio_codec_qos_pref struct.

Signed-off-by: Emil Gydesen <[email protected]>

Author: Thalley

include/zephyr/bluetooth/audio/audio.h

@@ -978,13 +978,22 @@ struct bt_audio_codec_qos_pref {
 	 */
 	bool unframed_supported;
 
-	/** Preferred PHY */
+	/**
+	 * @brief Preferred PHY bitfield
+	 *
+	 * Bitfield consisting of one or more of @ref BT_GAP_LE_PHY_1M, @ref BT_GAP_LE_PHY_2M and
+	 * @ref BT_GAP_LE_PHY_CODED.
+	 */
 	uint8_t phy;
 
 	/** Preferred Retransmission Number */
 	uint8_t rtn;
 
-	/** Preferred Transport Latency */
+	/**
+	 * Preferred Transport Latency
+	 *
+	 * Value range @ref BT_ISO_LATENCY_MIN to @ref BT_ISO_LATENCY_MAX
+	 */
 	uint16_t latency;
 
 	/**
@@ -1010,14 +1019,15 @@ struct bt_audio_codec_qos_pref {
 	/**
 	 * @brief Preferred minimum Presentation Delay
 	 *
-	 * Value range 0 to @ref BT_AUDIO_PD_MAX.
+	 * Value range @ref bt_audio_codec_qos_pref.pd_min to @ref bt_audio_codec_qos_pref.pd_max.
 	 */
 	uint32_t pref_pd_min;
 
 	/**
 	 * @brief Preferred maximum Presentation Delay
 	 *
-	 * Value range 0 to @ref BT_AUDIO_PD_MAX.
+	 * Value range @ref bt_audio_codec_qos_pref.pd_min to @ref bt_audio_codec_qos_pref.pd_max,
+	 * and higher than @ref bt_audio_codec_qos_pref.pref_pd_min
 	 */
 	uint32_t pref_pd_max;
 };

subsys/bluetooth/audio/ascs.c

@@ -1631,6 +1631,16 @@ static int ase_config(struct bt_ascs_ase *ase, const struct bt_ascs_config *cfg)
 					      BT_BAP_ASCS_REASON_NONE);
 		}
 
+		if (err == 0 && !bt_audio_valid_qos_pref(&ase->ep.qos_pref)) {
+			LOG_ERR("Invalid QoS preferences");
+
+			/* If the upper layers provide an invalid QoS preferences we reject the
+			 * request from the client, as it would not be able to parse the result
+			 * as valid anyways
+			 */
+			err = -EINVAL;
+		}
+
 		if (err) {
 			ascs_app_rsp_warn_valid(&rsp);
 
@@ -1662,6 +1672,16 @@ static int ase_config(struct bt_ascs_ase *ase, const struct bt_ascs_config *cfg)
 					      BT_BAP_ASCS_REASON_NONE);
 		}
 
+		if (err == 0 && !bt_audio_valid_qos_pref(&ase->ep.qos_pref)) {
+			LOG_ERR("Invalid QoS preferences");
+
+			/* If the upper layers provide an invalid QoS preferences we reject the
+			 * request from the client, as it would not be able to parse the result
+			 * as valid anyways
+			 */
+			err = -EINVAL;
+		}
+
 		if (err || stream == NULL) {
 			ascs_app_rsp_warn_valid(&rsp);
 

subsys/bluetooth/audio/bap_stream.c

@@ -16,6 +16,7 @@
 #include <zephyr/autoconf.h>
 #include <zephyr/bluetooth/bluetooth.h>
 #include <zephyr/bluetooth/conn.h>
+#include <zephyr/bluetooth/gap.h>
 #include <zephyr/bluetooth/gatt.h>
 #include <zephyr/bluetooth/hci.h>
 #include <zephyr/bluetooth/hci_types.h>
@@ -264,6 +265,69 @@ bool bt_audio_valid_codec_cfg(const struct bt_audio_codec_cfg *codec_cfg)
 	return true;
 }
 
+bool bt_audio_valid_qos_pref(const struct bt_audio_codec_qos_pref *qos_pref)
+{
+	const uint8_t phy_mask = BT_GAP_LE_PHY_1M | BT_GAP_LE_PHY_2M | BT_GAP_LE_PHY_CODED;
+
+	if ((qos_pref->phy & (~phy_mask)) != 0U) {
+		LOG_DBG("Invalid phy: %u", qos_pref->phy);
+
+		return false;
+	}
+
+	if (!IN_RANGE(qos_pref->latency, BT_ISO_LATENCY_MIN, BT_ISO_LATENCY_MAX)) {
+		LOG_DBG("Invalid latency: %u", qos_pref->latency);
+
+		return false;
+	}
+
+	if (qos_pref->pd_min > BT_AUDIO_PD_MAX) {
+		LOG_DBG("Invalid pd_min: %u", qos_pref->pd_min);
+
+		return false;
+	}
+
+	if (qos_pref->pd_max > BT_AUDIO_PD_MAX) {
+		LOG_DBG("Invalid pd_min: %u", qos_pref->pd_min);
+
+		return false;
+	}
+
+	if (qos_pref->pd_max < qos_pref->pd_min) {
+		LOG_DBG("Invalid combination of pd_min %u and pd_max: %u", qos_pref->pd_min,
+			qos_pref->pd_max);
+
+		return false;
+	}
+
+	/* The absolute minimum and maximum values of pref_pd_min and pref_pd_max are implicitly
+	 * checked using the bounds of pd_min and pd_max, so we can just compare the preferences
+	 * to the min and max values that have been bound checked already
+	 */
+	if (!IN_RANGE(qos_pref->pref_pd_min, qos_pref->pd_min, qos_pref->pd_max)) {
+		LOG_DBG("Invalid combination of pref_pd_min %u, pd_min %u and pd_max: %u",
+			qos_pref->pref_pd_min, qos_pref->pd_min, qos_pref->pd_max);
+
+		return false;
+	}
+
+	if (qos_pref->pref_pd_max < qos_pref->pref_pd_min) {
+		LOG_DBG("Invalid combination of pref_pd_min %u and pref_pd_max: %u",
+			qos_pref->pref_pd_min, qos_pref->pref_pd_max);
+
+		return false;
+	}
+
+	if (!IN_RANGE(qos_pref->pref_pd_max, qos_pref->pd_min, qos_pref->pd_max)) {
+		LOG_DBG("Invalid combination of pref_pd_max %u, pd_min %u and pd_max: %u",
+			qos_pref->pref_pd_max, qos_pref->pd_min, qos_pref->pd_max);
+
+		return false;
+	}
+
+	return true;
+}
+
 #if defined(CONFIG_BT_AUDIO_TX)
 static bool bt_bap_stream_can_send(const struct bt_bap_stream *stream)
 {

subsys/bluetooth/audio/bap_stream.h

@@ -32,6 +32,7 @@ void bt_bap_stream_detach(struct bt_bap_stream *stream);
 
 enum bt_bap_ascs_reason bt_audio_verify_qos(const struct bt_audio_codec_qos *qos);
 bool bt_audio_valid_codec_cfg(const struct bt_audio_codec_cfg *codec_cfg);
+bool bt_audio_valid_qos_pref(const struct bt_audio_codec_qos_pref *qos_pref);
 bool bt_bap_stream_can_disconnect(const struct bt_bap_stream *stream);
 
 enum bt_bap_ascs_reason bt_bap_stream_verify_qos(const struct bt_bap_stream *stream,

subsys/bluetooth/audio/bap_unicast_client.c

@@ -740,6 +740,16 @@ static void unicast_client_ep_config_state(struct bt_bap_ep *ep, struct net_buf_
 		pref->latency, pref->pd_min, pref->pd_max, pref->pref_pd_min, pref->pref_pd_max,
 		stream->codec_cfg->id);
 
+	if (!bt_audio_valid_qos_pref(pref)) {
+		LOG_DBG("Invalid QoS preferences");
+		memset(pref, 0, sizeof(*pref));
+
+		/* If the sever provide an invalid QoS preferences we treat it as an error and do
+		 * nothing
+		 */
+		return;
+	}
+
 	unicast_client_ep_set_codec_cfg(ep, cfg->codec.id, sys_le16_to_cpu(cfg->codec.cid),
 					sys_le16_to_cpu(cfg->codec.vid), cc, cfg->cc_len, NULL);