Re: [PATCH v1] Bluetooth: mgmt: always reply when a queued mgmt command is cancelled

Hi Shuai,

On Fri, Jul 31, 2026 at 12:29=E2=80=AFAM Shuai Zhang
<[email protected]> wrote:
>
> On boot, the kernel's HCI_AUTO_OFF timer powers a newly probed
> controller back off after 2 seconds if nothing has taken ownership of
> it yet. bluetooth.service is expected to take ownership within that
> window by powering the adapter on, but on slower boots bluetoothd can
> start late enough that the two race: HCI_AUTO_OFF fires and the
> controller is torn down and re-initialised while bluetoothd is still
> in the middle of its own adapter setup sequence.
>
> When that teardown happens, any mgmt command bluetoothd had already
> queued for that adapter is cancelled - but userspace is never told.
> bluez's mgmt command queue is strictly serialised, so a command that
> never receives a reply blocks every command queued after it on that
> adapter indefinitely. In practice this means bluetoothd gets stuck
> before it can register the adapter, and reports no controller being
> available at all, even though the controller itself is healthy and
> fully usable moments later.
>
> Fix this by treating a cancelled command like any other failure
> instead of silently dropping it: always send a reply back to
> userspace, using the existing "cancelled" status value. This mirrors
> how other mgmt commands already behave when they fail for other
> reasons, and lets bluetoothd's normal error handling take over
> (log the failure and continue) instead of hanging forever.
>
> Verified on a QCS6490 rb3gen2 board where this HCI_AUTO_OFF vs.
> bluetooth.service race was reproduced: before this change, hitting
> the race left the adapter permanently unavailable to bluetoothd until
> the service was restarted; after this change, hitting the same race
> no longer affects adapter registration and the controller comes up
> normally.
>
> Signed-off-by: Shuai Zhang <[email protected]>
> ---
>  net/bluetooth/mgmt.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 167d75e34..9031b5d5d 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -301,6 +301,8 @@ static u8 mgmt_errno_status(int err)
>                 return MGMT_STATUS_ALREADY_CONNECTED;
>         case -ENOTCONN:
>                 return MGMT_STATUS_DISCONNECTED;
> +       case -ECANCELED:
> +               return MGMT_STATUS_CANCELLED;
>         }
>
>         return MGMT_STATUS_FAILED;
> @@ -1338,7 +1340,7 @@ static void mgmt_set_powered_complete(struct hci_de=
v *hdev, void *data, int err)
>         struct mgmt_mode *cp;
>
>         /* Make sure cmd still outstanding. */
> -       if (err =3D=3D -ECANCELED || !mgmt_pending_valid(hdev, cmd))
> +       if (!mgmt_pending_valid(hdev, cmd))
>                 return;

On set_powered_sync:

    mutex_lock(&hdev->mgmt_pending_lock);

    /* Make sure cmd still outstanding. */
    if (!__mgmt_pending_listed(hdev, cmd)) {
        mutex_unlock(&hdev->mgmt_pending_lock);
        return -ECANCELED;
    }

Which is why we consider -ECANCELED as a NOP, also if you dig deeper
you will see that hci_power_off -> hci_dev_do_close ->
hci_dev_close_sync -> if (!auto_off) __mgmt_power_off ->
mgmt_pending_foreach(0, hdev, true, cmd_complete_rsp, &match);

So the reason this is not working as intended is because we are not
clearing HCI_AUTO_OFF once we receive Set Powered command only once we
run hci_power_on_sync which create a race where the likes of
bluetoothd will be trying to power the adapter but power_off work is
still pending, so we should probably clear HCI_AUTO_OFF whenever
setting HCI_MGMT as at that point we know there is a user already
there is no point in powering it off.

>         cp =3D cmd->param;
> @@ -5651,9 +5653,6 @@ static void mgmt_remove_adv_monitor_complete(struct=
 hci_dev *hdev,
>         struct mgmt_pending_cmd *cmd =3D data;
>         struct mgmt_cp_remove_adv_monitor *cp;
>
> -       if (status =3D=3D -ECANCELED)
> -               return;
> -
>         hci_dev_lock(hdev);
>
>         cp =3D cmd->param;
> --
> 2.34.1
>


--=20
Luiz Augusto von Dentz