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

Hi Luiz

Sorry for the delayed response. I was OOO last week.

On 7/31/2026 11:25 PM, Luiz Augusto von Dentz wrote:
> Hi Shuai,
>
> On Fri, Jul 31, 2026 at 12:29 AM 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_dev *hdev, void *data, int err)
>>          struct mgmt_mode *cp;
>>
>>          /* Make sure cmd still outstanding. */
>> -       if (err == -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.

Agreed on mgmt_set_powered_complete, will drop that hunk in v2.

On mgmt_remove_adv_monitor_complete: the __mgmt_power_off() path you
describe only runs when auto_off=0, i.e. when bluetoothd explicitly
closes the adapter during normal operation.

In the race we are fixing, bluetoothd starts late and the HCI_AUTO_OFF
timer expires before it finishes adapter initialisation, so auto_off=1.
In that case __mgmt_power_off() is never called and there is no other
path that replies to the cancelled commands. hci_cmd_sync_clear()
cancels them with -ECANCELED, and the complete callback is the only
place left to send a reply. Silently returning there is what causes
bluetoothd's serialised mgmt queue to stall indefinitely.


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

Thanks,

Shuai

>