2017-10-26 15:47:38

by Anupam Roy

[permalink] [raw]
Subject: Re: [PATCH] client: Fix segmentation fault while fetching advertising data

Hi Luiz,

On 26 Oct 2017 7:30 pm, "Luiz Augusto von Dentz"
<[email protected]> wrote:
>Hi Anupam,
>
>On Thu, Oct 26, 2017 at 3:29 PM, Luiz Augusto von Dentz
><[email protected]> wrote:
>> Hi Anupam,
>>
>> On Wed, Oct 25, 2017 at 9:39 AM, Anupam Roy <[email protected]> wrote:
>>> While testing advertisement, I encountered Seg fault in client, when bluetoothd
>>> tries to fetch the Adv data set by client. It can happen either while fetching
>>> Manufacturer specific data or Service data. Backtrace is provided below for reference
>>> After fix is applied, advertisement works fine for me. I am sending the following patch
>>> your review. Thank you.
>>>
>>> Passing val instead of &val in dbus_message_iter_append_fixed_array
>>> DBUS API causes segmentation fault while fecthing Manufacturer
>>> data or service data set by client.
>>>
>>> BT Before Fix:
>>> [bluetooth]# set-advertise-name Test
>>> [bluetooth]# set-advertise-uuids 0x1824
>>> [bluetooth]# set-advertise-manufacturer 0x75 0x02 0x03 0x04
>>> [bluetooth]# advertise on
>>>
>>> Program received signal SIGSEGV, Segmentation fault.
>>> in append_array_variant(iter=iter@entry=0x7fffffffd780,
>>> val=val@entry=0x62485a <ad+90>, n_elements=n_elements@entry=3, type=121) at client/advertising.c:178
>>> in dict_append_basic_array(type=121, n_elements=3,
>>> val=0x62485a <ad+90>, key=0x624858 <ad+88>, key_type=113, dict=0x7fffffffd730) at client/advertising.c:205
>>> get_manufacturer_data(property=<optimized out>, iter=0x7fffffffd840,
>>> user_data=<optimized out>) at client/advertising.c:253
>>>
>>> After Fix:
>>> [bluetooth]# set-advertise-name Test
>>> [bluetooth]# set-advertise-uuids 0x1824
>>> [bluetooth]# set-advertise-manufacturer 0x75 0x02 0x03 0x04
>>> [bluetooth]# advertise on
>>> [CHG] Controller 00:19:0E:11:55:44 SupportedInstances: 0x04
>>> [CHG] Controller 00:19:0E:11:55:44 ActiveInstances: 0x01
>>> Advertising object registered
>>> [bluetooth]#
>>> ---
>>> client/advertising.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/client/advertising.c b/client/advertising.c
>>> index 76cee3d..7d98ae3 100644
>>> --- a/client/advertising.c
>>> +++ b/client/advertising.c
>>> @@ -175,7 +175,7 @@ static void append_array_variant(DBusMessageIter *iter, int type, void *val,
>>> type_sig, &array);
>>>
>>> if (dbus_type_is_fixed(type) == TRUE) {
>>> - dbus_message_iter_append_fixed_array(&array, type, val,
>>> + dbus_message_iter_append_fixed_array(&array, type, &val,
>>> n_elements);
>>> } else if (type == DBUS_TYPE_STRING || type == DBUS_TYPE_OBJECT_PATH) {
>>> const char ***str_array = val;
>>> --
>>> 1.9.1
>>
>> Thanks for the patch but the proper fix is to call dict_append_array
>> with correct pointer otherwise this API will not be consistent with
>> libdbus, so we may want to have pointer to &ad->data and then pass its
>> address there.
>
>I went ahead and made the changes suggested above and applied it.
>Thanks for the patch.
>

Thank You for the review and applying the patch!

>--
>Luiz Augusto von Dentz

BR,
Anupam Roy