[PATCH BlueZ 0/2] shared/bass: Implement CP opcode handlers

[PATCH BlueZ 0/2] shared/bass: Implement CP opcode handlers

[Iulia Tanasescu]

This patch series introduces opcode handlers for the following
BASS Broadcast Audio Scan Control Point opcodes:
   Remote Scan Stopped
   Remote Scan Started
   Remove Source

Iulia Tanasescu (2):
  gatt-server: Check pointer before memcpy
  shared/bass: Implement CP opcode handlers

 src/shared/bass.c        | 155 +++++++++++++++++++++++++++++++++++----
 src/shared/gatt-server.c |   5 +-
 2 files changed, 146 insertions(+), 14 deletions(-)


base-commit: 3030883005c02c77766e1a27a8d5c4d579daa9b5
-- 
2.34.1

[PATCH BlueZ 1/2] gatt-server: Check pointer before memcpy

[Iulia Tanasescu]

This adds a check before calling memcpy inside
bt_gatt_server_send_notification, to avoid getting
the following error in case the user wants to send
an empty notification for an attribute:

src/shared/gatt-server.c:1789:3: runtime error:
null pointer passed as argument 2, which is declared to never be null

---
 src/shared/gatt-server.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 85cff30ec..0512d06f6 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -4,6 +4,7 @@
  *  BlueZ - Bluetooth protocol stack for Linux
  *
  *  Copyright (C) 2014  Google Inc.
+ *  Copyright 2023 NXP
  *
  *
  */
@@ -1785,7 +1786,9 @@ bool bt_gatt_server_send_notification(struct bt_gatt_server *server,
 		length = MIN(data->len - data->offset, length);
 	}
 
