2021-06-11 08:22:19

by Sebastian Urban

[permalink] [raw]
Subject: [PATCH BlueZ] gatt-server: Flush notify multiple buffer when full and fix overflow

This fixes the calculation of available buffer space in
bt_gatt_server_send_notification and sends pending notifications
immediately when there is no more room to add a notification.

Previously there was a buffer overflow caused by incorrect calculation
of available buffer space: data->offset can equal data->len
from a previous call to this function, leading
(data->len - data->offset) to underflow after data->offset += 2.
---
src/shared/gatt-server.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 970c35f94..e7155f16a 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -1700,20 +1700,34 @@ bool bt_gatt_server_send_notification(struct bt_gatt_server *server,
if (!server || (length && !value))
return false;

- if (multiple)
+ if (multiple) {
data = server->nfy_mult;

+ /* Flush buffered data, if this request hits buffer size limit */
+ if (data && data->offset > 0 && data->len - data->offset < 4 + length) {
+ if (server->nfy_mult->id)
+ timeout_remove(server->nfy_mult->id);
+ notify_multiple(server);
+ data = NULL;
+ }
+ }
+
if (!data) {
data = new0(struct nfy_mult_data, 1);
data->len = bt_att_get_mtu(server->att) - 1;
data->pdu = malloc(data->len);
}

+ if (multiple) {
+ if (data->len - data->offset < 4 + length)
+ return false;
+ } else {
+ if (data->len - data->offset < 2 + length)
+ return false;
+ }
+
put_le16(handle, data->pdu + data->offset);
data->offset += 2;
-
- length = MIN(data->len - data->offset, length);
-
if (multiple) {
put_le16(length, data->pdu + data->offset);
data->offset += 2;
--
2.25.1


2021-06-11 08:46:32

by bluez.test.bot

[permalink] [raw]
Subject: RE: [BlueZ] gatt-server: Flush notify multiple buffer when full and fix overflow

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=498667

---Test result---

Test Summary:
CheckPatch FAIL 0.31 seconds
GitLint PASS 0.11 seconds
Prep - Setup ELL PASS 44.15 seconds
Build - Prep PASS 0.10 seconds
Build - Configure PASS 7.14 seconds
Build - Make PASS 189.01 seconds
Make Check PASS 9.48 seconds
Make Distcheck PASS 207.31 seconds
Build w/ext ELL - Configure PASS 6.78 seconds
Build w/ext ELL - Make PASS 157.09 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
gatt-server: Flush notify multiple buffer when full and fix overflow
WARNING:LONG_LINE_COMMENT: line length of 81 exceeds 80 columns
#28: FILE: src/shared/gatt-server.c:1706:
+ /* Flush buffered data, if this request hits buffer size limit */

WARNING:LONG_LINE: line length of 88 exceeds 80 columns
#29: FILE: src/shared/gatt-server.c:1707:
+ if (data && data->offset > 0 && data->len - data->offset < 4 + length) {

- total: 0 errors, 2 warnings, 38 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] gatt-server: Flush notify multiple buffer when full and fix" has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - PASS
Desc: Run gitlint with rule in .gitlint

##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree

##############################
Test: Make Check - PASS
Desc: Run 'make check'

##############################
Test: Make Distcheck - PASS
Desc: Run distcheck to check the distribution

##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration



---
Regards,
Linux Bluetooth