Re: [RFC PATCH] Bluetooth: fix Set Public Address on controller in HCI_AUTO_OFF grace period

[Luiz Augusto von Dentz]

Hi Dan,

On Thu, Apr 30, 2026 at 12:55 AM Dan Klishch <danilklishch@xxxxxxxxx> wrote:
>
> When mgmt's Set Public Address command (and Set External Configuration)
> stages a configuration change, set_public_address() sets HCI_CONFIG and
> HCI_AUTO_OFF on the device and queues hci_power_on. The intent is for
> the queued power_on to run hci_dev_init_sync() -- which detects
> HCI_CONFIG and calls hdev->set_bdaddr() to program the new address into
> the firmware -- and then re-emit Index Added with the controller back in
> the boot-init grace period (HCI_UP=1, HCI_AUTO_OFF=1) so userspace can
> re-push pending config commands and drive a fresh Set Powered 1 cycle.
>
> The bug bites when the command is issued while the controller is in
> that same grace period itself -- the normal post-boot state, before
> bluetoothd has claimed the controller via Set Powered 1. In that state
> HCI_UP=1 and HCI_AUTO_OFF=1, so hdev_is_powered() is false (the command
> is accepted) but hci_power_on()'s "already up" early-return condition
> HCI_UP && HCI_MGMT && HCI_AUTO_OFF is true. The early-return path runs
> hci_powered_update_sync() -- which is the wrong thing for this case:
> neither hci_dev_init_sync() nor mgmt_index_added() runs. The result:
>
>   - hdev->public_addr is recorded but never reaches the firmware via
>     hdev->set_bdaddr(),
>   - userspace sees Index Removed (from set_public_address) but no
>     Index Added, leaving the controller invisible to mgmt clients;
>     it is not in the configured, unconfigured or extended index list
>     yet remains registered in the kernel.
>
> The other two starting states are fine: HCI_UP=0 falls through to the
> full hci_dev_do_open() path (correct); HCI_UP=1 with HCI_AUTO_OFF=0
> makes hdev_is_powered() true, so set_public_address() rejects with
> MGMT_STATUS_REJECTED before queueing power_on at all.
>
> Fix: at the top of hci_power_on(), if HCI_CONFIG is pending and we are
> in the affected grace-period state (HCI_UP, HCI_MGMT, !HCI_RFKILLED),
> close the device first. The early-return condition then fails and we
> fall through to hci_dev_do_open() -> hci_dev_init_sync(), which honors
> HCI_CONFIG by invoking hdev->set_bdaddr(); the post-open block then
> re-emits Index Added via the existing HCI_CONFIG branch.
>
> hci_dev_close_sync() clears HCI_AUTO_OFF as a side effect of going
> through the regular power-down path, but set_public_address() had set
> it deliberately so the post-reopen state matches the boot-init grace
> period that mgmt clients expect ("treat as new one" per the protocol
> contract). Several mgmt commands -- Set Privacy, Set Wideband Speech,
> Set LE, Set BR/EDR, etc. -- gate on !hdev_is_powered(), and bluetoothd
> expects to push them during this window before issuing Set Powered 1
> itself. Restore HCI_AUTO_OFF after the close so hdev_is_powered() ==
> false again.
>
> This matches the documented behavior for Set Public Address on a
> fully-configured controller ("Once the address has been successfully
> changed an Index Added event will be sent.")
>
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Dan Klishch <danilklishch@xxxxxxxxx>
> ---
>  net/bluetooth/hci_core.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -948,6 +948,14 @@ static void hci_power_on(struct work_struct *work)
>
>         BT_DBG("%s", hdev->name);
>
> +       if (hci_dev_test_flag(hdev, HCI_CONFIG) &&
> +           hci_dev_test_flag(hdev, HCI_MGMT) &&
> +           !hci_dev_test_flag(hdev, HCI_RFKILLED) &&
> +           test_bit(HCI_UP, &hdev->flags)) {
> +               hci_dev_do_close(hdev);
> +               hci_dev_set_flag(hdev, HCI_AUTO_OFF);
> +       }
> +
>         if (test_bit(HCI_UP, &hdev->flags) &&
>             hci_dev_test_flag(hdev, HCI_MGMT) &&
>             hci_dev_test_and_clear_flag(hdev, HCI_AUTO_OFF)) {
>
> ---
>
> Claude was able to fully convince me that the patch and the explanation
> are correct. Moreover, this indeed fixed a problem I had with a custom
> patched bluetoothd that happened to override public-addr in the 2
> second post-boot window. However, both the patch and commit description
> are fully AI-generated.

This doesn't inspire much confidence, though. Have you at least tried
it yourself? Perhaps we need to ask Claude to first add a test to
mgmt-tester to ensure it actually fixes something rather than just
hallucinating. Most models are not actually very critical of their
proposals if there is nothing to validate their claims with.

-- 
Luiz Augusto von Dentz