[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] Bluetooth: fix Set Public Address on controller in HCI_AUTO_OFF grace period

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 5075 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1087873

---Test result---

Test Summary:
CheckPatch                    FAIL      0.63 seconds
GitLint                       FAIL      0.26 seconds
SubjectPrefix                 PASS      0.09 seconds
BuildKernel                   PASS      25.03 seconds
CheckAllWarning               PASS      27.56 seconds
CheckSparse                   PASS      26.39 seconds
BuildKernel32                 PASS      24.04 seconds
TestRunnerSetup               PASS      523.74 seconds
TestRunner_l2cap-tester       PASS      27.31 seconds
TestRunner_iso-tester         PASS      34.75 seconds
TestRunner_bnep-tester        PASS      6.36 seconds
TestRunner_mgmt-tester        FAIL      109.68 seconds
TestRunner_rfcomm-tester      PASS      9.47 seconds
TestRunner_sco-tester         PASS      14.20 seconds
TestRunner_ioctl-tester       PASS      9.96 seconds
TestRunner_mesh-tester        FAIL      11.22 seconds
TestRunner_smp-tester         PASS      8.64 seconds
TestRunner_userchan-tester    PASS      6.74 seconds
TestRunner_6lowpan-tester     FAIL      8.59 seconds
IncrementalBuild              PASS      23.68 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[RFC] Bluetooth: fix Set Public Address on controller in HCI_AUTO_OFF grace period
WARNING: Non-standard signature: Assisted-by:
#148: 
Assisted-by: Claude:claude-opus-4-7

ERROR: Unrecognized email address: 'Claude:claude-opus-4-7'
#148: 
Assisted-by: Claude:claude-opus-4-7

total: 1 errors, 1 warnings, 0 checks, 14 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14548392.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[RFC] Bluetooth: fix Set Public Address on controller in HCI_AUTO_OFF grace period

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
1: T1 Title exceeds max length (82>80): "[RFC] Bluetooth: fix Set Public Address on controller in HCI_AUTO_OFF grace period"
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 494, Passed: 489 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.096 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    1.834 seconds
Mesh - Send cancel - 2                               Timed out    1.995 seconds
##############################
Test: TestRunner_6lowpan-tester - FAIL
Desc: Run 6lowpan-tester with test-runner
Output:
WARNING: possible circular locking dependency detected
7.0.0-rc7-gb8928b8010b2 #1 Not tainted
------------------------------------------------------
kworker/0:1/11 is trying to acquire lock:
ffff888002735940 ((wq_completion)hci0#2){+.+.}-{0:0}, at: touch_wq_lockdep_map+0x75/0x180

but task is already holding lock:
ffffffff9c44faa0 (rtnl_mutex){+.+.}-{4:4}, at: lowpan_unregister_netdev+0xd/0x30

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #4 (rtnl_mutex){+.+.}-{4:4}:
       lock_acquire+0xf7/0x2c0
       __mutex_lock+0x16b/0x1fc0
       lowpan_register_netdev+0x11/0x30
       chan_ready_cb+0x836/0xd00
       l2cap_recv_frame+0x61bb/0x88e0
       l2cap_recv_acldata+0x790/0xdf0
       hci_rx_work+0x500/0xd00
       process_scheduled_works+0xba7/0x1a90
       worker_thread+0x514/0xbb0
       kthread+0x368/0x490
       ret_from_fork+0x498/0x7e0
       ret_from_fork_asm+0x19/0x30

-> #3 (&chan->lock#3/1){+.+.}-{4:4}:
       lock_acquire+0xf7/0x2c0
       __mutex_lock+0x16b/0x1fc0
       l2cap_chan_connect+0x74e/0x1980
       lowpan_control_write+0x523/0x660
       full_proxy_write+0x10b/0x190
       vfs_write+0x1c0/0xf60
       ksys_write+0xf1/0x1d0
       do_syscall_64+0xa0/0x570
       entry_SYSCALL_64_after_hwframe+0x74/0x7c

-> #2 (&conn->lock){+.+.}-{4:4}:
...
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0


https://github.com/bluez/bluetooth-next/pull/135

---
Regards,
Linux Bluetooth

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)