Skip to content

[PW_SID:1168573] [v5,1/2] Bluetooth: hci_sync: re-enable legacy advertising on resume - #792

Open
BluezTestBot wants to merge 2 commits into
workflowfrom
1168573
Open

BluezTestBot wants to merge 2 commits into
workflowfrom
1168573

Conversation

@BluezTestBot

Copy link
Copy Markdown

hci_resume_advertising_sync() never re-enables advertising on a
controller without extended advertising.

It restores HCI_ADVERTISING from advertising_old_state, then calls
hci_schedule_adv_instance_sync(), which starts with:

if (hci_dev_test_flag(hdev, HCI_ADVERTISING) && !ext_adv_capable(hdev))
return -EPERM;

The flag it tests was set a few lines earlier by the resume itself, so
the call returns -EPERM without sending anything. The return value is
discarded by every caller, so the failure is silent: advertising_paused
is cleared, HCI_ADVERTISING is set, and the controller is not
advertising.

Observed on a BCM43455 (no extended advertising) after an active scan:
btmon shows the pause disabling advertising and no re-enable in the ten
seconds that follow, the device does not appear in another controller's
scan, and btmgmt info still reports "advertising" in current settings.

Enable instance 0x00 directly in that case, the way reenable_adv_sync()
already does for the same situation. Instances continue to go through
hci_schedule_adv_instance_sync() so the software rotation loop is
rearmed.

This affects every caller of hci_resume_advertising_sync(), not only the
scan path: address resolution changes, accept list updates, discovery
stop, suspend/resume and the end of an LE connection attempt all leave
advertising disabled on such a controller today.

err is initialised because the extended branch leaves it unset when
adv_instances is empty and HCI_LE_ADV_0 is clear.

Fixes: ad383c2 ("Bluetooth: hci_sync: Enable advertising when LL privacy is enabled")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5 btmon
Signed-off-by: Valentin Kindschi valentin.kindschi@fiveco.ch

Changes in v5:

  • New patch. v4 of the scan fix resumed advertising through
    hci_schedule_adv_instance_sync(), which refuses HCI_ADVERTISING on a
    controller without extended advertising, so the resume never sent
    anything. Patch 2 depends on this.

net/bluetooth/hci_sync.c | 10 ++++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

--
2.34.1

hci_resume_advertising_sync() never re-enables advertising on a
controller without extended advertising.

It restores HCI_ADVERTISING from advertising_old_state, then calls
hci_schedule_adv_instance_sync(), which starts with:

  if (hci_dev_test_flag(hdev, HCI_ADVERTISING) && !ext_adv_capable(hdev))
          return -EPERM;

The flag it tests was set a few lines earlier by the resume itself, so
the call returns -EPERM without sending anything. The return value is
discarded by every caller, so the failure is silent: advertising_paused
is cleared, HCI_ADVERTISING is set, and the controller is not
advertising.

Observed on a BCM43455 (no extended advertising) after an active scan:
btmon shows the pause disabling advertising and no re-enable in the ten
seconds that follow, the device does not appear in another controller's
scan, and btmgmt info still reports "advertising" in current settings.

Enable instance 0x00 directly in that case, the way reenable_adv_sync()
already does for the same situation. Instances continue to go through
hci_schedule_adv_instance_sync() so the software rotation loop is
rearmed.

This affects every caller of hci_resume_advertising_sync(), not only the
scan path: address resolution changes, accept list updates, discovery
stop, suspend/resume and the end of an LE connection attempt all leave
advertising disabled on such a controller today.

err is initialised because the extended branch leaves it unset when
adv_instances is empty and HCI_LE_ADV_0 is clear.

Fixes: ad383c2 ("Bluetooth: hci_sync: Enable advertising when LL privacy is enabled")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5 btmon
Signed-off-by: Valentin Kindschi <valentin.kindschi@fiveco.ch>
hci_active_scan_sync() programs a non-resolvable private address with LE
Set Random Address on every active scan start.

BLUETOOTH CORE SPECIFICATION Vol 4, Part E, 7.8.4 says the controller
shall return Command Disallowed (0x0C) for that command while legacy
advertising or scanning is enabled. hci_pause_addr_resolution(), called
just above, only stops advertising when LL privacy is in use, so on a
controller without it the command is issued while advertising is still
on:

  Bluetooth: hci0: Opcode 0x2005 failed: -16

