[PATCH AUTOSEL 6.18] PCI: cadence: Protect root bus removal with rescan lock

[Sasha Levin]

From: Hans Zhang <18255117159@xxxxxxx>

[ Upstream commit 713331969ce89489c84af917058df6d9910cff97 ]

Hold the pci_rescan_remove_lock lock while stopping and removing a root bus
to avoid racing with concurrent rescan or hotplug operations triggered via
sysfs.  Such races may lead to use-after-free issues or system crashes.

Signed-off-by: Hans Zhang <18255117159@xxxxxxx>
Signed-off-by: Manivannan Sadhasivam <mani@xxxxxxxxxx>
[bhelgaas: commit log]
Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
Link: https://patch.msgid.link/20260521161822.132996-2-18255117159@xxxxxxx
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

### Step 1.1: Subject line
**Record:** `[PCI: cadence] [Protect] Root bus removal must hold the
global PCI rescan/remove mutex during teardown.`

### Step 1.2: Commit message tags
**Record:**
- **Fixes:** none
- **Reported-by:** none
- **Tested-by:** none
- **Reviewed-by:** none (in commit message; cover letter references
  sashiko-bot review)
- **Acked-by:** none
- **Link:**
  https://patch.msgid.link/20260521161822.132996-2-18255117159@xxxxxxx
- **Cc: stable:** none
- **Signed-off-by:** Hans Zhang, Manivannan Sadhasivam, Bjorn Helgaas
  (ignore pipeline-added SOBs)

Notable: no syzbot/user crash report; motivation comes from code review
and subsystem locking rules.

### Step 1.3: Commit body analysis
**Record:**
- **Bug:** `cdns_pcie_host_disable()` calls `pci_stop_root_bus()` /
  `pci_remove_root_bus()` without `pci_rescan_remove_lock`.
- **Symptom:** Race with concurrent sysfs PCI rescan or hotplug/remove;
  author cites possible UAF or system crash.
- **Root cause:** Root-bus teardown is not serialized with sysfs-driven
  rescan/remove paths that take the same lock.
- **Version info:** none in commit message.

### Step 1.4: Hidden bug fix detection
**Record:** Yes. Despite “Protect” rather than “fix”, this is a
synchronization bug fix, not cleanup or a feature. It matches the
established pattern used elsewhere in PCI host drivers.

---

## Phase 2: Diff Analysis

### Step 2.1: Change inventory
**Record:**
- **Files:** `drivers/pci/controller/cadence/pcie-cadence-host.c` (+2
  lines)
- **Function:** `cdns_pcie_host_disable()`
- **Scope:** Single-file, surgical fix (2 lines)

### Step 2.2: Code flow change
**Record:**
- **Before:** `pci_stop_root_bus()` → `pci_remove_root_bus()` with no
  lock.
- **After:** `pci_lock_rescan_remove()` wraps those calls, then
  `pci_unlock_rescan_remove()`.
- **Path:** Driver teardown / module unload (`j721e_pcie_remove()`,
  `sg2042_pcie_remove()`).

### Step 2.3: Bug mechanism
**Record:**
- **Category:** Race condition / missing synchronization.
- **Mechanism:** Sysfs rescan/remove holds `pci_rescan_remove_lock` (see
  `rescan_store`, `dev_rescan_store`, `remove_store` in `pci-sysfs.c`).
  Cadence host disable did not, so teardown could interleave with sysfs
  operations on the same bus hierarchy.

### Step 2.4: Fix quality
**Record:**
- **Quality:** High. Matches `pci_host_common_remove()` and
  `mtk_pcie_remove()`.
- **Regression risk:** Very low. Standard mutex already used across PCI
  core and many host drivers.
- **Red flags:** None.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:**
- Unlocked `pci_stop_root_bus()` / `pci_remove_root_bus()` introduced in
  **47f25da6c5ea5** (“PCI: cadence-host: Introduce
  cdns_pcie_host_disable() helper for cleanup”, 2025-04-17).
- Present in this tree (6.18.44).

### Step 3.2: Fixes: tag
**Record:** N/A — no `Fixes:` tag. Bug introduced by 47f25da6c5ea5 when
the helper was added without the lock.

### Step 3.3: Related file history
**Record:**
- **47f25da6c5ea5:** introduced `cdns_pcie_host_disable()`.
- **a2790bf81f0f7:** J721E module support uses the helper on remove.
- **1c72774df0284:** SG2042 driver uses it on remove.
- **1d59d474e1cb7:** probe-side rescan-lock fix with real crash trace
  (related pattern).
- **60e7b5aa85712:** lockdep assert added because this lock is required
  for removal paths.
- Fix commit is **not** merged in this tree yet.

### Step 3.4: Author context
**Record:** Hans Zhang has recent PCI controller patches. Mani
Sadhasivam and Bjorn Helgaas are PCI maintainers. Patch is part of a
9-patch series; cover letter says each patch is independent.

### Step 3.5: Dependencies
**Record:** None.
- `pci_lock_rescan_remove()` / `pci_unlock_rescan_remove()` exist and
  are exported.
