Re: [PATCH BlueZ] device: Fix race condition between device connection and disconnection

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

 



Hi Sonny,

On Fri, Aug 21, 2020 at 12:47 AM Sonny Sasaka <sonnysasaka@xxxxxxxxxxxx> wrote:
>
> When Connect() is called and waiting for return, dev_disconnected may be
> called due to MGMT_EV_DEVICE_DISCONNECTED event from kernel. In that
> case reply to client that the connection failed otherwise the dbus
> method will timeout because bluetoothd never replies.
>
> Tested with simulation of a lot of Connect() to bluetooth devices and
> check that error is returned from bluetoothd rather than dbus timeout
> when this race condition happens.
>
> Reviewed-by: Miao-chen Chou <mcchou@xxxxxxxxxxxx>
>
> ---
>  src/device.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/src/device.c b/src/device.c
> index 7b7808405..55e7b4042 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -3006,6 +3006,7 @@ void device_add_connection(struct btd_device *dev, uint8_t bdaddr_type)
>  void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type)
>  {
>         struct bearer_state *state = get_state(device, bdaddr_type);
> +       DBusMessage *reply;
>
>         if (!state->connected)
>                 return;
> @@ -3020,6 +3021,17 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type)
>                 device->disconn_timer = 0;
>         }
>
> +       // This could be executed while the client is waiting for Connect() but
> +       // att_connect_cb has not been invoked.
> +       // In that case reply the client that the connection failed.

Please use C style comment /* */

> +       if (device->connect) {
> +               DBG("connection removed while Connect() is waiting reply");
> +               reply = btd_error_failed(device->connect, "Disconnected early");
> +               g_dbus_send_message(dbus_conn, reply);
> +               dbus_message_unref(device->connect);
> +               device->connect = NULL;
> +       }
> +
>         while (device->disconnects) {
>                 DBusMessage *msg = device->disconnects->data;
>
> --
> 2.26.2
>

Otherwise it looks good.

-- 
Luiz Augusto von Dentz



[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