Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Carlos Bilbao <carlos.bilbao@kernel.org>,
	Manivannan Sadhasivam <mani@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Sasha Levin <sashal@kernel.org>,
	kwilczynski@kernel.org, gregkh@linuxfoundation.org,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.18] misc: pci_endpoint_test: Validate BAR index in doorbell test
Date: Mon, 31 Aug 2026 09:24:13 -0400	[thread overview]
Message-ID: <20260831133314.4125787-225-sashal@kernel.org> (raw)
In-Reply-To: <20260831133314.4125787-1-sashal@kernel.org>

From: Carlos Bilbao <carlos.bilbao@kernel.org>

[ Upstream commit 854bd081c7680029d7886689f6bef8f740625fde ]

pci_endpoint_test_doorbell() reads the BAR number directly from an endpoint
test register and uses it as an index into test->bar[].  Add a defensive
bounds check before the dereference: positive values >= PCI_STD_NUM_BARS
are out of range, and NO_BAR (-1) as a negative signed value would slip
past an upper-bound-only check.

Signed-off-by: Carlos Bilbao (Lambda) <carlos.bilbao@kernel.org>
[mani: changed errno to -ERANGE]
Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://patch.msgid.link/20260410230300.135631-2-carlos.bilbao@kernel.org
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

# Stable Backport Analysis: `misc: pci_endpoint_test: Validate BAR index
in doorbell test`

**Local tree:** `v6.18.44-1-g2736c32da98b9` (kernel version **6.18.44**,
6.18.y stable series)

**Commit under review:** `854bd081c7680` (not yet in this tree; present
on `master`)

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject line
**Record:** `[misc: pci_endpoint_test]` `[Validate]` — defensive bounds
check on BAR index used in the doorbell test path.

### Step 1.2: Tags
**Record:**
- **Signed-off-by:** Carlos Bilbao (Lambda), Manivannan Sadhasivam,
  Bjorn Helgaas
- **Link:** https://patch.msgid.link/20260410230300.135631-2-
  carlos.bilbao@kernel.org
- No `Fixes:`, `Reported-by:`, `Cc: stable@vger.kernel.org`, `Tested-
  by:`, or `Reviewed-by:` in the committed message
- Notable: PCI subsystem maintainers (Mani, Bjorn) signed off; no syzbot
  or user bug report

### Step 1.3: Body analysis
**Record:**
- **Bug:** `pci_endpoint_test_doorbell()` reads a BAR number from
  endpoint MMIO (`PCI_ENDPOINT_TEST_DB_BAR`) and uses it unvalidated as
  `test->bar[bar]`.
- **Symptom:** Out-of-range positive values (`>= PCI_STD_NUM_BARS`) or
  `NO_BAR (-1)` cause out-of-bounds indexing before `writel()`.
- **Root cause:** Missing lower/upper bounds check; an upper-bound-only
  check would miss negative values because `bar` is `enum pci_barno`
  (signed, with `NO_BAR = -1`).
- **Version info:** None in the message.

### Step 1.4: Hidden bug fix?
**Record:** Yes — explicitly a memory-safety bounds-check fix, not
cosmetic cleanup. Same bug class as the earlier ioctl underflow fix
(`1ad82f9db13d8`).

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
**Record:**
- **Files:** `drivers/misc/pci_endpoint_test.c` (+5 lines)
- **Function:** `pci_endpoint_test_doorbell()`
- **Scope:** Single-file, surgical fix

### Step 2.2: Code flow change
**Record:**
- **Before:** After re-reading `PCI_ENDPOINT_TEST_DB_BAR`, code
  immediately does `writel(data, test->bar[bar] + addr)`.
- **After:** Validates `bar < BAR_0 || bar >= PCI_STD_NUM_BARS`; logs
  error and returns `-ERANGE` on failure; only then dereferences
  `test->bar[bar]`.
- **Path affected:** Error/safety path inside doorbell test, reached via
  `ioctl(PCITEST_DOORBELL)`.

### Step 2.3: Bug mechanism
**Record:** **Category:** Out-of-bounds array access / buffer overflow.
- `test->bar` is `void __iomem *bar[PCI_STD_NUM_BARS]` (6 elements,
  indices 0–5).
- `pci_endpoint_test_readl()` returns `u32`; assigned to signed `enum
  pci_barno`.
- `bar == -1` (NO_BAR) → array underflow; `bar >= 6` → array overflow.
- Either can yield a garbage pointer passed to `writel()` → kernel oops
  or memory corruption.

### Step 2.4: Fix quality
**Record:** Obviously correct; mirrors the existing ioctl guard (`bar <=
NO_BAR || bar > BAR_5`). Minimal, no API changes. Low regression risk.
Does not add a NULL-bar check (consistent with other paths that validate
index separately).

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
**Record:** Lines 895–897 introduced by `eefb83790a0dd` ("Add doorbell
test case", Frank Li, 2025-07-10). First appeared in **v6.17**. Bug
present since doorbell support landed.

### Step 3.2: Fixes: tag
**Record:** N/A — no `Fixes:` tag. Related introducing commit:
`eefb83790a0dd`, which is an ancestor of this 6.18.44 tree.

### Step 3.3: Related file history
**Record:**
- `eefb83790a0dd` — added doorbell test (v6.17+)
- `1ad82f9db13d8` — fixed ioctl array underflow for user-supplied BAR
  (same `NO_BAR` issue)
- `cc8e391067164`, `384b1b29481e3` — other doorbell-related cleanups
- Fix is standalone (patch 1/2 of a series); patch 2/2 only removes a
  dead register read (cleanup, not required for the bounds fix)

### Step 3.4: Author context
**Record:** Carlos Bilbao is a PCI endpoint contributor. Manivannan
Sadhasivam (PCI endpoint maintainer) applied the series. Dan Carpenter
previously fixed the parallel ioctl-path bug.

### Step 3.5: Dependencies
**Record:** No prerequisites. Doorbell code exists in this tree. `git
apply --check` on the patch succeeds cleanly against 6.18.44.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1: Original discussion
**Record:** `b4 dig -c 854bd081c7680` → https://patch.msgid.link/2026041
0230300.135631-2-carlos.bilbao@kernel.org. Part of **v2 1/2** series.
Manivannan Sadhasivam replied "Applied, thanks!" No explicit stable
nomination found.

### Step 4.2: Reviewers
**Record:** `b4 dig -w`: CC'd to `mani@kernel.org`,
`kwilczynski@kernel.org`, `kishon@kernel.org`, `den@valinux.co.jp`,
`linux-pci@vger.kernel.org`. **Reviewed-by: Koichiro Den** on the
series.

### Step 4.3: Bug reports
**Record:** No external bug report, syzbot report, or crash log.
Proactive defensive fix identified during code review.

### Step 4.4: Series context
**Record:** 2-patch series. Only patch 1/2 (this commit) fixes the OOB
bug. Patch 2/2 removes an unused earlier BAR read.

### Step 4.5: Stable list history
**Record:** Not searched separately; no stable-list discussion found in
the patch thread.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key functions
**Record:** `pci_endpoint_test_doorbell()` (modified), called from
`pci_endpoint_test_ioctl()`.

### Step 5.2: Callers
**Record:**
- `pci_endpoint_test_ioctl()` → `case PCITEST_DOORBELL:` →
  `pci_endpoint_test_doorbell(test)`
- Exposed via `misc_device` (`/dev/pci-endpoint-test.*`) through
  `unlocked_ioctl`
- Selftest: `tools/testing/selftests/pci_endpoint/pci_endpoint_test.c`
  calls `PCITEST_DOORBELL`

### Step 5.3: Callees
**Record:** `pci_endpoint_test_readl/writel`,
`wait_for_completion_timeout`, `writel()` to BAR-mapped MMIO.

### Step 5.4: Reachability
**Record:** Reachable from userspace via `ioctl()` on the misc device.
Requires access to the PCI endpoint test device node (typically root or
delegated permissions). Not triggerable by unprivileged users without
device access. With device access + buggy/malicious endpoint firmware
returning an invalid BAR register value, the OOB path is reachable.

### Step 5.5: Similar patterns
**Record:** Ioctl path already has equivalent validation at line 940:
```940:941:drivers/misc/pci_endpoint_test.c
                if (bar <= NO_BAR || bar > BAR_5)
                        goto ret;
