Skip to content

[PW_SID:1152930] Bluetooth: hci_core: Fix IRK lookup lifetime races - #658

Open
BluezTestBot wants to merge 1 commit into
workflowfrom
1152930
Open

[PW_SID:1152930] Bluetooth: hci_core: Fix IRK lookup lifetime races#658
BluezTestBot wants to merge 1 commit into
workflowfrom
1152930

Conversation

@BluezTestBot

Copy link
Copy Markdown

The IRK lookup helpers traverse the identity resolving key list under
RCU, but return a raw pointer after leaving the read-side critical
section. A concurrent management unpair or key reload can unlink and
free that entry while SMP key distribution still updates its value and
RPA through hci_add_irk().

RCU also does not serialize list mutations. SMP cleanup and key
distribution can update the IRK list without the hdev mutex while
management paths update it with that mutex held, allowing concurrent
list_add_rcu() and list_del_rcu() operations on the same list.

Give each IRK a list-owned reference and return caller-owned references
from lookup and add helpers. Keep the SMP context reference until pairing
teardown, and drop the list reference only once when an entry is
unlinked. Add a dedicated spinlock for IRK list and payload updates, and
copy payload snapshots under that lock so readers do not race updates.
Initialize new entries completely before publishing them. Unlink an IRK
added during unpair before dropping the SMP context reference.

An exact KASAN interleaving that removes and drains the RCU entry after
lookup but before hci_add_irk() resumes now completes without a
use-after-free. A forced late-add/unpair interleaving leaves no linked IRK
behind. A KASAN and lockdep enabled VHCI pairing/unpair test also
completes successfully.

Fixes: a7ec733 ("Bluetooth: Fix removing any IRKs when unpairing devices")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Kazuki Hanai hnkz.64@gmail.com

include/net/bluetooth/hci_core.h | 19 ++-
net/bluetooth/hci_conn.c | 34 ++++--
net/bluetooth/hci_core.c | 202 +++++++++++++++++++++++++------
net/bluetooth/hci_debugfs.c | 6 +-
net/bluetooth/hci_event.c | 16 ++-
net/bluetooth/hci_sync.c | 16 ++-
net/bluetooth/iso.c | 18 ++-
net/bluetooth/mgmt.c | 12 +-
net/bluetooth/smp.c | 20 ++-
9 files changed, 274 insertions(+), 69 deletions(-)

The IRK lookup helpers traverse the identity resolving key list under
RCU, but return a raw pointer after leaving the read-side critical
section. A concurrent management unpair or key reload can unlink and
free that entry while SMP key distribution still updates its value and
RPA through hci_add_irk().

RCU also does not serialize list mutations. SMP cleanup and key
distribution can update the IRK list without the hdev mutex while
management paths update it with that mutex held, allowing concurrent
list_add_rcu() and list_del_rcu() operations on the same list.

Give each IRK a list-owned reference and return caller-owned references
from lookup and add helpers. Keep the SMP context reference until pairing
teardown, and drop the list reference only once when an entry is
unlinked. Add a dedicated spinlock for IRK list and payload updates, and
copy payload snapshots under that lock so readers do not race updates.
Initialize new entries completely before publishing them. Unlink an IRK
added during unpair before dropping the SMP context reference.

An exact KASAN interleaving that removes and drains the RCU entry after
lookup but before hci_add_irk() resumes now completes without a
use-after-free. A forced late-add/unpair interleaving leaves no linked IRK
behind. A KASAN and lockdep enabled VHCI pairing/unpair test also
completes successfully.

Fixes: a7ec733 ("Bluetooth: Fix removing any IRKs when unpairing devices")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Kazuki Hanai <hnkz.64@gmail.com>
@github-actions

Copy link
Copy Markdown

CheckPatch
Desc: Run checkpatch.pl script
Duration: 2.75 seconds
Result: FAIL
Output:

Bluetooth: hci_core: Fix IRK lookup lifetime races
WARNING: Assisted-by expects 'AGENT_NAME:MODEL_VERSION [TOOL1] [TOOL2]' format
#129: 
Assisted-by: LLM

total: 0 errors, 1 warnings, 0 checks, 661 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/14772457.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.


@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

GitLint
Desc: Run gitlint
Duration: 0.35 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

CheckKernelLLVM
Desc: Build kernel with LLVM + context analysis
Duration: 0.00 seconds
Result: SKIP
Output:

Clang not found

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

TestRunner_l2cap-tester
Desc: Run l2cap-tester with test-runner
Duration: 63.36 seconds
Result: PASS

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

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

Failed Test Cases
Read Exp Feature - Success                           Failed       0.252 seconds

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

TestRunner_mesh-tester
Desc: Run mesh-tester with test-runner
Duration: 26.28 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.232 seconds
Mesh - Send cancel - 2                               Timed out    1.988 seconds

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants