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

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

[Dan Klishch]

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@gmail.com>
---
 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.

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@gmail.com> 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@gmail.com>
> ---
>  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

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

[Dan Klishch]

Hi Luiz,

On 5/4/26 2:27 PM, Luiz Augusto von Dentz wrote:
> Hi Dan,
> 
> On Thu, Apr 30, 2026 at 12:55 AM Dan Klishch <danilklishch@gmail.com> 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@gmail.com>
>> ---
>>   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.
> 

I did test this indeed. The bluez patch I mentioned:
https://gist.github.com/DanShaders/4d280096a5f0f2f45ce07028cbd36761
(this is yet another AI slop, sorry!; I just wanted something quick for
myself). Without the kernel patch, MGMT_OP_SET_PUBLIC_ADDRESS is just
silently swallowed, adapter is removed, and then the userspace never
sees it again.

Claude says it cannot easily add an in-tree test for the patch without
additional changes in vhci to support setting public address. Should I
tell it to do a 3 commit series (set public addr on vhci; this; a test)
and send as V2?

One more question: seems like CI is angry on me because I included the
`Assisted-by: Claude:claude-opus-4-7` tag as per
https://docs.kernel.org/process/coding-assistants.html . Should I remove
it?

(Resending since I accidentally chose "Reply" instead of "Reply All" the
first time, I'm not very used to mailing lists)