2020-05-26 14:37:52

by Andrii Batyiev

[permalink] [raw]
Subject: [BUG] bluez: impossible to start/stop discovery over dbus if the first discovery have failed

Hello everyone,

There is an interesting bug that causes StartDiscovery dbus method to
permanently
per connected dbus client. The root cause of it is incorrect
processing of errors
in src/adapter.c:start_discovery_complete function.

Steps:
0. bluetooth adapter should be powered down
1. start bluetoothctl and issue 'power on'
2. in separate console issue 'hcitool inq' (or otherwise make adapter BUSY)
3. return back to bluetoothctl and issue 'scan on'
4. there would and error 'Failed to start discovery: org.bluez.Error.InProgress'
5. wait until 'hcitool inq' completes
6. issue 'scan on' again
7. there would be same error 'Failed to start discovery:
org.bluez.Error.InProgress'

Expected:
7. discovery should start

My investigation:
1. in src/adapter.c:start_discovery, there is specific check with comment
"Every client can only start one discovery"
2. src/adapter.c:start_discovery adds new client into 'adapter->discovery_list'
3. in src/adapter.c:start_discovery_complete, if 'status' is not
MGMT_STATUS_SUCCESS,
the 'btd_error_busy' is sent to client, but client is not removed from
discovery_list
4. apparently, it leads to a situation where bluez thinks that client
have started discovery,
but adapter is not discovering

Personally, I'm not sure how to fix this bug (should the client be
removed from discovery_list on error?).

Hope it helps to fix the bug.

Thanks,
Andrey


2020-05-28 22:23:48

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [BUG] bluez: impossible to start/stop discovery over dbus if the first discovery have failed

Hi Andrey,

On Tue, May 26, 2020 at 7:39 AM Andrey Batyiev <[email protected]> wrote:
>
> Hello everyone,
>
> There is an interesting bug that causes StartDiscovery dbus method to
> permanently
> per connected dbus client. The root cause of it is incorrect
> processing of errors
> in src/adapter.c:start_discovery_complete function.
>
> Steps:
> 0. bluetooth adapter should be powered down
> 1. start bluetoothctl and issue 'power on'
> 2. in separate console issue 'hcitool inq' (or otherwise make adapter BUSY)
> 3. return back to bluetoothctl and issue 'scan on'
> 4. there would and error 'Failed to start discovery: org.bluez.Error.InProgress'
> 5. wait until 'hcitool inq' completes
> 6. issue 'scan on' again
> 7. there would be same error 'Failed to start discovery:
> org.bluez.Error.InProgress'
>
> Expected:
> 7. discovery should start
>
> My investigation:
> 1. in src/adapter.c:start_discovery, there is specific check with comment
> "Every client can only start one discovery"
> 2. src/adapter.c:start_discovery adds new client into 'adapter->discovery_list'
> 3. in src/adapter.c:start_discovery_complete, if 'status' is not
> MGMT_STATUS_SUCCESS,
> the 'btd_error_busy' is sent to client, but client is not removed from
> discovery_list
> 4. apparently, it leads to a situation where bluez thinks that client
> have started discovery,
> but adapter is not discovering
>
> Personally, I'm not sure how to fix this bug (should the client be
> removed from discovery_list on error?).
>
> Hope it helps to fix the bug.

Thanks for the bug reporting, we should indeed remove the client since
we are replying with an error to StartDiscovery, note though that the
use of hcitool inq in parallel is not recommended since as you
experience first hand it does impact the states on the MGMT.

--
Luiz Augusto von Dentz