Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210610181756.430-1-sonnysasaka@chromium.org>
In-Reply-To: <20210610181756.430-1-sonnysasaka@chromium.org>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Thu, 10 Jun 2021 13:59:36 -0700
Message-ID: <CABBYNZKG45JktXfxo=SkxrmykpKjTbDu34kz0NpaHhM0CqHTuQ@mail.gmail.com>
Subject: Re: [PATCH BlueZ] Check whether device is connected before attaching EATT
To:     Sonny Sasaka <sonnysasaka@chromium.org>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Sonny,

On Thu, Jun 10, 2021 at 11:20 AM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
>
> Due to a race condition, device_attach_att() may be reached when the
> dev is actually already disconnected but dev->att is not yet cleaned up
> by att_disconnect_cb(). Therefore we should check whether the dev is
> connected before attaching EATT.
>
> The race condition is discovered at rare cases when there is a very
> quick reconnection after disconnection so that device_attach_att() is
> called even before att_disconnect_cb(). This case is more probable to
> happen when the host goes to suspend right before dev_disconnected() is
> invoked and when the host is woken up by a reconnection the reconnection
> is processed earlier than the cleanup in att_disconnect_cb().
>
> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
>
> ---
>  src/device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/device.c b/src/device.c
> index 65838f59f..319a929ee 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -5306,7 +5306,7 @@ bool device_attach_att(struct btd_device *dev, GIOChannel *io)
>                 return false;
>         }
>
> -       if (dev->att) {
> +       if (btd_device_is_connected(dev) && dev->att) {
>                 if (btd_opts.gatt_channels == bt_att_get_channels(dev->att)) {
>                         DBG("EATT channel limit reached");
>                         return false;

Perhaps we should have this check earlier, also there seems to be
something wrong with att_io then, if the device is no longer connected
att_io shall have been unrefed as well.

> --
> 2.31.0
>


-- 
Luiz Augusto von Dentz