weliem / bluez_inc

A C library for Bluez (BLE) that hides all DBus communication. It doesn't get easier than this. This library can also be used in C++.
MIT License
86 stars 21 forks source link

Peripheral: Crash after 3 writes to a Characteristic #61

Open ShariqM opened 6 days ago

ShariqM commented 6 days ago

This project is amazing! I spent days looking at other projects that just didn't work. Thank for building this.

Issue After running ./examples/peripheral/peripheral, if a central bluetooth client tries to write 3 times to a characteristic the code will seg fault.

Cause I believe there is a bug where binc_characteristic_set_value() erroneously frees the byteArray pointer that comes from dbus GVariant *params.

Here's what happens on the 3 calls: 1) First call binc_internal_characteristic_method_call calls binc_characteristic_set_value() with a byteArray that comes from this code:

g_variant_get(params, "(@ay@a{sv})", &valueVariant, &optionsVariant);
GByteArray *byteArray = g_variant_get_byte_array(valueVariant);

This causes characteristic->value to point to the dbus variant byteArray

2) Second call Similarly but this time binc_characteristic_set_value() calls g_byte_array_free(characteristic->value, TRUE); because characteristic->value != NULL now which frees the dbus variant byteArray erroneously.

3) Third call System crashes because the dbus variant byte array memory has been freed.

I believe the fix is to modify binc_characteristic_set_value() to set characteristic->value to a newly allocate byte array that is a copy of byteArray. Then it never points to dbus memory. I've attached that fix here: char_write_double_free_fix.txt

I also have a couple questions: In peripheral.c why does on_local_char_read call binc_application_set_char_value. Shouldn't it call binc_application_get_char_value?

If anything the contents of on_local_char_write should actually look like the current contents of on_local_char_read