It does not converge either. hdev->random_addr is only set on a
successful command complete, so it stays BDADDR_ANY, and the deferral
added by commit c2994b0 ("Bluetooth: hci_sync: Fix not setting
Random Address when required") requires it to be set. Unlike the
resolvable address a few lines above, which is reused while rpa_valid(),
the non-resolvable one is regenerated and rewritten on every call, so a
controller that refuses the write is asked again at the scan restart
period of about 10 s, for as long as discovery keeps restarting.
Observed on a BCM43455, which has no LL privacy and no extended
advertising, 190 rejections in one capture, every one retrying the same
address:

  < LE Set Random Address  Address: 02:16:91:90:F1:D4 (Non-Resolvable)
  > Command Complete       LE Set Random Address, Command Disallowed
  < LE Set Random Address  Address: 26:90:57:96:9A:3E (Non-Resolvable)
  > Command Complete       LE Set Random Address, Command Disallowed

Pause advertising for the address update, and resume it once the update
is done and before the scan is started. The pause does not outlive the
function: it is taken only when nothing else holds it, and released on
every exit. A failed pause returns before marking anything paused, so
the error path is a no-op.

The resume in the error path is no longer guarded by ll_privacy_capable().
That guard matched a pause taken only under LL privacy; the pause added
here is unconditional, and hci_resume_advertising_sync() returns early
when nothing was paused.

One caveat this widens, raised on the previous posting. When
HCI_ADVERTISING is set, hci_pause_advertising_sync() also clears
HCI_DISCOVERABLE and HCI_LIMITED_DISCOVERABLE and zeroes discov_timeout,
and hci_resume_advertising_sync() restores only HCI_ADVERTISING, so the
discoverable state is lost. It reproduces today on an LL privacy
controller through hci_pause_addr_resolution(); the unconditional pause
makes it reachable without LL privacy as well. hci_suspend_sync() also
pauses unconditionally, so a device that suspends loses the same state
on any controller today. That asymmetry is pre-existing and is left
alone here rather than folded into a scan path fix.

Legacy controllers have one random address register, shared by the
scanner and the advertiser. Restoring the advertiser here can reprogram
it when the advertiser is non-connectable, so the scan then uses that
address rather than the one generated for it. Both are non-resolvable
and neither is linkable to the identity address. With privacy enabled
both roles already share one resolvable address the same way.

With the patch, on the same hardware:

  < LE Set Advertising Enable    Enable: 0x00              Success
  < LE Set Random Address                                  Success
  < LE Set Advertising Parameters                          Success
  < LE Set Advertising Enable    Enable: 0x01              Success
  < LE Set Scan Parameters                                 Success
  < LE Set Scan Enable           Enable: 0x01              Success

The address write is accepted, and advertising is restored before the
scan is started.

Over 119369 btmon records and about 12 minutes of the normal workload of
that device, alternating active and passive scanning with ten outgoing
connection attempts, every LE Set Random Address succeeded and there were
no Command Disallowed responses of any opcode, against one per scan
restart before.

Patch 1 is required: without it the resume added here returns -EPERM on
the controllers this fixes and leaves advertising disabled.

Fixes: 3c44a43 ("Bluetooth: hci_sync: Resume adv with no RPA when active scan")
Cc: stable@vger.kernel.org # depends on "Bluetooth: hci_sync: re-enable legacy advertising on resume"
Assisted-by: Claude:claude-opus-5 btmon
Signed-off-by: Valentin Kindschi <valentin.kindschi@fiveco.ch>
@github-actions

Copy link
Copy Markdown

CheckPatch
Desc: Run checkpatch.pl script
Duration: 1.23 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

VerifyFixes
Desc: Verify Fixes tag format and validity
Duration: 0.11 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

VerifySignedoff
Desc: Verify Signed-off-by chain
Duration: 0.11 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

GitLint
Desc: Run gitlint
Duration: 0.57 seconds
Result: FAIL
Output:

[v5,2/2] Bluetooth: hci_sync: pause advertising for the scan address update

82: B1 Line exceeds max length (101>80): "Cc: stable@vger.kernel.org # depends on "Bluetooth: hci_sync: re-enable legacy advertising on resume""

@github-actions

Copy link
Copy Markdown

SubjectPrefix
Desc: Check subject contains "Bluetooth" prefix
Duration: 0.21 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

BuildKernel
Desc: Build Kernel for Bluetooth
Duration: 25.25 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

CheckAllWarning
Desc: Run linux kernel with all warning enabled
Duration: 31.29 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

CheckSparse
Desc: Run sparse tool with linux kernel
Duration: 30.37 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

BuildKernel32
Desc: Build 32bit Kernel for Bluetooth
Duration: 25.23 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

CheckKernelLLVM
Desc: Build kernel with LLVM + context analysis
Duration: 29.88 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

TestRunnerSetup
Desc: Setup kernel and bluez for test-runner
Duration: 621.61 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

TestRunner_l2cap-tester
Desc: Run l2cap-tester with test-runner
Duration: 17.20 seconds
Result: FAIL
Output:

Total: 111, Passed: 109 (98.2%), Failed: 2, Not Run: 0

Failed Test Cases
L2CAP LE Client, Direct Advertising - Success        Failed       2.018 seconds
L2CAP Ext-Flowctl Client, Direct Advertising - Success Failed       2.046 seconds

@github-actions

Copy link
Copy Markdown

TestRunner_iso-tester
Desc: Run iso-tester with test-runner
Duration: 25.84 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

TestRunner_bnep-tester
Desc: Run bnep-tester with test-runner
Duration: 2.82 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

TestRunner_mgmt-tester
Desc: Run mgmt-tester with test-runner
Duration: 57.60 seconds
Result: FAIL
Output:

Total: 501, Passed: 496 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.019 seconds

@github-actions

Copy link
Copy Markdown

TestRunner_rfcomm-tester
Desc: Run rfcomm-tester with test-runner
Duration: 4.05 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

TestRunner_sco-tester
Desc: Run sco-tester with test-runner
Duration: 7.19 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

TestRunner_ioctl-tester
Desc: Run ioctl-tester with test-runner
Duration: 5.64 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

TestRunner_mesh-tester
Desc: Run mesh-tester with test-runner
Duration: 9.10 seconds
Result: FAIL
Output:

Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    2.601 seconds
Mesh - Send cancel - 2                               Timed out    1.996 seconds

@github-actions

Copy link
Copy Markdown

TestRunner_smp-tester
Desc: Run smp-tester with test-runner
Duration: 4.01 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

TestRunner_userchan-tester
Desc: Run userchan-tester with test-runner
Duration: 3.61 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

TestRunner_6lowpan-tester
Desc: Run 6lowpan-tester with test-runner
Duration: 13.41 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

IncrementalBuild
Desc: Incremental build with the patches in the series
Duration: 29.35 seconds
Result: PASS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants