[PATCH BlueZ v1 0/1] Fix memory leak when adding GATT Characteristic

[valorcool@gmail.com]

From: Stanislavs Nilovs <valorcool@gmail.com>

Hello,

As a self-education of Bluetooth world and future usage at work, I have
written small GATT Server project. It doesn't have a lot of stuff (like
authentication) yet, but basic functionality like read/write was tested
with gatttool on PC and some BLE app on my Android phone.

Recently I started to integrate it into propriatary Bluetooth stack at work.
When I started testing with BlueZ daemon - it segfaulted each time I was
trying to pair over BLE. When pairing over BLE BlueZ is reading GATT
information from remote device. After further debugging turns out I wrongly
assumed it is allowed by the specification for GATT Characteristic Value 
handle to be the same as GATT Characteristic Handle. However, this
didn't change the fact that BlueZ doesn't handle this case
properly and crashes.

Looking at the master code, the 4465c577778d812702d752dfd2812e25a2f69b31
commit has fixed a segfault as it added a check for Characteristic value
to not be NULL. However, there are still problems left after this fix:

First of all, the reason for segfault to appear was the overwrite of the
original attribute pointer with new one, which didn't had value pointer
allocated. With the above commit it may still happen, but we exit early,
before segfault appears in memcmp.

Second, the change of above commit is not freeing memory on error as it
is done in other error checks above in the "service_insert_characteristic"
function.

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).

The patch fixes abscent cleanup as well as adds additional check to exit
early if value handle is below or equal to Characteristic handle.

Stanislavs Nilovs (1):
  Fix memory leak when adding GATT Characteristic

 src/shared/gatt-db.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

-- 
2.48.1