Re: [Bluez PATCH v1] adapter: Don't remove device if adapter is powered off

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

Hi Archie,

On Mon, Dec 28, 2020 at 10:34 PM Archie Pusaka <apusaka@google.com> wrote:
>
> From: Archie Pusaka <apusaka@chromium.org>
>
> If adapter is powered off when a device is being removed, there is a
> possibility that the kernel couldn't clean the device's information,
> for example the pairing information. This causes the kernel to
> disagree with the user space about whether the device is paired.
>
> Therefore, to avoid discrepancy we must not proceed to remove the
> device within the user space as well.

This sounds like we have a bug in the kernel, aren't we calling
btd_adapter_remove_bonding or is that failing if the adapter is not
powered? Hmm it does like it:

This command can only be used when the controller is powered.

> Reviewed-by: Daniel Winkler <danielwinkler@google.com>
> ---
>
>  src/adapter.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index ec6a6a64c5..a2abc46706 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1238,6 +1238,14 @@ void btd_adapter_remove_device(struct btd_adapter *adapter,
>  {
>         GList *l;
>
> +       /* Test if adapter is or will be powered off.
> +        * This is to prevent removing the device information only on user
> +        * space, but failing to do so on the kernel.
> +        */
> +       if (!(adapter->current_settings & MGMT_SETTING_POWERED) ||
> +                       (adapter->pending_settings & MGMT_SETTING_POWERED))
> +               return;

We might need to return an error here so we can reply with an error on
Adapter.RemoveDevice.

>         adapter->connect_list = g_slist_remove(adapter->connect_list, dev);
>
>         adapter->devices = g_slist_remove(adapter->devices, dev);
> --
> 2.29.2.729.g45daf8777d-goog
>


-- 
Luiz Augusto von Dentz