-	memcpy(data->pdu + data->offset, value, length);
+	if (value)
+		memcpy(data->pdu + data->offset, value, length);
+
 	data->offset += length;
 
 	if (multiple) {
-- 
2.34.1

[PATCH BlueZ 2/2] shared/bass: Implement CP opcode handlers

[Iulia Tanasescu]

This adds handlers for the following BASS Broadcast Audio Scan
Control Point opcodes:
   Remote Scan Stopped
   Remote Scan Started
   Remove Source

---
 src/shared/bass.c | 155 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 142 insertions(+), 13 deletions(-)

diff --git a/src/shared/bass.c b/src/shared/bass.c
index 8906ca1ef..423ab5bf7 100644
--- a/src/shared/bass.c
+++ b/src/shared/bass.c
@@ -82,6 +82,8 @@ static struct queue *bass_db;
 static struct queue *bass_cbs;
 static struct queue *sessions;
 
+static void bass_bcast_src_free(void *data);
+
 static void bass_debug(struct bt_bass *bass, const char *format, ...)
 {
 	va_list ap;
@@ -385,7 +387,7 @@ static bool bass_check_cp_command_subgroup_data_len(uint8_t num_subgroups,
 	return true;
 }
 
-static bool bass_check_cp_command_len(struct iovec *iov)
+static bool bass_check_cp_command_len(const uint8_t *value, size_t len)
 {
 	struct bt_bass_bcast_audio_scan_cp_hdr *hdr;
 	union {
@@ -395,8 +397,13 @@ static bool bass_check_cp_command_len(struct iovec *iov)
 		struct bt_bass_remove_src_params *remove_src_params;
 	} params;
 
+	struct iovec iov = {
+		.iov_base = (void *)value,
+		.iov_len = len,
+	};
+
 	/* Get command header */
-	hdr = util_iov_pull_mem(iov, sizeof(*hdr));
+	hdr = util_iov_pull_mem(&iov, sizeof(*hdr));
 
 	if (!hdr)
 		return false;
@@ -404,38 +411,38 @@ static bool bass_check_cp_command_len(struct iovec *iov)
 	/* Check command parameters */
 	switch (hdr->op) {
 	case BT_BASS_ADD_SRC:
-		params.add_src_params = util_iov_pull_mem(iov,
+		params.add_src_params = util_iov_pull_mem(&iov,
 						sizeof(*params.add_src_params));
 		if (!params.add_src_params)
 			return false;
 
 		if (!bass_check_cp_command_subgroup_data_len(
 					params.add_src_params->num_subgroups,
-					iov))
+					&iov))
 			return false;
 
 		break;
 	case BT_BASS_MOD_SRC:
-		params.mod_src_params = util_iov_pull_mem(iov,
+		params.mod_src_params = util_iov_pull_mem(&iov,
 						sizeof(*params.mod_src_params));
 		if (!params.mod_src_params)
 			return false;
 
 		if (!bass_check_cp_command_subgroup_data_len(
 					params.mod_src_params->num_subgroups,
-					iov))
+					&iov))
 			return false;
 
 		break;
 	case BT_BASS_SET_BCAST_CODE:
-		params.set_bcast_code_params = util_iov_pull_mem(iov,
+		params.set_bcast_code_params = util_iov_pull_mem(&iov,
 					sizeof(*params.set_bcast_code_params));
 		if (!params.set_bcast_code_params)
 			return false;
 
 		break;
 	case BT_BASS_REMOVE_SRC:
-		params.remove_src_params = util_iov_pull_mem(iov,
+		params.remove_src_params = util_iov_pull_mem(&iov,
 					sizeof(*params.remove_src_params));
 		if (!params.remove_src_params)
 			return false;
@@ -448,25 +455,134 @@ static bool bass_check_cp_command_len(struct iovec *iov)
 		return true;
 	}
 
-	if (iov->iov_len > 0)
+	if (iov.iov_len > 0)
 		return false;
 
 	return true;
 }
 
+static void bass_handle_remote_scan_stopped_op(struct bt_bass_db *bdb,
+					struct gatt_db_attribute *attrib,
+					uint8_t opcode,
+					unsigned int id,
+					struct iovec *iov,
+					struct bt_att *att)
+{
+	if (opcode == BT_ATT_OP_WRITE_REQ)
+		gatt_db_attribute_write_result(attrib, id, 0x00);
+}
+
+static void bass_handle_remote_scan_started_op(struct bt_bass_db *bdb,
+					struct gatt_db_attribute *attrib,
+					uint8_t opcode,
+					unsigned int id,
+					struct iovec *iov,
+					struct bt_att *att)
+{
+	if (opcode == BT_ATT_OP_WRITE_REQ)
+		gatt_db_attribute_write_result(attrib, id, 0x00);
+}
+
+static bool bass_src_id_match(const void *data, const void *match_data)
+{
+	const struct bt_bcast_src *bcast_src = data;
+	const uint8_t *id = match_data;
+
+	return (bcast_src->id == *id);
+}
+
+static void bass_handle_remove_src_op(struct bt_bass_db *bdb,
+					struct gatt_db_attribute *attrib,
+					uint8_t opcode,
+					unsigned int id,
+					struct iovec *iov,
+					struct bt_att *att)
+{
+	struct bt_bass_remove_src_params *params;
+	struct bt_bcast_src *bcast_src;
+
+	/* Get Remove Source command parameters */
+	params = util_iov_pull_mem(iov, sizeof(*params));
+
+	bcast_src = queue_find(bdb->bcast_srcs,
+						bass_src_id_match,
+						&params->id);
+
+	if (!bcast_src) {
+		/* No source matches the written source id */
+		if (opcode == BT_ATT_OP_WRITE_REQ)
+			gatt_db_attribute_write_result(attrib, id,
+					BT_BASS_ERROR_INVALID_SOURCE_ID);
+
+		return;
+	}
+
+	/* Ignore if server is synchronized to the PA
+	 * of the source
+	 */
+	if (bcast_src->sync_state == BT_BASS_SYNCHRONIZED_TO_PA)
+		return;
+
+	/* Ignore if server is synchronized to any BIS
+	 * of the source
+	 */
+	for (int i = 0; i < bcast_src->num_subgroups; i++)
+		if (bcast_src->subgroup_data[i].bis_sync)
+			return;
+
+	/* Accept the operation and remove source */
+	queue_remove(bdb->bcast_srcs, bcast_src);
+	gatt_db_attribute_notify(bcast_src->attr, NULL, 0, att);
+	bass_bcast_src_free(bcast_src);
+
+	if (opcode == BT_ATT_OP_WRITE_REQ)
+		gatt_db_attribute_write_result(attrib, id, 0x00);
+}
+
+#define BASS_OP(_str, _op, _size, _func) \
+	{ \
+		.str = _str, \
+		.op = _op, \
+		.size = _size, \
+		.func = _func, \
+	}
+
+struct bass_op_handler {
+	const char	*str;
+	uint8_t		op;
+	size_t		size;
+	void		(*func)(struct bt_bass_db *bdb,
+				struct gatt_db_attribute *attrib,
+				uint8_t opcode,
+				unsigned int id,
+				struct iovec *iov,
+				struct bt_att *att);
+} bass_handlers[] = {
+	BASS_OP("Remote Scan Stopped", BT_BASS_REMOTE_SCAN_STOPPED,
+		0, bass_handle_remote_scan_stopped_op),
+	BASS_OP("Remote Scan Started", BT_BASS_REMOTE_SCAN_STARTED,
+		0, bass_handle_remote_scan_started_op),
+	BASS_OP("Remove Source", BT_BASS_REMOVE_SRC,
+		0, bass_handle_remove_src_op),
+	{}
+};
+
 static void bass_bcast_audio_scan_cp_write(struct gatt_db_attribute *attrib,
 				unsigned int id, uint16_t offset,
 				const uint8_t *value, size_t len,
 				uint8_t opcode, struct bt_att *att,
 				void *user_data)
 {
+	struct bt_bass_db *bdb = user_data;
+	struct bt_bass_bcast_audio_scan_cp_hdr *hdr;
+	struct bass_op_handler *handler;
 	struct iovec iov = {
 		.iov_base = (void *)value,
 		.iov_len = len,
 	};
 
 	/* Validate written command length */
-	if (!bass_check_cp_command_len(&iov)) {
+	if (!bass_check_cp_command_len(value, len)) {
 		if (opcode == BT_ATT_OP_WRITE_REQ) {
 			gatt_db_attribute_write_result(attrib, id,
 					BT_ERROR_WRITE_REQUEST_REJECTED);
@@ -474,9 +590,22 @@ static void bass_bcast_audio_scan_cp_write(struct gatt_db_attribute *attrib,
 		return;
 	}
 
-	/* TODO: Implement handlers for the written opcodes */
-	gatt_db_attribute_write_result(attrib, id,
-			BT_BASS_ERROR_OPCODE_NOT_SUPPORTED);
+	/* Get command header */
+	hdr = util_iov_pull_mem(&iov, sizeof(*hdr));
+
+	/* Call the appropriate opcode handler */
+	for (handler = bass_handlers; handler && handler->str; handler++) {
+		if (handler->op == hdr->op) {
+			handler->func(bdb, attrib, opcode, id, &iov, att);
+			return;
+		}
+	}
+
+	/* Send error response if unsupported opcode was written */
+	if (opcode == BT_ATT_OP_WRITE_REQ) {
+		gatt_db_attribute_write_result(attrib, id,
+				BT_BASS_ERROR_OPCODE_NOT_SUPPORTED);
+	}
 }
 
 static bool bass_src_match_attrib(const void *data, const void *match_data)
-- 
2.34.1

RE: shared/bass: Implement CP opcode handlers

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=756771

---Test result---

Test Summary:
CheckPatch                    PASS      1.14 seconds
GitLint                       PASS      0.63 seconds
BuildEll                      PASS      32.29 seconds
BluezMake                     PASS      1020.71 seconds
MakeCheck                     PASS      12.55 seconds
MakeDistcheck                 PASS      186.31 seconds
CheckValgrind                 PASS      302.76 seconds
CheckSmatch                   WARNING   405.95 seconds
bluezmakeextell               PASS      123.21 seconds
IncrementalBuild              PASS      1656.48 seconds
ScanBuild                     PASS      1261.57 seconds

Details
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/gatt-server.c:276:25: warning: Variable length array is used.src/shared/gatt-server.c:619:25: warning: Variable length array is used.src/shared/gatt-server.c:718:25: warning: Variable length array is used.src/shared/gatt-server.c:276:25: warning: Variable length array is used.src/shared/gatt-server.c:619:25: warning: Variable length array is used.src/shared/gatt-server.c:718:25: warning: Variable length array is used.src/shared/gatt-server.c:276:25: warning: Variable length array is used.src/shared/gatt-server.c:619:25: warning: Variable length array is used.src/shared/gatt-server.c:718:25: warning: Variable length array is used.


---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ 0/2] shared/bass: Implement CP opcode handlers

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Tue, 13 Jun 2023 17:16:23 +0300 you wrote:
> This patch series introduces opcode handlers for the following
> BASS Broadcast Audio Scan Control Point opcodes:
>    Remote Scan Stopped
>    Remote Scan Started
>    Remove Source
> 
> Iulia Tanasescu (2):
>   gatt-server: Check pointer before memcpy
>   shared/bass: Implement CP opcode handlers
> 
> [...]

Here is the summary with links:
  - [BlueZ,1/2] gatt-server: Check pointer before memcpy
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c0156edd198e
  - [BlueZ,2/2] shared/bass: Implement CP opcode handlers
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=ddd09531e936

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html