Return-Path: <linux-bluetooth-owner@vger.kernel.org>
MIME-Version: 1.0
In-Reply-To: <CABBYNZL9o_86J5LcobiJJMN7E8BAOaJAHBUbtF68D1e4=q1qBA@mail.gmail.com>
References: <CGME20171025063956epcas1p226e935dfc7347c738230c125c340766b@epcas1p2.samsung.com>
 <1508913572-10746-1-git-send-email-anupam.r@samsung.com> <CABBYNZL9o_86J5LcobiJJMN7E8BAOaJAHBUbtF68D1e4=q1qBA@mail.gmail.com>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Thu, 26 Oct 2017 16:58:58 +0300
Message-ID: <CABBYNZLqH8tOoBM5U0Yp1C2rjS48tZVVBsz9uW+L8tyYNDt3zw@mail.gmail.com>
Subject: Re: [PATCH BlueZ] client: Fix segmentation fault while fetching
 advertising data
To: Anupam Roy <anupam.r@samsung.com>
Cc: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        sachin.dev@samsung.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
List-ID: <linux-bluetooth.vger.kernel.org>

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.

-- 
Luiz Augusto von Dentz