Return-Path: <linux-bluetooth-owner@vger.kernel.org>
From: Anupam Roy <anupam.r@samsung.com>
To: linux-bluetooth@vger.kernel.org
Cc: sachin.dev@samsung.com, Anupam Roy <anupam.r@samsung.com>
Subject: Re: [PATCH] client: Fix segmentation fault while fetching
 advertising data
Date: Thu, 26 Oct 2017 21:17:38 +0530
Message-id: <1509032858-24392-1-git-send-email-anupam.r@samsung.com>
References: <CGME20171026154747epcas1p1b6c3132b2a29c09dcbbeb609b3abb34d@epcas1p1.samsung.com>
Sender: linux-bluetooth-owner@vger.kernel.org
List-ID: <linux-bluetooth.vger.kernel.org>

Hi Luiz,

On 26 Oct 2017 7:30 pm, "Luiz Augusto von Dentz"
<luiz.dentz@gmail.com> wrote:
>Hi Anupam,
>
>On Thu, Oct 26, 2017 at 3:29 PM, Luiz Augusto von Dentz
><luiz.dentz@gmail.com> wrote:
>> Hi Anupam,
>>
>> On Wed, Oct 25, 2017 at 9:39 AM, Anupam Roy <anupam.r@samsung.com> 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