```
Doorbell path lacks this guard — an inconsistency the patch corrects.

---

## PHASE 6: CROSS-REFERENCING AGAINST LOCAL TREE (6.18.44)

### Step 6.1: Buggy code present?
**Record:** **Yes.** `pci_endpoint_test_doorbell()` at lines 895–897
performs unchecked `test->bar[bar]` dereference. Doorbell support
(`eefb83790a0dd`) is an ancestor of HEAD (landed in v6.17, present in
6.18.44).

### Step 6.2: Backport complications
**Record:** **Clean apply.** `git apply --check` passes. No structural
conflicts; line numbers differ but context matches.

### Step 6.3: Related fixes already present?
**Record:** Ioctl underflow fix (`1ad82f9db13d8`) is in this tree. The
doorbell-path BAR validation fix (`854bd081c7680`) is **not** in this
tree (only on `master`).

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

### Step 7.1: Subsystem criticality
**Record:** **drivers/misc** — PCI Endpoint Test driver
(`CONFIG_PCI_ENDPOINT_TEST`). **PERIPHERAL** — host-side test driver for
PCI endpoint development (TI K3, Rockchip, etc.). Not a core subsystem,
but kernel code reachable from userspace ioctl.

### Step 7.2: Activity
**Record:** Actively maintained; multiple recent fixes in the same file
(IRQ range checks, ioctl underflow, integer overflow prevention).

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who is affected
**Record:** Users with `CONFIG_PCI_ENDPOINT_TEST` enabled who run the
doorbell selftest against PCI endpoint hardware. Primarily embedded/SoC
developers, not typical server/desktop workloads.

### Step 8.2: Trigger conditions
**Record:** `ioctl(PCITEST_DOORBELL)` after doorbell enable, when
endpoint MMIO reports `PCI_ENDPOINT_TEST_DB_BAR` outside [0, 5] or as
-1. Unlikely in correct firmware, but possible with bugs or during
bring-up. Requires device-node access.

### Step 8.3: Failure mode severity
**Record:** Out-of-bounds pointer dereference in `writel()` → **kernel
oops / potential memory corruption**. **Severity: HIGH** if triggered;
**likelihood: LOW** (niche driver, privileged access, depends on
endpoint behavior).

### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** Prevents kernel crash from a real OOB bug; aligns
  doorbell path with already-fixed ioctl path.
- **Risk:** Very low — 5-line bounds check, maintainer-reviewed, clean
  apply.
- **Ratio:** Favorable for backport despite narrow exposure.

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence summary

**FOR backport:**
- Real out-of-bounds array index bug (underflow and overflow)
- Can cause kernel oops
- Small (5 lines), obviously correct, applies cleanly
- Buggy code confirmed in 6.18.44 since v6.17 doorbell addition
- PCI maintainers signed off; parallel ioctl fix already in this tree
- Self-contained; no dependencies

**AGAINST backport:**
- Niche test driver (`CONFIG_PCI_ENDPOINT_TEST`)
- Requires privileged device access
- No syzbot/user crash reports
- Invalid BAR comes from endpoint MMIO, not direct userspace input
- Limited production impact

**Unresolved:** None material to the decision.

### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — simple bounds check;
   selftests exercise doorbell path; maintainer applied.
2. Fixes a real bug? **PASS** — OOB array indexing.
3. Important issue? **PASS** — kernel crash on invalid index (HIGH
   severity, lower exposure).
4. Small and contained? **PASS** — 5 lines, one function.
5. No new features/APIs? **PASS**.
6. Can apply to local tree? **PASS** — clean apply verified.

### Step 9.3: Exception categories
**Record:** None (not device ID, quirk, DT, build, or docs). Standard
bug fix.

### Step 9.4: Decision rationale
For **6.18.44**, the doorbell test exists and has had an unchecked BAR
index since v6.17. This is the same class of memory-safety bug already
fixed on the ioctl path in this tree. The fix is minimal, maintainer-
approved, and prevents a kernel oops. Narrow driver scope lowers urgency
but does not negate that it is a real, reachable crash fix in code that
is present and active in this stable tree.

---

## Verification

- **[Phase 1]** Parsed commit `854bd081c7680` message and tags via `git
  show`
- **[Phase 1]** No syzbot/Reported-by; Link tag present; Bjorn Helgaas
  and Mani SOBs confirmed
- **[Phase 2]** Read `pci_endpoint_test_doorbell()` at lines 857–922;
  confirmed unchecked `test->bar[bar]` at line 897
- **[Phase 2]** Confirmed `test->bar[PCI_STD_NUM_BARS]` and `NO_BAR =
  -1` at lines 108–121
- **[Phase 3]** `git blame -L 895,897`: introduced by `eefb83790a0dd`
  (2025-07-10)
- **[Phase 3]** `git log -20 -- drivers/misc/pci_endpoint_test.c`:
  related ioctl fix `1ad82f9db13d8` present
- **[Phase 3]** `git tag --contains eefb83790a0dd`: first in **v6.17**
- **[Phase 3]** `git merge-base --is-ancestor`: doorbell IN tree; fix
  `854bd081c7680` NOT in HEAD; ioctl fix IN HEAD
- **[Phase 4]** `b4 dig -c 854bd081c7680`: lore URL found
- **[Phase 4]** `b4 dig -m /tmp/doorbell_bar.mbox`: v2 1/2 series;
  Reviewed-by Den; Mani "Applied, thanks!"
- **[Phase 4]** No stable nomination in thread
- **[Phase 5]** Traced call chain: `pci_endpoint_test_ioctl` →
  `PCITEST_DOORBELL` → `pci_endpoint_test_doorbell`
- **[Phase 5]** Grep `PCITEST_DOORBELL`: ioctl case at line 974,
  selftest at
  `tools/testing/selftests/pci_endpoint/pci_endpoint_test.c:261`
- **[Phase 5]** Confirmed ioctl bounds check at line 940 (`bar <= NO_BAR
  || bar > BAR_5`)
- **[Phase 6]** `git describe HEAD`: v6.18.44; `make kernelversion`:
  6.18.44
- **[Phase 6]** `git apply --check` on patch: **clean apply**
- **[Phase 6]** Read probe: `test` allocated with `devm_kzalloc` (line
  1019); `test->bar[]` sized `PCI_STD_NUM_BARS`
- **[Phase 8]** `CONFIG_PCI_ENDPOINT_TEST` in `drivers/misc/Kconfig:482`
  — tristate host-side PCI endpoint test driver

**YES**The background lookups finished and match the earlier analysis:

- **Commit hash:** `854bd081c7680` — `misc: pci_endpoint_test: Validate
  BAR index in doorbell test`
- **Location:** on `master` (via merge `ae385ca812325`), **not** in the
  checked-out **6.18.44** tree
- **Local state:** the doorbell path still does an unchecked
  `test->bar[bar]` dereference; the patch applies cleanly here

**Verdict remains YES** for backport to this 6.18.y tree — small bounds-
check fix for a real OOB bug in code that’s already present since v6.17.

 drivers/misc/pci_endpoint_test.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 1c0fd185114fc..43e2282f8a76e 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -893,6 +893,11 @@ static int pci_endpoint_test_doorbell(struct pci_endpoint_test *test)
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_STATUS, 0);
 
 	bar = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_BAR);
+	if (bar < BAR_0 || bar >= PCI_STD_NUM_BARS) {
+		dev_err(dev, "BAR %d reported by endpoint out of range [0, %u]\n",
+			bar, PCI_STD_NUM_BARS - 1);
+		return -ERANGE;
+	}
 
 	writel(data, test->bar[bar] + addr);
 
-- 
2.53.0


  parent reply	other threads:[~2026-08-31 13:40 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260831133314.4125787-1-sashal@kernel.org>
2026-08-31 13:20 ` [PATCH AUTOSEL 6.18-6.12] PCI: plda: Protect root bus removal with rescan lock Sasha Levin
2026-08-31 13:43   ` sashiko-bot
2026-08-31 13:20 ` [PATCH AUTOSEL 6.18-6.12] PCI: Avoid FLR for MediaTek MT7925 WiFi Sasha Levin
2026-08-31 13:45   ` sashiko-bot
2026-08-31 13:21 ` [PATCH AUTOSEL 6.18-5.10] PCI: altera: Protect root bus removal with rescan lock Sasha Levin
2026-08-31 14:05   ` sashiko-bot
2026-08-31 13:21 ` [PATCH AUTOSEL 6.18-6.1] PCI: intel-gw: Enable clock before PHY init Sasha Levin
2026-08-31 14:12   ` sashiko-bot
2026-08-31 13:22 ` [PATCH AUTOSEL 6.18-6.1] PCI/proc: Fix race between pci_proc_init() and pci_bus_add_device() Sasha Levin
2026-08-31 14:27   ` sashiko-bot
2026-08-31 13:22 ` [PATCH AUTOSEL 6.18-5.10] PCI: rockchip: Protect root bus removal with rescan lock Sasha Levin
2026-08-31 14:30   ` sashiko-bot
2026-08-31 13:23 ` [PATCH AUTOSEL 6.18-5.10] PCI: Avoid SBR for Qualcomm WCN6855/WCN7850 WiFi, SDX62/SDX65 modems Sasha Levin
2026-08-31 14:50   ` sashiko-bot
2026-08-31 13:24 ` Sasha Levin [this message]
2026-08-31 15:07   ` [PATCH AUTOSEL 6.18] misc: pci_endpoint_test: Validate BAR index in doorbell test sashiko-bot
2026-08-31 13:25 ` [PATCH AUTOSEL 6.18-6.1] PCI: Wait for device readiness after D3hot -> D0uninitialized transition Sasha Levin
2026-08-31 15:30   ` sashiko-bot
2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-6.1] PCI: switchtec: Add Gen6 Device IDs Sasha Levin
2026-08-31 15:43   ` sashiko-bot
2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-5.10] PCI: mediatek: Protect root bus removal with rescan lock Sasha Levin
2026-08-31 15:44   ` sashiko-bot
2026-08-31 13:27 ` [PATCH AUTOSEL 6.18] PCI: cadence: " Sasha Levin
2026-08-31 16:15   ` sashiko-bot
2026-08-31 13:28 ` [PATCH AUTOSEL 6.18-5.15] ACPI: PCI: Clear _DEP dependencies after PCI root bridge attach Sasha Levin
2026-08-31 16:33   ` sashiko-bot
2026-08-31 13:28 ` [PATCH AUTOSEL 6.18-6.12] PCI/sysfs: Add CAP_SYS_ADMIN check to __resource_resize_store() Sasha Levin
2026-08-31 13:29 ` [PATCH AUTOSEL 6.18-5.10] PCI/sysfs: Use kstrtobool() to parse the ROM attribute input Sasha Levin
2026-08-31 17:00   ` sashiko-bot
2026-08-31 13:29 ` [PATCH AUTOSEL 6.18] PCI: dwc: Protect root bus removal with rescan lock Sasha Levin
2026-08-31 17:09   ` sashiko-bot
2026-08-31 13:31 ` [PATCH AUTOSEL 6.18-5.10] PCI: Stop setting cached power state to 'unknown' on unbind Sasha Levin
2026-08-31 17:28   ` sashiko-bot
2026-08-31 13:31 ` [PATCH AUTOSEL 6.18-5.10] PCI: iproc: Protect root bus removal with rescan lock Sasha Levin
2026-08-31 17:43   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260831133314.4125787-225-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=carlos.bilbao@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox