bluez5: clean up LTV writing

Add ltv_writer that checks bounds, and use it. Simplify code a bit.

spa/plugins/bluez5/bap-codec-caps.h

@@ -137,12 +137,6 @@
 #define BT_ISO_QOS_TARGET_LATENCY_BALANCED	0x02
 #define BT_ISO_QOS_TARGET_LATENCY_RELIABILITY	0x03
 
-struct __attribute__((packed)) ltv {
-	uint8_t  len;
-	uint8_t  type;
-	uint8_t  value[];
-};
-
 struct bap_endpoint_qos {
 	uint8_t	framing;
 	uint8_t	phy;

spa/plugins/bluez5/bap-codec-lc3.c

@@ -191,32 +191,6 @@ static unsigned int get_duration_mask(uint8_t rate) {
 	return 0;
 }
 
-static int write_ltv(uint8_t *dest, uint8_t type, void* value, size_t len)
-{
-	struct ltv *ltv = (struct ltv *)dest;
-
-	ltv->len = len + 1;
-	ltv->type = type;
-	memcpy(ltv->value, value, len);
-
-	return len + 2;
-}
-
-static int write_ltv_uint8(uint8_t *dest, uint8_t type, uint8_t value)
-{
-	return write_ltv(dest, type, &value, sizeof(value));
-}
-
-static int write_ltv_uint16(uint8_t *dest, uint8_t type, uint16_t value)
-{
-	return write_ltv(dest, type, &value, sizeof(value));
-}
-
-static int write_ltv_uint32(uint8_t *dest, uint8_t type, uint32_t value)
-{
-	return write_ltv(dest, type, &value, sizeof(value));
-}
-
 static uint16_t parse_rates(const char *str)
 {
 	struct spa_json it;
@@ -319,7 +293,6 @@ static uint8_t parse_channel_counts(const char *str)
 static int codec_fill_caps(const struct media_codec *codec, uint32_t flags,
 		const struct spa_dict *settings, uint8_t caps[A2DP_MAX_CAPS_SIZE])
 {
-	uint8_t *data = caps;
 	const char *str;
 	uint16_t framelen[2];
 	uint16_t rate_mask = LC3_FREQ_48KHZ | LC3_FREQ_44KHZ | LC3_FREQ_32KHZ | \
@@ -330,6 +303,7 @@ static int codec_fill_caps(const struct media_codec *codec, uint32_t flags,
 	uint16_t framelen_max = LC3_MAX_FRAME_BYTES;
 	uint8_t max_frames = 2;
 	uint32_t value;
+	struct ltv_writer writer = LTV_WRITER(caps, A2DP_MAX_CAPS_SIZE);
 
 	if (settings && (str = spa_dict_lookup(settings, "bluez5.bap-server-capabilities.rates")))
 		rate_mask = parse_rates(str);
@@ -355,17 +329,17 @@ static int codec_fill_caps(const struct media_codec *codec, uint32_t flags,
 	framelen[0] = htobs(framelen_min);
 	framelen[1] = htobs(framelen_max);
 
-	data += write_ltv_uint16(data, LC3_TYPE_FREQ, htobs(rate_mask));
+	ltv_writer_uint16(&writer, LC3_TYPE_FREQ, rate_mask);
-	data += write_ltv_uint8(data, LC3_TYPE_DUR, duration_mask);
+	ltv_writer_uint8(&writer, LC3_TYPE_DUR, duration_mask);
-	data += write_ltv_uint8(data, LC3_TYPE_CHAN, channel_counts);
+	ltv_writer_uint8(&writer, LC3_TYPE_CHAN, channel_counts);
-	data += write_ltv(data, LC3_TYPE_FRAMELEN, framelen, sizeof(framelen));
+	ltv_writer_data(&writer, LC3_TYPE_FRAMELEN, framelen, sizeof(framelen));
 	/* XXX: we support only one frame block -> max 2 frames per SDU */
 	if (max_frames > 2)
 		max_frames = 2;
 
-	data += write_ltv_uint8(data, LC3_TYPE_BLKS, max_frames);
+	ltv_writer_uint8(&writer, LC3_TYPE_BLKS, max_frames);
 
-	return data - caps;
+	return ltv_writer_end(&writer);
 }
 
 static void debugc_ltv(struct spa_debug_context *debug_ctx, int pac, struct ltv *ltv)
@@ -845,10 +819,10 @@ static int codec_select_config(const struct media_codec *codec, uint32_t flags,
 	struct pac_data pacs[MAX_PACS];
 	int npacs;
 	bap_lc3_t conf;
-	uint8_t *data = config;
 	struct spa_debug_log_ctx debug_ctx;
 	struct settings s;
 	int i;
+	struct ltv_writer writer = LTV_WRITER(config, A2DP_MAX_CAPS_SIZE);
 
 	if (caps == NULL)
 		return -EINVAL;
@@ -875,17 +849,17 @@ static int codec_select_config(const struct media_codec *codec, uint32_t flags,
 	if (!select_config(&conf, &pacs[0], &debug_ctx.ctx))
 		return -ENOTSUP;
 
-	data += write_ltv_uint8(data, LC3_TYPE_FREQ, conf.rate);
+	ltv_writer_uint8(&writer, LC3_TYPE_FREQ, conf.rate);
-	data += write_ltv_uint8(data, LC3_TYPE_DUR, conf.frame_duration);
+	ltv_writer_uint8(&writer, LC3_TYPE_DUR, conf.frame_duration);
 
 	/* Indicate MONO with absent Audio_Channel_Allocation (BAP v1.0.1 Sec. 4.3.2) */
 	if (conf.channels != 0)
-		data += write_ltv_uint32(data, LC3_TYPE_CHAN, htobl(conf.channels));
+		ltv_writer_uint32(&writer, LC3_TYPE_CHAN, conf.channels);
 
-	data += write_ltv_uint16(data, LC3_TYPE_FRAMELEN, htobs(conf.framelen));
+	ltv_writer_uint16(&writer, LC3_TYPE_FRAMELEN, conf.framelen);
-	data += write_ltv_uint8(data, LC3_TYPE_BLKS, conf.n_blks);
+	ltv_writer_uint8(&writer, LC3_TYPE_BLKS, conf.n_blks);
 
-	return data - config;
+	return ltv_writer_end(&writer);
 }
 
 static int codec_caps_preference_cmp(const struct media_codec *codec, uint32_t flags, const void *caps1, size_t caps1_size,
@@ -1378,80 +1352,60 @@ static int codec_get_bis_config(const struct media_codec *codec, uint8_t *caps,
 				uint8_t *caps_size, struct spa_dict *settings,
 				struct bap_codec_qos *qos)
 {
-	int index = 0x0;
+	const char *preset_name = NULL;
-	bool preset_found = false;
-	const char *preset = NULL;
 	int channel_allocation = 0;
-	uint8_t *data = caps;
+	int i, ret;
+	struct ltv_writer writer = LTV_WRITER(caps, *caps_size);
+	const struct bap_qos *preset = NULL;
+
 	*caps_size = 0;
-	int i;
 
 	if (settings) {
 		for (i = 0; i < (int)settings->n_items; ++i) {
 			if (spa_streq(settings->items[i].key, "channel_allocation"))
 				sscanf(settings->items[i].value, "%"PRIu32, &channel_allocation);
 			if (spa_streq(settings->items[i].key, "preset"))
-				preset = spa_dict_lookup(settings, "preset");
+				preset_name = settings->items[i].value;
 		}
 	}
 
-	if (preset == NULL)
+	if (preset_name == NULL)
 		return -EINVAL;
 
 	SPA_FOR_EACH_ELEMENT_VAR(bap_bcast_qos_configs, c) {
-		if (spa_streq(c->name, preset)) {
+		if (spa_streq(c->name, preset_name)) {
-			preset_found = true;
+			preset = c;
 			break;
 		}
-		index++;
 	}
 
-	if (!preset_found)
+	if (!preset)
 		return -EINVAL;
 
-	switch (bap_bcast_qos_configs[index].rate) {
+	ltv_writer_uint8(&writer, LC3_TYPE_FREQ, preset->rate);
-	case LC3_CONFIG_FREQ_48KHZ:
+	ltv_writer_uint16(&writer, LC3_TYPE_FRAMELEN, preset->framelen);
-		data += write_ltv_uint8(data, LC3_TYPE_FREQ, LC3_CONFIG_FREQ_48KHZ);
+	ltv_writer_uint8(&writer, LC3_TYPE_DUR, preset->frame_duration);
-		break;
+	ltv_writer_uint32(&writer, LC3_TYPE_CHAN, channel_allocation);
-	case LC3_CONFIG_FREQ_44KHZ:
-		data += write_ltv_uint8(data, LC3_TYPE_FREQ, LC3_CONFIG_FREQ_44KHZ);
-		break;
-	case LC3_CONFIG_FREQ_32KHZ:
-		data += write_ltv_uint8(data, LC3_TYPE_FREQ, LC3_CONFIG_FREQ_32KHZ);
-		break;
-	case LC3_CONFIG_FREQ_24KHZ:
-		data += write_ltv_uint8(data, LC3_TYPE_FREQ, LC3_CONFIG_FREQ_24KHZ);
-		break;
-	case LC3_CONFIG_FREQ_16KHZ:
-		data += write_ltv_uint8(data, LC3_TYPE_FREQ, LC3_CONFIG_FREQ_16KHZ);
-		break;
-	case LC3_CONFIG_FREQ_8KHZ:
-		data += write_ltv_uint8(data, LC3_TYPE_FREQ, LC3_CONFIG_FREQ_8KHZ);
-		break;
-	default:
-		return -EINVAL;
-	}
-	*caps_size += 3;
 
-	data += write_ltv_uint16(data, LC3_TYPE_FRAMELEN, htobs(bap_bcast_qos_configs[index].framelen));
+	if (preset->framing)
-	*caps_size += 4;
-	data += write_ltv_uint8(data, LC3_TYPE_DUR, bap_bcast_qos_configs[index].frame_duration);
-	*caps_size += 3;
-	data += write_ltv_uint32(data, LC3_TYPE_CHAN, htobl(channel_allocation));
-	*caps_size += 6;
-
-	if(bap_bcast_qos_configs[index].framing)
 		qos->framing = 1;
 	else
 		qos->framing = 0;
-	qos->sdu = bap_bcast_qos_configs[index].framelen * get_channel_count(channel_allocation);
+	qos->sdu = preset->framelen * get_channel_count(channel_allocation);
-	qos->retransmission = bap_bcast_qos_configs[index].retransmission;
+	qos->retransmission = preset->retransmission;
-	qos->latency = bap_bcast_qos_configs[index].latency;
+	qos->latency = preset->latency;
-	qos->delay = bap_bcast_qos_configs[index].delay;
+	qos->delay = preset->delay;
 	qos->phy = 2;
-	qos->interval = (bap_bcast_qos_configs[index].frame_duration == LC3_CONFIG_DURATION_7_5 ? 7500 : 10000);
+	qos->interval = (preset->frame_duration == LC3_CONFIG_DURATION_7_5 ? 7500 : 10000);
 
-	return true;
+	ret = ltv_writer_end(&writer);
+	if (ret < 0)
+		return ret;
+	if (ret > UINT8_MAX)
+		return -ENOSPC;
+
+	*caps_size = ret;
+	return 0;
 }
 
 const struct media_codec bap_codec_lc3 = {

spa/plugins/bluez5/bluez5-dbus.c

@@ -5816,6 +5816,7 @@ static void configure_bis(struct spa_bt_monitor *monitor,
 	int sync_cte_type = 0;
 	int sync_timeout = 2000;
 	int timeout = 2000;
+	int ret;
 
 	/* Configure each BIS from a BIG */
 	spa_list_for_each(metadata_entry, &bis->metadata_list, link) {
@@ -5839,7 +5840,12 @@ static void configure_bis(struct spa_bt_monitor *monitor,
 	setting_items[1] = SPA_DICT_ITEM_INIT("preset", bis->qos_preset);
 	settings = SPA_DICT_INIT(setting_items, 2);
 
-	codec->get_bis_config(codec, caps, &caps_size, &settings, &qos);
+	caps_size = sizeof(caps);
+	ret = codec->get_bis_config(codec, caps, &caps_size, &settings, &qos);
+	if (ret < 0) {
+		spa_log_warn(monitor->log, "Getting BIS config failed");
+		return;
+	}
 
 	msg = dbus_message_new_method_call(BLUEZ_SERVICE,
 				object_path,

spa/plugins/bluez5/media-codecs.c

@@ -8,6 +8,8 @@
  *
  */
 
+#include <bluetooth/bluetooth.h>
+
 #include <spa/utils/string.h>
 #include <spa/utils/cleanup.h>
 
@@ -100,6 +102,52 @@ bool media_codec_check_caps(const struct media_codec *codec, unsigned int codec_
 		return ((size_t)res == caps_size);
 }
 
+void ltv_writer_data(struct ltv_writer *w, uint8_t type, void* value, size_t len)
+{
+	struct ltv *ltv;
+	size_t sz = (size_t)w->size + sizeof(struct ltv) + len;
+
+	if (!w->buf || sz > w->max_size || (uint16_t)sz != sz) {
+		w->buf = NULL;
+		return;
+	}
+
+	ltv = SPA_PTROFF(w->buf, w->size, struct ltv);
+	ltv->len = len + 1;
+	ltv->type = type;
+	memcpy(ltv->value, value, len);
+
+	w->size = sz;
+}
+
+void ltv_writer_uint8(struct ltv_writer *w, uint8_t type, uint8_t v)
+{
+	ltv_writer_data(w, type, &v, sizeof(v));
+}
+
+void ltv_writer_uint16(struct ltv_writer *w, uint8_t type, uint16_t value)
+{
+	uint16_t v = htobs(value);
+
+	ltv_writer_data(w, type, &v, sizeof(v));
+}
+
+void ltv_writer_uint32(struct ltv_writer *w, uint8_t type, uint32_t value)
+{
+	uint32_t v = htobl(value);
+
+	ltv_writer_data(w, type, &v, sizeof(v));
+}
+
+int ltv_writer_end(struct ltv_writer *w)
+{
+	if (!w->buf)
+		return -ENOSPC;
+
+	w->buf = NULL;
+	return w->size;
+}
+
 #ifdef CODEC_PLUGIN
 
 struct impl {

spa/plugins/bluez5/media-codecs.h

@@ -93,7 +93,7 @@ struct media_codec {
 				 * called again to parse the remaining data. */
 
 	int (*get_bis_config)(const struct media_codec *codec, uint8_t *caps,
-				uint8_t *caps_size,	struct spa_dict *settings,
+				uint8_t *caps_size, struct spa_dict *settings,
 				struct bap_codec_qos *qos);
 
 	/** If fill_caps is NULL, no endpoint is registered (for sharing with another codec). */
@@ -264,4 +264,24 @@ bool media_codec_check_caps(const struct media_codec *codec, unsigned int codec_
 	const void *caps, size_t caps_size, const struct media_codec_audio_info *info,
 	const struct spa_dict *global_settings);
 
+struct __attribute__((packed)) ltv {
+	uint8_t  len;
+	uint8_t  type;
+	uint8_t  value[];
+};
+
+struct ltv_writer {
+	void *buf;
+	uint16_t size;
+	size_t max_size;
+};
+
+#define LTV_WRITER(ptr, max) ((struct ltv_writer) { .buf = (ptr), .max_size = (max) })
+
+void ltv_writer_data(struct ltv_writer *w, uint8_t type, void* value, size_t len);
+void ltv_writer_uint8(struct ltv_writer *w, uint8_t type, uint8_t v);
+void ltv_writer_uint16(struct ltv_writer *w, uint8_t type, uint16_t value);
+void ltv_writer_uint32(struct ltv_writer *w, uint8_t type, uint32_t value);
+int ltv_writer_end(struct ltv_writer *w);
+
 #endif