device: fix memory leak

device_add_eir_uuids creates a list of added UUIDs, but it was never freed.

This was found with LeakSanitizer from the following backtrace:

==764182==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 16 byte(s) in 1 object(s) allocated from:

    #0 0x7f3c7db20cb5 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:67
    #1 0x7f3c7d870afa in g_malloc (/usr/lib/libglib-2.0.so.0+0x65afa) (BuildId: 8b07c017773317c7341f72bb8ca4a7a78b323f37)
    #2 0x7f3c7d88e31f in g_slist_append (/usr/lib/libglib-2.0.so.0+0x8331f) (BuildId: 8b07c017773317c7341f72bb8ca4a7a78b323f37)
    #3 0x564fa6ad9153 in device_add_eir_uuids src/device.c:2451
    #4 0x564fa6a6b2ec in btd_adapter_device_found src/adapter.c:7481
    #5 0x564fa6a6c5cd in device_found_callback src/adapter.c:7607
    #6 0x564fa6b9b73d in notify_handler src/shared/mgmt.c:337
    #7 0x564fa6b91ad8 in queue_foreach src/shared/queue.c:207
    #8 0x564fa6b9ba3f in process_notify src/shared/mgmt.c:349
    #9 0x564fa6b9c899 in can_read_data src/shared/mgmt.c:409
    ...
---
 src/device.c | 1 +
 1 file changed, 1 insertion(+)

index c8aaf042f..0842becde 100644
@@ -2453,6 +2453,7 @@ void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
 	}
 
 	device_probe_profiles(dev, added);
+	g_slist_free(added);
 }
 
 static void add_manufacturer_data(void *data, void *user_data)

2.52.0

Clients with an acquired characteristic subscription would free the client during disconnect. When notify_io_destroy then tries to remove the client, it'll use-after-free.

Add another reference when allocating the notify_io and unref it again in notify_io_destroy.

This was found with AddressSanitizer:

==766875==ERROR: AddressSanitizer: heap-use-after-free on address 0x7b7782a31df0 at pc 0x55d19ae87cf0 bp 0x7ffcc28ea0d0 sp 0x7ffcc28ea0c0 READ of size 8 at 0x7b7782a31df0 thread T0

    #0 0x55d19ae87cef in notify_io_destroy src/gatt-client.c:1567
    #1 0x55d19ae83462 in sock_io_destroy src/gatt-client.c:1171
    #2 0x55d19ae83a4b in destroy_sock src/gatt-client.c:1192
    #3 0x55d19ae83c52 in sock_hup src/gatt-client.c:1207
    ...

0x7b7782a31df0 is located 0 bytes inside of 32-byte region [0x7b7782a31df0,0x7b7782a31e10) freed by thread T0 here:

    #0 0x7f4784d1f79d in free /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:51
    #1 0x55d19ae85c57 in notify_client_free src/gatt-client.c:1360
    #2 0x55d19ae85dd5 in notify_client_unref src/gatt-client.c:1372
    #3 0x55d19ae86517 in notify_client_disconnect src/gatt-client.c:1418
    ...

previously allocated by thread T0 here:

    #0 0x7f4784d20cb5 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:67
    #1 0x55d19af6154d in util_malloc src/shared/util.c:46
    #2 0x55d19ae86550 in notify_client_create src/gatt-client.c:1426
    #3 0x55d19ae880cd in characteristic_acquire_notify src/gatt-client.c:1593
    ...

Other avenues could also result in this use-after-free. The root issue is that the client struct is put in to both the notify list, and the notify_io struct, but without an extra reference increment.

---
 src/gatt-client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 44ec95db0..374e67c34 100644
@@ -1566,6 +1566,7 @@ static void notify_io_destroy(void *data)
 
 	if (queue_remove(client->chrc->notify_clients, client))
 		notify_client_unref(client);
+	notify_client_unref(client);
 }
 
 static DBusMessage *characteristic_acquire_notify(DBusConnection *conn,

@@ -1607,7 +1608,7 @@ static DBusMessage *characteristic_acquire_notify(DBusConnection *conn,
 	queue_push_tail(chrc->notify_clients, client);
 
 	chrc->notify_io = new0(struct sock_io, 1);
-	chrc->notify_io->data = client;
+	chrc->notify_io->data = notify_client_ref(client);
 	chrc->notify_io->msg = dbus_message_ref(msg);
 	chrc->notify_io->destroy = notify_io_destroy;
 

2.52.0

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

---Test result---

Test Summary:

CheckPatch                    PENDING   0.31 seconds
GitLint                       PENDING   0.33 seconds
BuildEll                      PASS      18.28 seconds
BluezMake                     PASS      637.05 seconds
MakeCheck                     PASS      22.06 seconds
MakeDistcheck                 PASS      221.04 seconds
CheckValgrind                 PASS      289.24 seconds
CheckSmatch                   PASS      313.20 seconds
bluezmakeextell               PASS      167.38 seconds
IncrementalBuild              PENDING   0.28 seconds
ScanBuild                     PASS      912.45 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series Output:

---
Regards,
Linux Bluetooth

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

---Test result---

Test Summary:

CheckPatch                    PENDING   0.48 seconds
GitLint                       PENDING   0.40 seconds
BuildEll                      PASS      19.78 seconds
BluezMake                     PASS      623.26 seconds
MakeCheck                     PASS      22.20 seconds
MakeDistcheck                 PASS      239.35 seconds
CheckValgrind                 PASS      298.60 seconds
CheckSmatch                   PASS      347.25 seconds
bluezmakeextell               PASS      180.36 seconds
IncrementalBuild              PENDING   0.38 seconds
ScanBuild                     PASS      1005.23 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series Output:

---
Regards,
Linux Bluetooth