- `cdns_pcie_host_disable()` exists in this tree.
- `pcie-cadence.h` includes `<linux/pci.h>`.
- Standalone backport.

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original discussion
**Record:**
- Local mbox/cover: `20260522_18255117159_pci_controller_add_missing_res
  can_lock_around_root_bus_removal.{cover,mbx}`
- Series: **[PATCH 1/9]** for cadence; 9 independent controller-driver
  fixes.
- Cover cites sashiko-bot review asking whether unlocked teardown can
  race.
- `b4 dig` on HEAD did not match this commit (not merged yet).
- lore.kernel.org fetch returned 403 from this environment.

### Step 4.2: Reviewers
**Record:** Cover references sashiko-bot review thread. No explicit
maintainer stable nomination found in local mbox. Signed-off-by includes
PCI maintainers.

### Step 4.3: Bug report
**Record:** No user crash report or syzbot link for cadence
specifically. Precedent: **1d59d474e1cb7** documents a real NULL-deref
crash from the same class of race on the probe/add side.

### Step 4.4: Related patches
**Record:** 8 sibling patches for dwc, altera, brcmstb, iproc, mediatek,
rockchip, vmd, plda. Independent; cadence patch does not require them.

### Step 4.5: Stable list
**Record:** No stable-list discussion found (lore inaccessible; local
mbox has no `Cc: stable`).

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key functions
**Record:** `cdns_pcie_host_disable()` modified.

### Step 5.2: Callers
**Record:**
- `sg2042_pcie_remove()` — platform driver `.remove`
- `j721e_pcie_remove()` — platform driver `.remove` (RC mode)

Both run on driver unbind/module unload; can overlap with root-
privileged sysfs PCI operations.

### Step 5.3: Callees
**Record:** `pci_host_bridge_from_priv()`, `pci_stop_root_bus()`,
`pci_remove_root_bus()`, then `cdns_pcie_host_deinit()`,
`cdns_pcie_host_link_disable()`.

### Step 5.4: Reachability
**Record:**
- Trigger: driver remove/unbind while another context does sysfs
  `rescan` or `remove` on the same PCI hierarchy.
- Sysfs paths are root-accessible; concurrent admin activity during
  driver unload is realistic on embedded systems using Cadence PCIe (TI
  J721E, Sophgo SG2042).

### Step 5.5: Similar patterns
**Record:** Correct pattern already used in:
- `pci_host_common_remove()` (`pci-host-common.c:105-108`)
- `mtk_pcie_remove()` (`pcie-mediatek-gen3.c:1222-1225`)
- Many hotplug/EEH paths

Cadence was an outlier missing the lock.

---

## Phase 6: Cross-Reference Against Local Tree (6.18.44)

### Step 6.1: Buggy code present?
**Record:** **Yes.** Current `cdns_pcie_host_disable()` at lines 618-628
in `pcie-cadence-host.c` lacks the lock. Tree: **v6.18.44** (`git
describe HEAD` → `v6.18.44-1-g2736c32da98b9`).

### Step 6.2: Backport complications
**Record:** Clean apply expected — 2-line addition in one function. No
conflicting changes seen.

### Step 6.3: Related fixes already present?
**Record:** Probe-side lock fix (1d59d474e1cb7) and lockdep assert
(60e7b5aa85712) are in tree. Cadence removal-side fix is **not**.

---

## Phase 7: Subsystem and Maintainer Context

### Step 7.1: Subsystem criticality
**Record:** `drivers/pci/controller/cadence` — **IMPORTANT** (PCI host
controller). Affects embedded platforms with Cadence PCIe RC (J721E,
SG2042, etc.), not universal desktop paths.

### Step 7.2: Subsystem activity
**Record:** Active — recent cadence commits (module support, SG2042,
capability search, ops checks).

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who is affected
**Record:** Users of Cadence PCIe host controller drivers built as
modules or unbound during runtime, especially J721E and SG2042
platforms.

### Step 8.2: Trigger conditions
**Record:**
- Concurrent sysfs PCI rescan/remove and host driver teardown.
- Uncommon but plausible under admin automation or test scripts.
- Root can trigger sysfs paths; unprivileged users cannot directly, but
  crashes affect the whole system.

### Step 8.3: Failure mode severity
**Record:** UAF / kernel crash — **HIGH** (potential **CRITICAL** if
exploited or during production teardown). No cadence-specific crash log,
but PCI core documents this lock requirement and a similar race already
produced a crash on the add path.

### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** Medium-high for affected platforms; prevents a known
  class of PCI race.
- **Risk:** Very low — 2 lines, established pattern.
- **Ratio:** Favorable for backport.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence summary

**FOR backport:**
- Real missing synchronization on PCI root-bus removal.
- Matches documented PCI locking rules and peer drivers.
- Small, obviously correct, standalone.
- Buggy code exists in 6.18.44 since 47f25da6c5ea5.
- Same subsystem already fixed probe-side race with documented crash
  (1d59d474e1cb7).
- lockdep enforcement (60e7b5aa85712) shows maintainers treat this lock
  as mandatory.

**AGAINST backport:**
- No concrete crash report for cadence specifically.
- Race needs concurrent sysfs activity during driver remove.
- Exposure window starts only from April 2025 when
  `cdns_pcie_host_disable()` was added.

**Unresolved:**
- Full lore review thread not readable here (403).
- No Tested-by on the patch itself.

The unresolved items are not enough to override the clear
synchronization defect.

### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — matches existing PCI
   patterns; no Tested-by.
2. Fixes a real bug? **PASS** — missing lock on documented-required
   path.
3. Important issue? **PASS** — race can cause UAF/crash (HIGH).
4. Small and contained? **PASS** — 2 lines, one function.
5. No new features/APIs? **PASS**.
6. Can apply to local tree? **PASS** — prerequisites present; clean
   apply expected.

### Step 9.3: Exception categories
**Record:** None (not quirk/DT/build/docs). Standard bug-fix
synchronization.

### Step 9.4: Decision rationale

For **linux-6.18.y** (this checkout), `cdns_pcie_host_disable()`
performs root-bus teardown without `pci_rescan_remove_lock`, while sysfs
rescan/remove and many other PCI paths hold that lock. That is a real
race with crash/UAF potential, introduced when the helper was added in
47f25da6c5ea5. The fix is minimal, follows `pci_host_common_remove()`
and other host drivers, and needs no dependencies. PCI maintainers have
already treated this lock as mandatory elsewhere in the subsystem.

---

## Verification

- **[Phase 1]** Parsed subject, tags, body from user-provided commit
  message and local mbox.
- **[Phase 1]** Read cover letter in `20260522_18255117159_pci_controlle
  r_add_missing_rescan_lock_around_root_bus_removal.cover`.
- **[Phase 2]** Confirmed diff: +2 lines around `pci_stop_root_bus()` /
  `pci_remove_root_bus()` in `cdns_pcie_host_disable()`.
- **[Phase 2]** Read current `cdns_pcie_host_disable()` at lines 618-628
  — no lock present.
- **[Phase 3]** `git describe HEAD` → v6.18.44; Makefile
  VERSION/PATCHLEVEL/SUBLEVEL = 6.18.44.
- **[Phase 3]** `git blame -L 618,628` → 47f25da6c5ea5 introduced
  unlocked removal.
- **[Phase 3]** `git merge-base --is-ancestor 47f25da6c5ea5 HEAD` →
  ancestor confirmed.
- **[Phase 3]** `git show 1d59d474e1cb7` → probe-side rescan lock fix
  with crash trace.
- **[Phase 3]** `git show 60e7b5aa85712` → lockdep assert on removal
  paths.
- **[Phase 3]** `git log --oneline -S cdns_pcie_host_disable` → callers
  in j721e/sg2042.
- **[Phase 4]** Read local mbox patch 1/9; series is 9 independent
  patches.
- **[Phase 4]** `b4 dig -c HEAD` — no match (commit not merged).
- **[Phase 4]** lore.kernel.org curl — 403 Forbidden (discussion not
  fully verified online).
- **[Phase 5]** Grep callers: `pci-j721e.c:670`, `pcie-sg2042.c:80`.
- **[Phase 5]** Read `pci-sysfs.c` rescan/remove stores — they take
  `pci_lock_rescan_remove()`.
- **[Phase 5]** Read `pci_host_common_remove()` and `mtk_pcie_remove()`
  — correct locking pattern.
- **[Phase 5]** Read `probe.c:3536-3552` — documents mutex requirement
  for rescan/removal.
- **[Phase 6]** Buggy code confirmed present; fix not merged (`git log
  --grep="Protect root bus removal"` — empty).
- **[Phase 6]** `pcie-cadence.h` includes `<linux/pci.h>` — lock APIs
  available.
- **UNVERIFIED:** Whether any production system has hit this specific
  cadence crash (no report found).

**YES**The analysis is done for **linux-6.18.y** (`v6.18.44`).

**Verdict: YES** — backport this commit.

`cdns_pcie_host_disable()` tears down the root bus without
`pci_rescan_remove_lock`, while sysfs rescan/remove and other PCI paths
use that lock. That can race on driver remove/unbind (J721E, SG2042) and
lead to crashes or use-after-free. The fix is two lines, matches
patterns like `pci_host_common_remove()`, and applies cleanly here; the
buggy path has been present since `cdns_pcie_host_disable()` was added
in 47f25da6c5ea5.

 drivers/pci/controller/cadence/pcie-cadence-host.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
index fffd63d6665e8..c8daa5847c0f3 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -620,8 +620,10 @@ void cdns_pcie_host_disable(struct cdns_pcie_rc *rc)
 	struct pci_host_bridge *bridge;
 
 	bridge = pci_host_bridge_from_priv(rc);
+	pci_lock_rescan_remove();
 	pci_stop_root_bus(bridge->bus);
 	pci_remove_root_bus(bridge->bus);
+	pci_unlock_rescan_remove();
 
 	cdns_pcie_host_deinit(rc);
 	cdns_pcie_host_link_disable(rc);
-- 
2.53.0