[PATCH BlueZ] Check whether device is connected before attaching EATT

[PATCH BlueZ] Check whether device is connected before attaching EATT

[Sonny Sasaka]

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;
-- 
2.31.0

RE: [BlueZ] Check whether device is connected before attaching EATT

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=498303

---Test result---

Test Summary:
CheckPatch                    PASS      0.57 seconds
GitLint                       PASS      0.12 seconds
Prep - Setup ELL              PASS      46.51 seconds
Build - Prep                  PASS      0.14 seconds
Build - Configure             PASS      8.12 seconds
Build - Make                  PASS      204.53 seconds
Make Check                    PASS      9.47 seconds
Make Distcheck                PASS      240.18 seconds
Build w/ext ELL - Configure   PASS      8.21 seconds
Build w/ext ELL - Make        PASS      192.78 seconds

Details
##############################
Test: CheckPatch - PASS
Desc: Run checkpatch.pl script with rule in .checkpatch.conf

##############################
Test: GitLint - PASS
Desc: Run gitlint with rule in .gitlint

##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree

##############################
Test: Make Check - PASS
Desc: Run 'make check'

##############################
Test: Make Distcheck - PASS
Desc: Run distcheck to check the distribution

##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration



---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ] Check whether device is connected before attaching EATT

[Luiz Augusto von Dentz]

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