Re: [PATCH BlueZ v1 01/17] shared/gatt: Call bt_att_cancel_all in unref

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Luiz,

> On Thu, Feb 26, 2015 at 12:44 AM, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote:
> Hi Arman,
>
> On Thu, Feb 26, 2015 at 7:13 AM, Arman Uguray <armansito@xxxxxxxxxxxx> wrote:
>> This patch fixes a potential invalid access that can occur if bt_att
>> outlives bt_gatt_client and if there are pending discovery requests
>> when bt_gatt_client_unref is called.
>>
>> This patch fixes this by canceling all ATT operations that are handled
>> by the bt_att in bt_gatt_client_unref. The proper fix, however, is to
>> make the discovery procedures cancelable and to cancel those instead of
>> canceling everything. A TODO has been added to fix this later.
>
> Im not sure we should accept patches that are know to have corner
> cases like this, bt_gatt_client is not the sole user of bt_att so by
> cancelling all we may break other users so I think we should invest
> more time in fixing the problem properly.
>

OK, then I'll look at fixing this issue separately and send it to the
list in its own patch set. So this shouldn't need to block the GATT
server patches.

>> ---
>>  src/shared/gatt-client.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
>> index cc972d6..92e72e2 100644
>> --- a/src/shared/gatt-client.c
>> +++ b/src/shared/gatt-client.c
>> @@ -1529,6 +1529,15 @@ static void bt_gatt_client_free(struct bt_gatt_client *client)
>>                 bt_att_unregister_disconnect(client->att, client->disc_id);
>>                 bt_att_unregister(client->att, client->notify_id);
>>                 bt_att_unregister(client->att, client->ind_id);
>> +
>> +               /*
>> +                * TODO: If we free bt_gatt_client while there is an ongoing
>> +                * discovery procedure, the discovery callback may cause an
>> +                * invalid access. To avoid this, we cancel all ongoing ATT
>> +                * operations but the proper fix here is to make discovery
>> +                * procedures cancelable.
>> +                */
>> +               bt_att_cancel_all(client->att);
>>                 bt_att_unref(client->att);
>>         }
>>
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz

Arman
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux