From: Stanislavs Nilovs <valorcool@xxxxxxxxx>
Commit 4465c577778d812702d752dfd2812e25a2f69b31 has fixed a segfault
which may occur during new GATT Characteristic insertion. However, the
cleanup exercise isn't done before returning.
Bluetoothd address sanitizer backtrace:
=================================================================
==88967==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 768 byte(s) in 6 object(s) allocated from:
#0 0x768d1f8b3ec7 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
#1 0x59f0cac1ea07 in util_malloc src/shared/util.c:46
Indirect leak of 576 byte(s) in 18 object(s) allocated from:
#0 0x768d1f8b3ec7 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
#1 0x59f0cac1ea07 in util_malloc src/shared/util.c:46
Indirect leak of 114 byte(s) in 6 object(s) allocated from:
#0 0x768d1f8b4097 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x59f0cac75d6c in new_attribute src/shared/gatt-db.c:222
SUMMARY: AddressSanitizer: 1458 byte(s) leaked in 30 allocation(s).
Additionally added an explicit check that current Characteristic handle
is not greater or equal to its value handle. If value handle is equal to
Characteristic handle, then second "new_attribute" allocation will
overwrite the pointer in service->attributes, which will also cause a
leak.
---
src/shared/gatt-db.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index b67bf89f9..02b49e94b 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -969,6 +969,10 @@ service_insert_characteristic(struct gatt_db_service *service,
if (handle == UINT16_MAX)
return NULL;
+ /* Value handle can't be the same or less than Characteristic handle per reasons above */
+ if(value_handle <= handle)
+ return NULL;
+
i = service_get_attribute_index(service, &handle, 1);
if (!i)
return NULL;
@@ -1009,8 +1013,11 @@ service_insert_characteristic(struct gatt_db_service *service,
/* Update handle of characteristic value_handle if it has changed */
put_le16(value_handle, &value[1]);
- if (!(*chrc) || !(*chrc)->value)
+ if (!(*chrc)->value) {
+ free(*chrc);
+ *chrc = NULL;
return NULL;
+ }
if (memcmp((*chrc)->value, value, len))
memcpy((*chrc)->value, value, len);
--
2.48.1
