diff mbox series

[BlueZ,v1] shared/gatt-server: Fix not using correct MTU for responses

Message ID 20240708152823.2726052-1-luiz.dentz@gmail.com (mailing list archive)
State Accepted
Commit 110a8b47a4f159a8239795255b6c1c0edd79e4cd
Headers show
Series [BlueZ,v1] shared/gatt-server: Fix not using correct MTU for responses | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint success Gitlint PASS
tedd_an/BuildEll success Build ELL PASS
tedd_an/BluezMake success Bluez Make PASS
tedd_an/MakeCheck success Bluez Make Check PASS
tedd_an/MakeDistcheck success Make Distcheck PASS
tedd_an/CheckValgrind success Check Valgrind PASS
tedd_an/CheckSmatch warning CheckSparse WARNING src/shared/gatt-server.c:278:25: warning: Variable length array is used.src/shared/gatt-server.c:618:25: warning: Variable length array is used.src/shared/gatt-server.c:716:25: warning: Variable length array is used.src/shared/gatt-server.c:278:25: warning: Variable length array is used.src/shared/gatt-server.c:618:25: warning: Variable length array is used.src/shared/gatt-server.c:716:25: warning: Variable length array is used.src/shared/gatt-server.c:278:25: warning: Variable length array is used.src/shared/gatt-server.c:618:25: warning: Variable length array is used.src/shared/gatt-server.c:716:25: warning: Variable length array is used.
tedd_an/bluezmakeextell success Make External ELL PASS
tedd_an/ScanBuild success Scan Build PASS

Commit Message

Luiz Augusto von Dentz July 8, 2024, 3:28 p.m. UTC 
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Responses shall use the bt_att_channel MTU not the bt_att MTU since the
response shall be send over the same channel as the request.
---
 attrib/gattrib.c         |  8 ++--
 src/shared/att.c         |  5 ++-
 src/shared/att.h         |  2 +-
 src/shared/gatt-client.c |  2 +-
 src/shared/gatt-server.c | 91 +++++++++++++++++++---------------------
 5 files changed, 53 insertions(+), 55 deletions(-)

Comments

bluez.test.bot@gmail.com July 8, 2024, 4:41 p.m. UTC | #1 
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=869329

---Test result---

Test Summary:
CheckPatch                    PASS      0.32 seconds
GitLint                       PASS      0.18 seconds
BuildEll                      PASS      24.65 seconds
BluezMake                     PASS      1673.25 seconds
MakeCheck                     PASS      13.07 seconds
MakeDistcheck                 PASS      178.50 seconds
CheckValgrind                 PASS      253.43 seconds
CheckSmatch                   WARNING   375.14 seconds
bluezmakeextell               PASS      119.80 seconds
IncrementalBuild              PENDING   60.41 seconds
ScanBuild                     PENDING   1085.53 seconds

Details
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/gatt-server.c:278:25: warning: Variable length array is used.src/shared/gatt-server.c:618:25: warning: Variable length array is used.src/shared/gatt-server.c:716:25: warning: Variable length array is used.src/shared/gatt-server.c:278:25: warning: Variable length array is used.src/shared/gatt-server.c:618:25: warning: Variable length array is used.src/shared/gatt-server.c:716:25: warning: Variable length array is used.src/shared/gatt-server.c:278:25: warning: Variable length array is used.src/shared/gatt-server.c:618:25: warning: Variable length array is used.src/shared/gatt-server.c:716:25: warning: Variable length array is used.
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:

##############################
Test: ScanBuild - PENDING
Desc: Run Scan Build
Output:



---
Regards,
Linux Bluetooth
patchwork-bot+bluetooth@kernel.org July 8, 2024, 8:30 p.m. UTC | #2 
Hello:

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

On Mon,  8 Jul 2024 11:28:23 -0400 you wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> Responses shall use the bt_att_channel MTU not the bt_att MTU since the
> response shall be send over the same channel as the request.
> ---
>  attrib/gattrib.c         |  8 ++--
>  src/shared/att.c         |  5 ++-
>  src/shared/att.h         |  2 +-
>  src/shared/gatt-client.c |  2 +-
>  src/shared/gatt-server.c | 91 +++++++++++++++++++---------------------
>  5 files changed, 53 insertions(+), 55 deletions(-)

Here is the summary with links:
  - [BlueZ,v1] shared/gatt-server: Fix not using correct MTU for responses
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=110a8b47a4f1

You are awesome, thank you!
diff mbox series

Patch

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 997af3699c51..1795cd3a72ff 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -237,9 +237,9 @@  static void attrib_callback_result(uint8_t opcode, const void *pdu,
 	free(buf);
 }
 
-static void attrib_callback_notify(struct bt_att_chan *chan, uint8_t opcode,
-					const void *pdu, uint16_t length,
-					void *user_data)
+static void attrib_callback_notify(struct bt_att_chan *chan, uint16_t mtu,
+					uint8_t opcode, const void *pdu,
+					uint16_t length, void *user_data)
 {
 	uint8_t *buf;
 	struct attrib_callbacks *cb = user_data;
@@ -355,7 +355,7 @@  static void client_notify_cb(uint16_t value_handle, const uint8_t *value,
 	if (length)
 		memcpy(buf + 2, value, length);
 
-	attrib_callback_notify(NULL, ATT_OP_HANDLE_NOTIFY, buf, length + 2,
+	attrib_callback_notify(NULL, 0, ATT_OP_HANDLE_NOTIFY, buf, length + 2,
 							user_data);
 }
 
diff --git a/src/shared/att.c b/src/shared/att.c
index 485ef071bb24..a45e9c26801a 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -1009,8 +1009,9 @@  static void handle_notify(struct bt_att_chan *chan, uint8_t *pdu,
 		found = true;
 
 		if (notify->callback)
-			notify->callback(chan, opcode, pdu + 1, pdu_len - 1,
-							notify->user_data);
+			notify->callback(chan, chan->mtu, opcode,
+						pdu + 1, pdu_len - 1,
+						notify->user_data);
 
 		/* callback could remove all entries from notify list */
 		if (queue_isempty(att->notify_list))
diff --git a/src/shared/att.h b/src/shared/att.h
index 6fd78636e49b..53a3f7a2ae98 100644
--- a/src/shared/att.h
+++ b/src/shared/att.h
@@ -35,7 +35,7 @@  int bt_att_get_channels(struct bt_att *att);
 
 typedef void (*bt_att_response_func_t)(uint8_t opcode, const void *pdu,
 					uint16_t length, void *user_data);
-typedef void (*bt_att_notify_func_t)(struct bt_att_chan *chan,
+typedef void (*bt_att_notify_func_t)(struct bt_att_chan *chan, uint16_t mtu,
 					uint8_t opcode, const void *pdu,
 					uint16_t length, void *user_data);
 typedef void (*bt_att_destroy_func_t)(void *user_data);
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 8e4ae7e5e230..b48d739fc609 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -2228,7 +2228,7 @@  static void notify_handler(void *data, void *user_data)
 				value_data->len, notify_data->user_data);
 }
 
-static void notify_cb(struct bt_att_chan *chan, uint8_t opcode,
+static void notify_cb(struct bt_att_chan *chan, uint16_t mtu, uint8_t opcode,
 					const void *pdu, uint16_t length,
 					void *user_data)
 {
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 3a53d5dfde6b..b30ec8c6acc9 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -50,6 +50,7 @@  struct async_read_op {
 	struct bt_gatt_server *server;
 	uint8_t opcode;
 	bool done;
+	uint16_t mtu;
 	uint8_t *pdu;
 	size_t pdu_len;
 	size_t value_len;
@@ -228,7 +229,7 @@  static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
 		 * value is seen.
 		 */
 		if (iter == 0) {
-			data_val_len = MIN(MIN((unsigned)mtu - 6, 251),
+			data_val_len = MIN(MIN((unsigned int)mtu - 6, 251),
 								value.iov_len);
 			pdu[0] = data_val_len + 4;
 			iter++;
@@ -266,15 +267,14 @@  static void gatt_log(struct bt_gatt_server *server, const char *format, ...)
 	va_end(ap);
 }
 
-static void read_by_grp_type_cb(struct bt_att_chan *chan, uint8_t opcode,
-					const void *pdu, uint16_t length,
-					void *user_data)
+static void read_by_grp_type_cb(struct bt_att_chan *chan, uint16_t mtu,
+					uint8_t opcode, const void *pdu,
+					uint16_t length, void *user_data)
 {
 	struct bt_gatt_server *server = user_data;
 	uint16_t start, end;
 	bt_uuid_t type;
 	bt_uuid_t prim, snd;
-	uint16_t mtu = bt_att_get_mtu(server->att);
 	uint8_t rsp_pdu[mtu];
 	uint16_t rsp_len;
 	uint8_t ecode = 0;
@@ -359,11 +359,8 @@  static void read_by_type_read_complete_cb(struct gatt_db_attribute *attr,
 						size_t len, void *user_data)
 {
 	struct async_read_op *op = user_data;
-	struct bt_gatt_server *server = op->server;
-	uint16_t mtu;
 	uint16_t handle;
 
-	mtu = bt_att_get_mtu(server->att);
 	handle = gatt_db_attribute_get_handle(attr);
 
 	/* Terminate the operation if there was an error */
@@ -375,7 +372,7 @@  static void read_by_type_read_complete_cb(struct gatt_db_attribute *attr,
 	}
 
 	if (op->pdu_len == 0) {
-		op->value_len = MIN(MIN((unsigned) mtu - 4, 253), len);
+		op->value_len = MIN(MIN((unsigned int)op->mtu - 4, 253), len);
 		op->pdu[0] = op->value_len + 2;
 		op->pdu_len++;
 	} else if (len != op->value_len) {
@@ -384,7 +381,7 @@  static void read_by_type_read_complete_cb(struct gatt_db_attribute *attr,
 	}
 
 	/* Stop if this would surpass the MTU */
-	if (op->pdu_len + op->value_len + 2 > (unsigned) mtu - 1) {
+	if (op->pdu_len + op->value_len + 2 > (unsigned int) op->mtu - 1) {
 		op->done = true;
 		goto done;
 	}
@@ -395,7 +392,7 @@  static void read_by_type_read_complete_cb(struct gatt_db_attribute *attr,
 
 	op->pdu_len += op->value_len + 2;
 
-	if (op->pdu_len == (unsigned) mtu - 1)
+	if (op->pdu_len == (unsigned int) op->mtu - 1)
 		op->done = true;
 
 done:
@@ -491,9 +488,9 @@  error:
 	async_read_op_destroy(op);
 }
 
-static void read_by_type_cb(struct bt_att_chan *chan, uint8_t opcode,
-					const void *pdu, uint16_t length,
-					void *user_data)
+static void read_by_type_cb(struct bt_att_chan *chan, uint16_t mtu,
+				uint8_t opcode, const void *pdu,
+				uint16_t length, void *user_data)
 {
 	struct bt_gatt_server *server = user_data;
 	uint16_t start, end;
@@ -536,7 +533,8 @@  static void read_by_type_cb(struct bt_att_chan *chan, uint8_t opcode,
 	}
 
 	op = new0(struct async_read_op, 1);
-	op->pdu = malloc(bt_att_get_mtu(server->att));
+	op->mtu = mtu;
+	op->pdu = malloc(mtu);
 	if (!op->pdu) {
 		free(op);
 		ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
@@ -611,13 +609,12 @@  static bool encode_find_info_rsp(struct gatt_db *db, struct queue *q,
 	return true;
 }
 
-static void find_info_cb(struct bt_att_chan *chan, uint8_t opcode,
+static void find_info_cb(struct bt_att_chan *chan, uint16_t mtu, uint8_t opcode,
 					const void *pdu, uint16_t length,
 					void *user_data)
 {
 	struct bt_gatt_server *server = user_data;
 	uint16_t start, end;
-	uint16_t mtu = bt_att_get_mtu(server->att);
 	uint8_t rsp_pdu[mtu];
 	uint16_t rsp_len;
 	uint8_t ecode = 0;
@@ -709,14 +706,13 @@  static void find_by_type_val_att_cb(struct gatt_db_attribute *attrib,
 	data->len += 4;
 }
 
-static void find_by_type_val_cb(struct bt_att_chan *chan, uint8_t opcode,
-					const void *pdu, uint16_t length,
-					void *user_data)
+static void find_by_type_val_cb(struct bt_att_chan *chan, uint16_t mtu,
+					uint8_t opcode, const void *pdu,
+					uint16_t length, void *user_data)
 {
 	struct bt_gatt_server *server = user_data;
 	uint16_t start, end, uuid16;
 	struct find_by_type_val_data data;
-	uint16_t mtu = bt_att_get_mtu(server->att);
 	uint8_t rsp_pdu[mtu];
 	uint16_t ehandle = 0;
 	bt_uuid_t uuid;
@@ -821,8 +817,8 @@  static uint8_t check_length(uint16_t length, uint16_t offset)
 	return 0;
 }
 
-static void write_cb(struct bt_att_chan *chan, uint8_t opcode, const void *pdu,
-					uint16_t length, void *user_data)
+static void write_cb(struct bt_att_chan *chan, uint16_t mtu, uint8_t opcode,
+			const void *pdu, uint16_t length, void *user_data)
 {
 	struct bt_gatt_server *server = user_data;
 	struct gatt_db_attribute *attr;
@@ -908,12 +904,10 @@  static void read_complete_cb(struct gatt_db_attribute *attr, int err,
 	struct async_read_op *op = user_data;
 	struct bt_gatt_server *server = op->server;
 	uint8_t rsp_opcode;
-	uint16_t mtu;
 	uint16_t handle;
 
 	DBG(server, "Read Complete: err %d", err);
 
-	mtu = bt_att_get_mtu(server->att);
 	handle = gatt_db_attribute_get_handle(attr);
 
 	if (err) {
@@ -925,13 +919,14 @@  static void read_complete_cb(struct gatt_db_attribute *attr, int err,
 	rsp_opcode = get_read_rsp_opcode(op->opcode);
 
 	bt_att_chan_send_rsp(op->chan, rsp_opcode, len ? value : NULL,
-					MIN((unsigned int) mtu - 1, len));
+					MIN((unsigned int) op->mtu - 1, len));
 	async_read_op_destroy(op);
 }
 
 static void handle_read_req(struct bt_att_chan *chan,
-				struct bt_gatt_server *server, uint8_t opcode,
-				uint16_t handle, uint16_t offset)
+				struct bt_gatt_server *server, uint16_t mtu,
+				uint8_t opcode, uint16_t handle,
+				uint16_t offset)
 {
 	struct gatt_db_attribute *attr;
 	uint8_t ecode;
@@ -958,6 +953,7 @@  static void handle_read_req(struct bt_att_chan *chan,
 	op->chan = chan;
 	op->opcode = opcode;
 	op->server = bt_gatt_server_ref(server);
+	op->mtu = mtu;
 
 	if (gatt_db_attribute_read(attr, offset, opcode, server->att,
 							read_complete_cb, op))
@@ -972,8 +968,8 @@  error:
 	bt_att_chan_send_error_rsp(chan, opcode, handle, ecode);
 }
 
-static void read_cb(struct bt_att_chan *chan, uint8_t opcode, const void *pdu,
-					uint16_t length, void *user_data)
+static void read_cb(struct bt_att_chan *chan, uint16_t mtu, uint8_t opcode,
+			const void *pdu, uint16_t length, void *user_data)
 {
 	struct bt_gatt_server *server = user_data;
 	uint16_t handle;
@@ -986,10 +982,10 @@  static void read_cb(struct bt_att_chan *chan, uint8_t opcode, const void *pdu,
 
 	handle = get_le16(pdu);
 
-	handle_read_req(chan, server, opcode, handle, 0);
+	handle_read_req(chan, server, mtu, opcode, handle, 0);
 }
 
-static void read_blob_cb(struct bt_att_chan *chan, uint8_t opcode,
+static void read_blob_cb(struct bt_att_chan *chan, uint16_t mtu, uint8_t opcode,
 					const void *pdu, uint16_t length,
 					void *user_data)
 {
@@ -1005,7 +1001,7 @@  static void read_blob_cb(struct bt_att_chan *chan, uint8_t opcode,
 	handle = get_le16(pdu);
 	offset = get_le16(pdu + 2);
 
-	handle_read_req(chan, server, opcode, handle, offset);
+	handle_read_req(chan, server, mtu, opcode, handle, offset);
 }
 
 struct read_mult_data {
@@ -1105,6 +1101,7 @@  error:
 
 static struct read_mult_data *read_mult_data_new(struct bt_gatt_server *server,
 						struct bt_att_chan *chan,
+						uint16_t mtu,
 						uint8_t opcode,
 						uint16_t num_handles)
 {
@@ -1118,16 +1115,16 @@  static struct read_mult_data *read_mult_data_new(struct bt_gatt_server *server,
 	data->server = server;
 	data->num_handles = num_handles;
 	data->cur_handle = 0;
-	data->mtu = bt_att_get_mtu(server->att);
+	data->mtu = mtu;
 	data->length = 0;
 	data->rsp_data = new0(uint8_t, data->mtu - 1);
 
 	return data;
 }
 
-static void read_multiple_cb(struct bt_att_chan *chan, uint8_t opcode,
-					const void *pdu, uint16_t length,
-					void *user_data)
+static void read_multiple_cb(struct bt_att_chan *chan, uint16_t mtu,
+				uint8_t opcode, const void *pdu,
+				uint16_t length, void *user_data)
 {
 	struct bt_gatt_server *server = user_data;
 	struct gatt_db_attribute *attr;
@@ -1141,7 +1138,7 @@  static void read_multiple_cb(struct bt_att_chan *chan, uint8_t opcode,
 		goto error;
 	}
 
-	data = read_mult_data_new(server, chan, opcode, length / 2);
+	data = read_mult_data_new(server, chan, mtu, opcode, length / 2);
 
 	for (i = 0; i < data->num_handles; i++)
 		data->handles[i] = get_le16(pdu + i * 2);
@@ -1304,9 +1301,9 @@  static void prep_write_complete_cb(struct gatt_db_attribute *attr, int err,
 	free(pwcd);
 }
 
-static void prep_write_cb(struct bt_att_chan *chan, uint8_t opcode,
-					const void *pdu, uint16_t length,
-					void *user_data)
+static void prep_write_cb(struct bt_att_chan *chan, uint16_t mtu,
+				uint8_t opcode, const void *pdu,
+				uint16_t length, void *user_data)
 {
 	struct bt_gatt_server *server = user_data;
 	uint16_t handle = 0;
@@ -1436,9 +1433,9 @@  static bool find_no_reliable_characteristic(const void *data,
 	return !prep_data->reliable_supported;
 }
 
-static void exec_write_cb(struct bt_att_chan *chan, uint8_t opcode,
-					const void *pdu, uint16_t length,
-					void *user_data)
+static void exec_write_cb(struct bt_att_chan *chan, uint16_t mtu,
+				uint8_t opcode, const void *pdu,
+				uint16_t length, void *user_data)
 {
 	struct bt_gatt_server *server = user_data;
 	struct exec_data *data;
@@ -1499,9 +1496,9 @@  error:
 	bt_att_chan_send_error_rsp(chan, opcode, ehandle, ecode);
 }
 
-static void exchange_mtu_cb(struct bt_att_chan *chan, uint8_t opcode,
-				const void *pdu, uint16_t length,
-				void *user_data)
+static void exchange_mtu_cb(struct bt_att_chan *chan, uint16_t mtu,
+				uint8_t opcode, const void *pdu,
+				uint16_t length, void *user_data)
 {
 	struct bt_gatt_server *server = user_data;
 	uint16_t client_rx_mtu;