Bluetooth: audio: Fix building and reading PAC Records

This fixes missing checks, and invalid struct bt_pac definition that was
missing one mandatory metadata byte.

Signed-off-by: Mariusz Skamra <mariusz.skamra@codecoup.pl>

subsys/bluetooth/audio/pacs.c

@@ -41,21 +41,29 @@ IF_ENABLED(CONFIG_BT_PAC_SRC, (static enum bt_audio_context source_available_con
 
 NET_BUF_SIMPLE_DEFINE_STATIC(read_buf, CONFIG_BT_L2CAP_TX_MTU);
 
-static void pac_data_add(struct net_buf_simple *buf, uint8_t num,
+static ssize_t pac_data_add(struct net_buf_simple *buf, size_t count,
-			 struct bt_codec_data *data)
+			    struct bt_codec_data *data)
 {
-	for (uint8_t i = 0; i < num; i++) {
+	size_t len = 0;
-		struct bt_pac_codec_capability *cc;
-		struct bt_data *d = &data[i].data;
 
-		cc = net_buf_simple_add(buf, sizeof(*cc));
+	for (size_t i = 0; i < count; i++) {
-		cc->len = d->data_len + sizeof(cc->type);
+		struct bt_pac_ltv *ltv;
-		cc->type = d->type;
+		struct bt_data *d = &data[i].data;
+		const size_t ltv_len = sizeof(*ltv) + d->data_len;
+
+		if (net_buf_simple_tailroom(buf) < ltv_len) {
+			return -ENOMEM;
+		}
+
+		ltv = net_buf_simple_add(buf, sizeof(*ltv));
+		ltv->len = d->data_len + sizeof(ltv->type);
+		ltv->type = d->type;
 		net_buf_simple_add_mem(buf, d->data, d->data_len);
 
-		BT_DBG("  %u: type %u: %s",
+		len += ltv_len;
-		       i, d->type, bt_hex(d->data, d->data_len));
 	}
+
+	return len;
 }
 
 struct pac_records_build_data {
@@ -66,41 +74,60 @@ struct pac_records_build_data {
 static bool build_pac_records(const struct bt_audio_capability *capability, void *user_data)
 {
 	struct pac_records_build_data *data = user_data;
+	struct bt_codec *codec = capability->codec;
 	struct net_buf_simple *buf = data->buf;
-	struct bt_pac_meta *meta;
+	struct net_buf_simple_state state;
-	struct bt_codec *codec;
+	struct bt_pac_ltv_data *cc, *meta;
 	struct bt_pac *pac;
+	ssize_t len;
 
-	codec = capability->codec;
+	net_buf_simple_save(buf, &state);
+
+	if (net_buf_simple_tailroom(buf) < sizeof(*pac)) {
+		goto fail;
+	}
 
 	pac = net_buf_simple_add(buf, sizeof(*pac));
 	pac->codec.id = codec->id;
 	pac->codec.cid = sys_cpu_to_le16(codec->cid);
 	pac->codec.vid = sys_cpu_to_le16(codec->vid);
-	pac->cc_len = buf->len;
 
-	BT_DBG("Parsing codec config data");
+	if (net_buf_simple_tailroom(buf) < sizeof(*cc)) {
-	pac_data_add(buf, codec->data_count, codec->data);
+		goto fail;
+	}
 
-	/* Buffer size shall never be below PAC len since we are just
+	cc = net_buf_simple_add(buf, sizeof(*cc));
-	 * append data.
-	 */
-	__ASSERT_NO_MSG(buf->len >= pac->cc_len);
 
-	pac->cc_len = buf->len - pac->cc_len;
+	len = pac_data_add(buf, codec->data_count, codec->data);
+	if (len < 0 || len > UINT8_MAX) {
+		goto fail;
+	}
+
+	cc->len = len;
+
+	if (net_buf_simple_tailroom(buf) < sizeof(*meta)) {
+		goto fail;
+	}
 
 	meta = net_buf_simple_add(buf, sizeof(*meta));
-	meta->len = buf->len;
-	BT_DBG("Parsing metadata");
-	pac_data_add(buf, codec->meta_count, codec->meta);
-	meta->len = buf->len - meta->len;
 
-	BT_DBG("pac #%u: codec capability len %u metadata len %u",
+	len = pac_data_add(buf, codec->meta_count, codec->meta);
-		data->rsp->num_pac, pac->cc_len, meta->len);
+	if (len < 0 || len > UINT8_MAX) {
+		goto fail;
+	}
+
+	meta->len = len;
 
 	data->rsp->num_pac++;
 
 	return true;
+
+fail:
+	__ASSERT(true, "No space for %p", capability);
+
+	net_buf_simple_restore(buf, &state);
+
+	return false;
 }
 
 static void get_pac_records(struct bt_conn *conn, enum bt_audio_dir dir,

subsys/bluetooth/audio/pacs_internal.h

@@ -22,22 +22,21 @@ struct bt_pac_codec {
 #define BT_CODEC_CAP_DRM		0x0a
 #define BT_CODEC_CAP_DRM_VALUE		0x0b
 
-struct bt_pac_codec_capability {
+struct bt_pac_ltv {
-	uint8_t  len;			/* Codec Capability length */
+	uint8_t  len;
-	uint8_t  type;			/* Codec Capability type */
+	uint8_t  type;
-	uint8_t  data[0];		/* Codec Capability data */
+	uint8_t  value[0];
 } __packed;
 
-struct bt_pac_meta {
+struct bt_pac_ltv_data {
-	uint8_t  len;			/* Metadata Length */
+	uint8_t  len;
-	uint8_t  value[0];		/* Metadata Value */
+	struct bt_pac_ltv data[0];
 } __packed;
 
 struct bt_pac {
 	struct bt_pac_codec codec;	/* Codec ID */
-	uint8_t  cc_len;		/* Codec Capabilities Length */
+	struct bt_pac_ltv_data cc;	/* Codec Specific Capabilities */
-	struct bt_pac_codec_capability cc[0]; /* Codec Specific Capabilities */
+	struct bt_pac_ltv_data meta;	/* Metadata */
-	struct bt_pac_meta meta[0];	/* Metadata */
 } __packed;
 
 struct bt_pacs_read_rsp {

subsys/bluetooth/audio/unicast_client.c

@@ -69,14 +69,12 @@ static struct bt_gatt_discover_params avail_ctx_cc_disc[CONFIG_BT_MAX_CONN];
 static const struct bt_audio_unicast_client_cb *unicast_client_cbs;
 
 /* TODO: Move the functions to avoid these prototypes */
-static int unicast_client_ep_set_metadata(struct bt_audio_ep *ep,
+static int unicast_client_ep_set_metadata(struct bt_audio_ep *ep, void *data,
-					  struct net_buf_simple *buf,
 					  uint8_t len, struct bt_codec *codec);
 
 static int unicast_client_ep_set_codec(struct bt_audio_ep *ep, uint8_t id,
 				       uint16_t cid, uint16_t vid,
-				       struct net_buf_simple *buf,
+				       void *data, uint8_t len, struct bt_codec *codec);
-				       uint8_t len, struct bt_codec *codec);
 
 static void unicast_client_ep_iso_recv(struct bt_iso_chan *chan,
 				       const struct bt_iso_recv_info *info,
@@ -525,6 +523,7 @@ static void unicast_client_ep_config_state(struct bt_audio_ep *ep,
 	struct bt_ascs_ase_status_config *cfg;
 	struct bt_codec_qos_pref *pref;
 	struct bt_audio_stream *stream;
+	void *cc;
 
 	if (buf->len < sizeof(*cfg)) {
 		BT_ERR("Config status too short");
@@ -555,6 +554,8 @@ static void unicast_client_ep_config_state(struct bt_audio_ep *ep,
 		return;
 	}
 
+	cc = net_buf_simple_pull_mem(buf, cfg->cc_len);
+
 	pref = &stream->ep->qos_pref;
 
 	/* Convert to interval representation so they can be matched by QoS */
@@ -575,7 +576,7 @@ static void unicast_client_ep_config_state(struct bt_audio_ep *ep,
 	unicast_client_ep_set_codec(ep, cfg->codec.id,
 				    sys_le16_to_cpu(cfg->codec.cid),
 				    sys_le16_to_cpu(cfg->codec.vid),
-				    buf, cfg->cc_len, NULL);
+				    cc, cfg->cc_len, NULL);
 
 	/* Notify upper layer */
 	if (stream->ops != NULL && stream->ops->configured != NULL) {
@@ -663,6 +664,7 @@ static void unicast_client_ep_enabling_state(struct bt_audio_ep *ep,
 {
 	struct bt_ascs_ase_status_enable *enable;
 	struct bt_audio_stream *stream;
+	void *metadata;
 
 	if (buf->len < sizeof(*enable)) {
 		BT_ERR("Enabling status too short");
@@ -677,10 +679,18 @@ static void unicast_client_ep_enabling_state(struct bt_audio_ep *ep,
 
 	enable = net_buf_simple_pull_mem(buf, sizeof(*enable));
 
+	if (buf->len < enable->metadata_len) {
+		BT_ERR("Malformed PDU: remaining len %u expected %u",
+		       buf->len, enable->metadata_len);
+		return;
+	}
+
+	metadata = net_buf_simple_pull_mem(buf, enable->metadata_len);
+
 	BT_DBG("dir 0x%02x cig 0x%02x cis 0x%02x",
 	       ep->dir, ep->cig_id, ep->cis_id);
 
-	unicast_client_ep_set_metadata(ep, buf, enable->metadata_len, NULL);
+	unicast_client_ep_set_metadata(ep, metadata, enable->metadata_len, NULL);
 
 	/* Notify upper layer
 	 *
@@ -1024,7 +1034,7 @@ static bool unicast_client_codec_config_store(struct bt_data *data,
 
 static int unicast_client_ep_set_codec(struct bt_audio_ep *ep, uint8_t id,
 				       uint16_t cid, uint16_t vid,
-				       struct net_buf_simple *buf, uint8_t len,
+				       void *data, uint8_t len,
 				       struct bt_codec *codec)
 {
 	struct net_buf_simple ad;
@@ -1052,8 +1062,7 @@ static int unicast_client_ep_set_codec(struct bt_audio_ep *ep, uint8_t id,
 		return 0;
 	}
 
-	net_buf_simple_init_with_data(&ad, net_buf_simple_pull_mem(buf, len),
+	net_buf_simple_init_with_data(&ad, data, len);
-				      len);
 
 	/* Parse LTV entries */
 	bt_data_parse(&ad, unicast_client_codec_config_store, codec);
@@ -1107,8 +1116,7 @@ static bool unicast_client_codec_metadata_store(struct bt_data *data,
 	return true;
 }
 
-static int unicast_client_ep_set_metadata(struct bt_audio_ep *ep,
+static int unicast_client_ep_set_metadata(struct bt_audio_ep *ep, void *data,
-					  struct net_buf_simple *buf,
 					  uint8_t len, struct bt_codec *codec)
 {
 	struct net_buf_simple meta;
@@ -1132,8 +1140,7 @@ static int unicast_client_ep_set_metadata(struct bt_audio_ep *ep,
 		return 0;
 	}
 
-	net_buf_simple_init_with_data(&meta, net_buf_simple_pull_mem(buf, len),
+	net_buf_simple_init_with_data(&meta, data, len);
-				      len);
 
 	/* Parse LTV entries */
 	bt_data_parse(&meta, unicast_client_codec_metadata_store, codec);
@@ -2507,7 +2514,10 @@ static uint8_t unicast_client_read_func(struct bt_conn *conn, uint8_t err,
 	while (rsp->num_pac) {
 		struct unicast_client_pac *bpac;
 		struct bt_pac *pac;
-		struct bt_pac_meta *meta;
+		struct bt_pac_ltv_data *meta, *cc;
+		void *cc_ltv, *meta_ltv;
+
+		BT_DBG("pac #%u", params->num_caps);
 
 		if (buf.len < sizeof(*pac)) {
 			BT_ERR("Malformed PAC: remaining len %u expected %zu",
@@ -2517,14 +2527,36 @@ static uint8_t unicast_client_read_func(struct bt_conn *conn, uint8_t err,
 
 		pac = net_buf_simple_pull_mem(&buf, sizeof(*pac));
 
-		BT_DBG("pac #%u: cc len %u", params->num_caps, pac->cc_len);
+		if (buf.len < sizeof(*cc)) {
-
+			BT_ERR("Malformed PAC: remaining len %u expected %zu",
-		if (buf.len < pac->cc_len) {
+			       buf.len, sizeof(*cc));
-			BT_ERR("Malformed PAC: buf->len %u < %u pac->cc_len",
-			       buf.len, pac->cc_len);
 			break;
 		}
 
+		cc = net_buf_simple_pull_mem(&buf, sizeof(*cc));
+		if (buf.len < cc->len) {
+			BT_ERR("Malformed PAC: remaining len %u expected %zu",
+			       buf.len, cc->len);
+			break;
+		}
+
+		cc_ltv = net_buf_simple_pull_mem(&buf, cc->len);
+
+		if (buf.len < sizeof(*meta)) {
+			BT_ERR("Malformed PAC: remaining len %u expected %zu",
+			       buf.len, sizeof(*meta));
+			break;
+		}
+
+		meta = net_buf_simple_pull_mem(&buf, sizeof(*meta));
+		if (buf.len < meta->len) {
+			BT_ERR("Malformed PAC: remaining len %u expected %u",
+			       buf.len, meta->len);
+			break;
+		}
+
+		meta_ltv = net_buf_simple_pull_mem(&buf, meta->len);
+
 		bpac = unicast_client_pac_alloc(conn, params->dir);
 		if (!bpac) {
 			BT_WARN("No space left to parse PAC");
@@ -2538,22 +2570,13 @@ static uint8_t unicast_client_read_func(struct bt_conn *conn, uint8_t err,
 		if (unicast_client_ep_set_codec(NULL, pac->codec.id,
 						sys_le16_to_cpu(pac->codec.cid),
 						sys_le16_to_cpu(pac->codec.vid),
-						&buf, pac->cc_len,
+						cc_ltv, cc->len,
 						&bpac->codec)) {
 			BT_ERR("Unable to parse Codec");
 			break;
 		}
 
-		meta = net_buf_simple_pull_mem(&buf, sizeof(*meta));
+		if (unicast_client_ep_set_metadata(NULL, meta_ltv, meta->len, &bpac->codec)) {
-
-		if (buf.len < meta->len) {
-			BT_ERR("Malformed PAC: remaining len %u expected %u",
-			       buf.len, meta->len);
-			break;
-		}
-
-		if (unicast_client_ep_set_metadata(NULL, &buf, meta->len,
-						   &bpac->codec)) {
 			BT_ERR("Unable to parse Codec Metadata");
 			break;
 		}