* [PATCH AUTOSEL 6.18-6.12] powercap: intel_rapl: Fix kernel panic during PMU unbind
[not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:46 ` Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.6] pmdomain: qcom: rpmhpd: Skip retention by default Sasha Levin
` (5 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2026-10-19 13:46 UTC (permalink / raw)
To: patches, stable
Cc: Sumeet Pawnikar, Abel Vesa, Rafael J. Wysocki, Sasha Levin,
rafael, linux-pm, linux-kernel
From: Sumeet Pawnikar <sumeet4linux@gmail.com>
[ Upstream commit 916b61abba586d950a4d8264e2841cc5ccfa4df1 ]
rapl_package_add_pmu() fails internally at perf_pmu_register(),
and rapl_pmu_update() leaves the global rapl_pmu.pmu structure
zero-initialized via memset and returns an error. But any
previously probed packages retain has_pmu = true.
When the driver is subsequently unbound or removed,
rapl_package_remove_pmu_locked() sees has_pmu == true and
unconditionally calls perf_pmu_unregister(&rapl_pmu.pmu) on the
zeroed-out structure. This attempts a list_del_rcu() on a NULL
list head, immediately causing a kernel panic.
Fix this by checking if the PMU is actually registered before
attempting to unregister it.
Signed-off-by: Sumeet Pawnikar <sumeet4linux@gmail.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
[ rjw: Added empty line after the new conditional ]
Link: https://patch.msgid.link/20260822094657.12489-1-sumeet4linux@gmail.com
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.12.111** (`pending-6.12`). The panic path is
present here; only the later `_locked` wrapper is missing, so the same
two-line check belongs in `rapl_package_remove_pmu`.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[powercap: intel_rapl:] [Fix] [kernel panic during PMU
unbind]**
Subsystem `powercap` / Intel RAPL. Verb is explicitly **fix**. Claimed
intent: stop a panic when the RAPL perf PMU is unbound after a failed
registration.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Reviewed-by:** Abel Vesa `<abel.vesa@oss.qualcomm.com>`
- **Link:** `https://patch.msgid.link/20260822094657.12489-1-
sumeet4linux@gmail.com`
- **Signed-off-by:** Sumeet Pawnikar (author); Rafael J. Wysocki (PM
maintainer)
- **Ignored (pipeline):** Signed-off-by: Sasha Levin (not present on
this candidate text, but would be ignored)
- **No Fixes:** tag (expected)
- **No Cc: stable** (expected)
- **No Reported-by / Tested-by / syzbot**
Notable: PM subsystem maintainer applied it; one external Reviewed-by.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `rapl_pmu_update()` can fail at `perf_pmu_register()` after
`memset` of `rapl_pmu.pmu`. Packages that already succeeded keep
`has_pmu = true`.
- **Symptom:** On unbind/remove, `rapl_package_remove_pmu*()` sees
`has_pmu` and calls `perf_pmu_unregister()` on a PMU whose `list_head`
is NULL → `list_del_rcu()` NULL deref → **kernel panic**.
- **Root cause:** Unregister is gated only on `rp->has_pmu`, not on
whether the global PMU is actually registered.
- **Version info in message:** none. Introduced with RAPL PMU APIs
(verified in Phase 3 as v6.11).
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not hidden.** This is an explicit panic fix, not cleanup
disguised as a fix.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `drivers/powercap/intel_rapl_common.c`
- **Scope:** 2 insertions, 1 deletion (plus blank line from rjw)
- **Function (upstream):** `rapl_package_remove_pmu_locked()`
- **Classification:** single-file surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** If this package has `has_pmu` and no other package still
needs the PMU, always `perf_pmu_unregister(&rapl_pmu.pmu)` then
`memset` the global `rapl_pmu`.
- **After:** Same, but unregister only if `rapl_pmu.registered`.
- **Path:** driver remove / unbind / (later kernels) CPU-hotplug
teardown — error-recovery path after a failed PMU (re)register.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[NULL deref / invalid list_del on unregistered PMU]**
In this tree, `rapl_pmu_update()` (unchanged since 575024a8aa7cf):
```1996:2024:drivers/powercap/intel_rapl_common.c
/* Unregister previous registered PMU */
if (rapl_pmu.registered)
perf_pmu_unregister(&rapl_pmu.pmu);
rapl_pmu.registered = false;
rapl_pmu.domain_map |= rp->domain_map;
memset(&rapl_pmu.pmu, 0, sizeof(struct pmu));
/* ... fill callbacks ... */
ret = perf_pmu_register(&rapl_pmu.pmu, "power", -1);
if (ret) {
pr_info("Failed to register PMU\n");
return ret; /* does NOT set rp->has_pmu; does NOT
restore prior PMU */
}
rapl_pmu.registered = true;
end:
rp->has_pmu = true;
```
If package A already registered, then package B needs extra domains
(documented dual-package Psys case): previous PMU is unregistered,
`pmu.entry` is zeroed, re-register fails. Package A still has `has_pmu
== true`. Remove of A then hits:
```2104:2105:drivers/powercap/intel_rapl_common.c
perf_pmu_unregister(&rapl_pmu.pmu);
memset(&rapl_pmu, 0, sizeof(struct rapl_pmu));
```
`perf_pmu_unregister()` does `list_del_rcu(&pmu->entry)`. A zeroed
`list_head` has `next = prev = NULL`. `__list_del(entry->prev,
entry->next)` writes through NULL → oops.
**Step 2.4: ASSESS THE FIX QUALITY**
Record: **Obviously correct.** `rapl_pmu.registered` is already
maintained in this file (set true only after successful
`perf_pmu_register()`, cleared before re-register). Skipping unregister
when it is false cannot skip a still-registered PMU. Regression risk is
very low. No API/data-structure change.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record: Unconditional `perf_pmu_unregister` in
`rapl_package_remove_pmu()` is from **575024a8aa7cf** (“powercap:
intel_rapl: Introduce APIs for PMU support”, Zhang Rui, 2024-04-28).
`git describe --contains` → **v6.11**. Present in this 6.12 tree (`git
merge-base --is-ancestor 575024a8aa7cf HEAD` succeeded).
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No Fixes: tag. The introducing commit is 575024a8aa7cf; TPMI
started calling it in **963a9ad3c589** (“Enable PMU support”), also in
this tree (v6.11).
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- **35cf088430978** already in this tree (v6.12.110): “Handle PMU
registration failure during probe”. That only logs
`rapl_package_add_pmu()` failure; probe still succeeds; remove still
calls `rapl_package_remove_pmu()`. It does **not** fix the panic.
- No `remove_pmu_locked` commits here.
- Standalone logic; not patch X/Y of a series that 6.12 needs.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Sumeet Pawnikar has several powercap fixes in this tree,
including 35cf088 (already backported). Not the RAPL maintainer; Zhang
Rui / Rafael Wysocki own the subsystem. Rafael applied this patch as
7.3-rc material.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Upstream applies the check in
`rapl_package_remove_pmu_locked()`, added later (~6.19) by “Fix possible
recursive lock warning” (`dcd0b625`) together with MSR RAPL PMU. **`git
merge-base --is-ancestor dcd0b625… HEAD` failed** — that refactor is
**not** in 6.12. It is **not required**: 6.12 already has
`rapl_pmu.registered` and the same unregister in
`rapl_package_remove_pmu()`. Standalone one-line semantic backport.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c` cannot be used on this SHA (commit is not in the
checkout). Related commit 35cf088 was found as v3 on lore.
- Original: 2026-07-26 (linux-pm). Resend: 2026-08-22, msgid
`20260822094657.12489-1-sumeet4linux@gmail.com`.
- URLs: https://lists.openwall.net/linux-kernel/2026/08/22/310 and
https://lkml.iu.edu/2608.2/11624.html
- Abel Vesa: “Looks OK to me” + Reviewed-by. Rafael: “Applied as 7.3-rc
material, thanks!”
- No NAKs. Same 3-line diff from v1 through resend. No reviewer request
for Cc: stable.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record: To/Cc: Rafael J. Wysocki, linux-pm, linux-kernel. Reviewer Abel
Vesa. Applied by the PM maintainer. Appropriate audience.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot/bugzilla. Mechanism is explained in the commit
(list_del_rcu on NULL). Author of the sibling probe-failure patch, so
this is from the same investigation, not theoretical cleanup.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Sibling **35cf088** already in 6.12.110. This panic fix is the
missing half of that failure handling. Not a multi-patch series that
6.12 lacks.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable returned 403. Open web search found no
stable-list rejection. **UNVERIFIED:** whether anyone already asked for
this on stable@ (does not change the technical verdict).
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: Upstream: `rapl_package_remove_pmu_locked`. This tree:
`rapl_package_remove_pmu`. Related: `rapl_pmu_update`,
`rapl_package_add_pmu`.
**Step 5.2: TRACE CALLERS**
Record:
- `intel_rapl_tpmi_remove()` → `rapl_package_remove_pmu()` (only caller
in 6.12; MSR driver does **not** use these APIs here).
- `intel_rapl_tpmi_probe()` → `rapl_package_add_pmu()` →
`rapl_pmu_update()`. Probe **continues** if add_pmu fails (after
35cf088, still `return 0`).
- Remove runs on unbind, rmmod, or TPMI device teardown — process
context, not IRQ.
**Step 5.3: TRACE CALLEES**
Record: `perf_pmu_unregister()` → `list_del_rcu(&pmu->entry)` under
`pmus_lock`, then `synchronize_srcu` / `synchronize_rcu`, sysfs
teardown. `list_add_rcu(&pmu->entry, &pmus)` happens only at the **end**
of successful `perf_pmu_register()`. Failed register never links
`entry`.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Reachable on Intel TPMI RAPL (`CONFIG_INTEL_RAPL_TPMI` +
`CONFIG_PERF_EVENTS`): Meteor Lake / Lunar Lake / Arrow Lake class
platforms that 6.12 actually ships on. Trigger: successful PMU add, then
a later package’s `rapl_pmu_update()` re-register fails, then driver
remove. Unprivileged users typically cannot unbind, but
rmmod/unbind/hot-unplug and failed second-package probe are real
admin/boot paths. Documented dual-package Psys re-register is exactly
this update path.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same file already checks `rapl_pmu.registered` before unregister
**inside** `rapl_pmu_update()`. Remove path is the missing twin of that
check. No other RAPL unregister site in 6.12.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** `drivers/powercap/intel_rapl_common.c` lines 2089–2106:
`rapl_package_remove_pmu()` still calls `perf_pmu_unregister` with only
a `has_pmu` guard. `struct rapl_pmu { … bool registered; }` exists at
1574–1579. Bug since **v6.11** (575024a8aa7cf + 963a9ad3c589).
Local tree: `git describe HEAD` = `p-6.12-3-gc504cf0baa3d8`, `make
kernelversion` = **6.12.111**, branch `pending-6.12`.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Minor context rework, not a semantic rewrite.**
- Function name: `rapl_package_remove_pmu` vs upstream `_locked`
- Extra `guard(cpus_read_lock)()` still in the 6.12 function
- `EXPORT_SYMBOL_GPL` vs `EXPORT_SYMBOL_NS_GPL`
The two-line `if (rapl_pmu.registered)` change applies in place.
`registered` already exists.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: 35cf088 is here and only adds a `dev_info`. **This panic is not
already fixed.** `git log --grep='panic during PMU'` on this path is
empty.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **drivers/powercap (Intel RAPL)** — **IMPORTANT** for Intel
platforms with TPMI RAPL + perf, not core-mm/VFS. Panic still takes down
the machine.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Mature driver; PMU APIs added in v6.11. Moderate churn (platform
IDs, locking, leak fixes). This is a leftover hole in the v6.11 PMU
feature, not a brand-new 7.x path.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Config/driver-specific:** `CONFIG_INTEL_RAPL_TPMI` +
`CONFIG_PERF_EVENTS` on Intel TPMI RAPL hardware (client MTL/LNL/ARL and
any TPMI Xeon). Not universal, not staging.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: (1) At least one package successfully added the PMU
(`has_pmu=true`, `registered=true`). (2) A later package causes
unregister + `memset(&rapl_pmu.pmu)` + failed `perf_pmu_register`
(`registered` stays false; prior packages keep `has_pmu`). (3) Last
`has_pmu` package is removed. Dual-package extra-domain update is
documented in-tree. First-package-only register failure is already safe
(`has_pmu` not set). Commonness: uncommon; severity when hit: panic.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Kernel oops/panic** via NULL deref in `list_del_rcu` /
`__list_del`. **Severity: CRITICAL.**
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** Prevents a real panic on TPMI RAPL unbind after a failed
PMU update. Complements the probe-failure log already in 6.12.110.
- **Risk:** Very low — one boolean already used on the add path; skip
unregister only when the PMU is not on the perf list.
- **Ratio:** Strongly favors backport. Needs a 2-line context-adjusted
backport, not the later `_locked` API.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real kernel panic (`list_del_rcu` on a zeroed `pmu->entry`)
- Buggy code and `rapl_pmu.registered` both exist in 6.12.111
- Bug present since v6.11
- 2-line, obviously correct
- Reviewed and applied by Rafael Wysocki
- Sibling PMU-failure handling already backported here (35cf088)
- No new feature/API
**AGAINST:**
- Exact hunk targets `rapl_package_remove_pmu_locked()` (post-6.12
split) — needs a tiny backport
- Trigger needs a failed re-register after a successful one (not every
boot)
- TPMI-only in this tree
- No syzbot / no Fixes tag (expected)
**UNRESOLVED:**
- Stable@ discussion (lore 403)
- Whether a later mainline `rapl_pmu_update()` memsets the whole
`rapl_pmu` on error (6.12 path is still panic-capable as shown)
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — flag already tracks
registration; maintainer applied it.
2. Fixes a real bug that affects users? **PASS** — panic on RAPL TPMI
unbind after failed PMU update.
3. Important issue? **PASS** — kernel panic, CRITICAL.
4. Small and contained? **PASS** — 2 lines, one function.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** (with trivial rename/context:
same check in `rapl_package_remove_pmu`).
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/build/docs exception. It is a straight
crash fix.
**Step 9.4: MAKE YOUR DECISION**
This is a small, obviously correct panic fix for code that 6.12 has had
since v6.11. The `_locked` wrapper is not a dependency. Backport the
same `if (rapl_pmu.registered)` guard into `rapl_package_remove_pmu()`.
Verification:
- [Phase 1] Parsed tags: Reviewed-by Abel Vesa; Link msgid
20260822094657; Rafael SOB; no Fixes/Cc:stable/syzbot
- [Phase 1] Body describes list_del_rcu on NULL after failed register +
leftover has_pmu
- [Phase 2] Diff: 2 lines in remove path; add `if (rapl_pmu.registered)`
before `perf_pmu_unregister`
- [Phase 2] Local `rapl_pmu_update()` confirmed: memset pmu, set
registered=false, return on register failure without clearing other
packages’ has_pmu
- [Phase 2] `perf_pmu_unregister` → `list_del_rcu(&pmu->entry)`;
`list_add_rcu` only after successful register
- [Phase 3] git blame: 575024a8aa7cf (v6.11) introduced the bug;
ancestor of HEAD
- [Phase 3] 963a9ad3c589 TPMI Enable PMU support is in this tree
- [Phase 3] 35cf088430978 already in v6.12.110; does not fix unregister
panic
- [Phase 3] dcd0b625 `_locked` split is NOT an ancestor of HEAD
- [Phase 4] Original 2026-07-26; resend 2026-08-22; Abel Reviewed-by;
Rafael applied as 7.3-rc; no NAK
- [Phase 4] b4 dig on this SHA N/A (commit not in tree); b4 dig on
35cf088 found v3 lore thread
- [Phase 4] lore.kernel.org/stable fetch 403 — UNVERIFIED stable-list
history
- [Phase 5] Only 6.12 caller: intel_rapl_tpmi_remove; probe ignores
add_pmu errors
- [Phase 6] git describe HEAD: p-6.12-3-gc504cf0baa3d8; kernelversion
6.12.111; buggy unregister at intel_rapl_common.c:2104
- [Phase 6] rapl_pmu.registered exists; patch needs function-name
context tweak
- [Phase 8] Failure mode: NULL deref / oops on unbind, severity CRITICAL
- UNVERIFIED: whether anyone nominated this on stable@
The fix is small, surgical, and meets the stable criteria for this 6.12
tree.
**YES**
drivers/powercap/intel_rapl_common.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c
index f08ac99f5e55e..977c4c90923d5 100644
--- a/drivers/powercap/intel_rapl_common.c
+++ b/drivers/powercap/intel_rapl_common.c
@@ -2106,7 +2106,9 @@ void rapl_package_remove_pmu(struct rapl_package *rp)
return;
}
- perf_pmu_unregister(&rapl_pmu.pmu);
+ if (rapl_pmu.registered)
+ perf_pmu_unregister(&rapl_pmu.pmu);
+
memset(&rapl_pmu, 0, sizeof(struct rapl_pmu));
}
EXPORT_SYMBOL_GPL(rapl_package_remove_pmu);
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH AUTOSEL 6.18-6.6] pmdomain: qcom: rpmhpd: Skip retention by default
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-6.12] powercap: intel_rapl: Fix kernel panic during PMU unbind Sasha Levin
@ 2026-10-19 13:48 ` Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.6] thermal: intel: int340x: clean up RFIM groups on DVFS failure Sasha Levin
` (4 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2026-10-19 13:48 UTC (permalink / raw)
To: patches, stable
Cc: Mike Tipton, Konrad Dybcio, Ulf Hansson, Sasha Levin,
linux-arm-msm, linux-pm, linux-kernel
From: Mike Tipton <mike.tipton@oss.qualcomm.com>
[ Upstream commit 7178817f190496f2bfdec181fb4aa72832448468 ]
Retention is very rarely an operational corner. In the majority of
cases, HW cannot operate properly at Retention levels and so the minimum
operational level when enabling a rail is the first corner above
Retention. A small subset of always-on use cases can operate at
Retention, but those aren't controlled from HLOS.
Currently, we allow Retention by default and only disallow it special
cases. But this leaves us open to various failures when the PD is
enabled without first being voted to an OPP above Retention. Such as
when a child GDSC PD requests to enable its parent RPMh PD. In this
case, the GDSC would get stuck powering on.
Newer chips have started supporting Retention on rails that didn't
previously (such as for MMCX). Instead of adding more special cases to
skip Retention on MMCX, start skipping Retention by default since it's
almost never desired from an HLOS perspective.
Signed-off-by: Mike Tipton <mike.tipton@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Ulf Hansson <ulfh@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit message forensics
**Step 1.1: Subject line**
- Record: `[pmdomain/qcom/rpmhpd] [Skip] Skip Retention voltage as the
default enable corner`
**Step 1.2: Tags**
- Record:
- `Signed-off-by: Mike Tipton <mike.tipton@oss.qualcomm.com>` (author,
Qualcomm)
- `Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>`
(Qualcomm SoC reviewer)
- `Signed-off-by: Ulf Hansson <ulfh@kernel.org>` (GENERIC PM DOMAINS
maintainer)
- No `Fixes:` tag (expected)
- No `Cc: stable` (expected)
- No `Reported-by`, `Tested-by`, `Link`, or syzbot tags
- Notable: Qualcomm author + Qualcomm reviewer + subsystem maintainer
SOB. No user/syzbot report.
**Step 1.3: Body**
- Record:
- **Bug:** HLOS enable of an RPMh PD without an OPP above Retention
can vote Retention. Most hardware cannot run at Retention.
- **Symptom:** Child GDSC enable of a parent RPMh PD leaves the GDSC
stuck powering on (hardware hang, not a kernel oops).
- **Root cause:** Retention is allowed by default; only MXC opted out
via `skip_retention_level`. Newer chips now expose Retention on
rails that previously did not (e.g. MMCX).
- **Fix approach:** Skip Retention for every rail when building the
cmd-db mapping, instead of adding more per-rail flags.
- **Version info:** None in the message. “Newer chips” / MMCX is the
motivation.
**Step 1.4: Hidden bug fix?**
- Record: Not hidden. It is a real hardware-failure fix described as a
default-policy change. Skipping Retention is the workaround, not a
cleanup.
## Phase 2: Diff analysis
**Step 2.1: Inventory**
- Record:
- File: `drivers/pmdomain/qcom/rpmhpd.c` (+9 / −5)
- Functions/data: `struct rpmhpd` (drop `skip_retention_level`), `mxc`
/ `mxc_ao` initializers, `rpmhpd_update_level_mapping()`
- Scope: single-file surgical change
**Step 2.2: Code flow**
- Record:
- **Before:** `rpmhpd_update_level_mapping()` copied cmd-db aux levels
into `rpmhpd->level[]`. Retention (vlvl 16) was skipped only if
`skip_retention_level` was set. That flag was true only on `mxc` /
`mxc_ao`. `enable_corner` became the first non-zero mapped corner,
which could be Retention.
- **`rpmhpd_power_on()`:** `corner = max(pd->corner,
pd->enable_corner)` then aggregate. Enable without an OPP uses
`enable_corner`.
- **After:** Retention is always skipped. `enable_corner` is the first
corner above Retention. MXC’s flag is redundant and removed.
- Path affected: probe-time mapping, then every power-on that does not
already have a higher OPP (including parent enable from a child
GDSC).
**Step 2.3: Bug mechanism**
- Record: **Hardware workaround / logic fix.** Retention as min enable
corner is not a valid operational vote for HLOS. Child GDSC parent-on
uses that corner and the GDSC never completes power-on.
- Verified path: `gdsc_register()` attaches GDSCs as genpd subdomains of
the clock-controller’s `dev->pm_domain` (`drivers/clk/qcom/gdsc.c`).
`genpd_power_on()` walks parents and calls `rpmhpd_power_on()` with no
performance state. DT has many `power-domains = <&rpmhpd RPMHPD_MMCX>`
nodes (sm8450/sm8550/sm8650/sm8750 and others).
**Step 2.4: Fix quality**
- Record: Obviously correct given the existing skip logic. Minimal. If
cmd-db has no Retention entry, the new `if` never matches (no-op on
older rails). Regression risk is a slightly higher min vote (Retention
16 → typically MIN_SVS 48) on rails that advertised Retention; the
author states HLOS must not use Retention. No API/ABI change.
## Phase 3: Git history
**Step 3.1: Blame**
- Record:
- Mapping loop: `279b7e8a62cc` (“soc: qcom: rpmhpd: Add RPMh power
domain driver”, 2019)
- `enable_corner`: `e3e56c050ab6` (“soc: qcom: rpmhpd: Make power_on
actually enable the domain”, 2021) — this is what makes Retention
reachable as the enable vote
- Opt-in skip: `ddab91f4b2de5` (“Skip retention level for Power
Domains”, 2024), MXC only, `Cc: stable@vger.kernel.org # v5.17`
- This change: `7178817f19049` (2026-07-13)
**Step 3.2: Fixes tag**
- Record: None. Closest predecessor is `ddab91f4b2de5` (MXC-only skip).
Both `ddab91f4b2de5` and `e3e56c050ab6` are ancestors of this HEAD.
**Step 3.3: File history**
- Record: Recent work is mostly new SoC tables. No later revert of
default-skip. Eliza MXC/MMCX add was reverted for unrelated reasons
(`9fca7779ad185`). Standalone v1 patch, not part of a numbered series.
**Step 3.4: Author**
- Record: Mike Tipton (Qualcomm); this is his only recent commit on
`drivers/pmdomain/qcom/`. Konrad reviews rpmhpd regularly. Ulf is the
pmdomain maintainer.
**Step 3.5: Dependencies**
- Record: Needs `enable_corner` + Retention skip `continue` in
`rpmhpd_update_level_mapping()`. Both exist here. Applies standalone
on a tree that still has `skip_retention_level`. No other patches
required.
## Phase 4: Mailing list / external
**Step 4.1: Original discussion**
- Record: `b4 dig -c 7178817f19049` →
https://patch.msgid.link/20260629-rpmhpd-skip-
ret-v1-1-d94180ad9a29@oss.qualcomm.com
- Series: **v1 only** (no v2/v3).
- Thread (mbox): Ulf Hansson: “Applied for next, thanks!” No NAKs, no
stable nomination, no reviewer risk flags.
**Step 4.2: Recipients (`b4 dig -w`)**
- Record: Ulf Hansson, linux-arm-msm, linux-pm, linux-kernel, Bjorn
Andersson, Konrad Dybcio, Taniya Das (author of the MXC skip), Fenglin
Wu. Right lists and people.
**Step 4.3: Bug report**
- Record: No public user/syzbot/bugzilla report. Failure mode is
described by the Qualcomm author from platform bring-up.
**Step 4.4: Related series**
- Record: Standalone. Extends `ddab91f4b2de5` rather than depending on a
multi-patch series.
**Step 4.5: Stable list**
- Record: lore.kernel.org/stable fetch returned 403. UNVERIFIED whether
stable@ discussed this subject. The 2024 MXC skip *was* Cc:stable.
## Phase 5: Semantic / call graph
**Step 5.1: Functions**
- Record: `rpmhpd_update_level_mapping()` (changed), `rpmhpd_power_on()`
/ `rpmhpd_set_performance_state()` (consumers of `enable_corner`,
unchanged).
**Step 5.2: Callers**
- Record: `rpmhpd_update_level_mapping()` ← `rpmhpd_probe()` only.
`rpmhpd_power_on()` is `generic_pm_domain.power_on`, called from genpd
(including parent-on for child GDSCs). Probe + device/runtime PM, not
a syscall-only path.
**Step 5.3: Callees**
- Record: `cmd_db_read_aux_data()`, then later
`rpmhpd_aggregate_corner()` → `rpmhpd_send_corner()` (RPMh votes).
**Step 5.4: Reachability**
- Record: Yes, from normal Qualcomm device probe. Clock-controller GDSCs
are subdomains of rpmhpd (often MMCX). Userspace does not need a
special trigger; enabling display/video/camera clocks is enough.
**Step 5.5: Similar patterns**
- Record: Only MXC used `skip_retention_level`. MMCX/`mmcx_w_cx_parent`
did not. That matches “instead of adding more special cases for MMCX.”
## Phase 6: This tree only
**Step 6.1: Does the buggy code exist here?**
- Record:
- Local tree: **Linux 7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`,
Makefile `VERSION=7 PATCHLEVEL=3`).
- `drivers/pmdomain/qcom/rpmhpd.c` is present. Pre-commit code had
`skip_retention_level` only on MXC.
- `7178817f19049` is already an ancestor of HEAD (in v7.3-rc1; **not**
in v7.2). Current file already skips Retention unconditionally.
- Bug class (Retention as `enable_corner`) dates to `e3e56c050ab6`
(2021) whenever cmd-db lists Retention; MMCX Retention on newer
chips is why this default change was made. That hardware **is**
represented in this 7.3 tree.
**Step 6.2: Backport difficulty**
- Record: Against current HEAD the patch is already applied (forward
apply would fail). Against 7.3 immediately before this commit it is a
clean apply (same file layout, `skip_retention_level` present). No
extra backport rework for this tree’s rpmhpd.c.
**Step 6.3: Related fixes already here**
- Record: MXC-only skip (`ddab91f4b2de5`) is here; it does **not** cover
MMCX/CX/etc. `2a93c6cbd5a70` (enabled_corner aggregation) is a
different bug (peer off during sync_state). No substitute for this
commit.
## Phase 7: Subsystem context
**Step 7.1**
- Record: `drivers/pmdomain/qcom/` — Qualcomm RPMh power domains.
**IMPORTANT** (not core mm/fs, but used on a large set of Snapdragon
phones, laptops, and embedded boards). Maintainer: Ulf Hansson
(`GENERIC PM DOMAINS`).
**Step 7.2**
- Record: Active (new SoC tables: Nord, Hawi, Eliza, Kaanapali, Glymur,
etc.). The Retention default is independent of those tables.
## Phase 8: Impact and risk
**Step 8.1: Who is affected**
- Record: Qualcomm rpmhpd platforms (`CONFIG_QCOM_RPMHPD`), especially
SoCs whose cmd-db advertises Retention on MMCX or other non-MXC rails.
Not universal; not staging.
**Step 8.2: Trigger**
- Record: Common: child GDSC on, parent rpmhpd has no OPP. Happens at
probe/runtime PM. Unprivileged userspace can cause it indirectly by
using multimedia devices; it also happens at boot when those
controllers probe.
**Step 8.3: Failure mode**
- Record: GDSC stuck powering on → device/controller hang, probe
failure, missing display/video/camera (or boot issues if that path is
on the critical path). Severity: **HIGH** (hardware hang), not a
KASAN/UAF class crash.
**Step 8.4: Risk-benefit**
- Record:
- **Benefit:** Prevents GDSC/parent-rail hangs on Qualcomm platforms
in this tree, including MMCX without extending a per-rail flag list.
- **Risk:** Low. 14-line change; no-op if Retention is absent from
cmd-db; MXC behavior unchanged; Qualcomm + maintainer reviewed.
- **Ratio:** Benefit > risk.
## Phase 9: Synthesis
**Step 9.1: Evidence**
For:
- Concrete hang: parent RPMh PD at Retention → child GDSC stuck on.
- Call path and DT consumers verified in this tree.
- Small, obviously correct extension of an existing skip that was
already Cc:stable for MXC.
- Safe no-op when Retention is not in cmd-db.
- Reviewed by Konrad, applied by Ulf; Taniya Das (original skip author)
was on Cc.
- This 7.3 tree has rpmhpd, `enable_corner`, `skip_retention_level`, and
MMCX users.
Against:
- No `Fixes:` / public reproducer / syzbot (not a negative per rules).
- Default change for all rails, not only MMCX (mitigated: Retention skip
is a no-op where unused).
- lore.kernel.org/stable search could not be fetched (403).
**Step 9.2: Stable rules**
1. Obviously correct and tested? **PASS** — v1 accepted, maintainer
applied, Qualcomm review.
2. Real user-visible bug? **PASS** — GDSC stuck on parent enable.
3. Important (hang/corruption/security)? **PASS** — hardware hang on a
widely used SoC family; HIGH, not a panic.
4. Small and contained? **PASS** — +9/−5, one file.
5. No new features/APIs? **PASS** — same skip, defaulted; no userspace
API.
6. Can apply to this tree? **PASS** — 7.3 has the pre-image; the commit
is in 7.3-rc1.
**Step 9.3: Exception category**
- Record: Hardware workaround (skip a non-operational corner), same
class as the 2024 MXC Retention skip.
**Step 9.4: Decision**
This tree is 7.3.0-rc4. The rpmhpd driver, the Retention-
as-`enable_corner` bug, and MMCX parent-GDSC consumers are all here. The
change is a small, correct hang fix, not a feature. It should be in this
tree.
---
### What it solves for users of this tree
Without it, enabling a Qualcomm GDSC whose parent RPMh rail (often MMCX)
has no OPP votes Retention. That rail cannot actually run there, so the
GDSC never finishes power-on. Display, video, camera, and similar
controllers that list `power-domains = <&rpmhpd RPMHPD_MMCX>` hit this
on probe or runtime PM. Skipping Retention when building the cmd-db map
makes the min enable corner the first real operational level.
### Risk vs benefit
Benefit is avoiding a platform hang on common Qualcomm PM paths. Risk is
low: SoCs without Retention in cmd-db are unchanged; MXC already skipped
Retention; HLOS is not supposed to use Retention.
### Concerns
No public reproducer. Stable-list discussion unverified (HTTP 403).
Slightly higher min vote on rails that listed Retention is intended, not
a silent functional change for rails that never had it.
### Verification
- [Phase 1] Parsed tags: author/reviewer/maintainer SOBs only; no
Fixes/stable/syzbot
- [Phase 2] Diff: +9/−5 in `rpmhpd.c`; drop `skip_retention_level`;
always skip `RPMH_REGULATOR_LEVEL_RETENTION` (16)
- [Phase 2] `rpmhpd_power_on()` uses `max(pd->corner,
pd->enable_corner)` — verified in file
- [Phase 2] GDSC subdomain attach in `gdsc_register()` — verified
- [Phase 2] DT MMCX consumers in sm8450/sm8550/sm8650/sm8750 dtsi —
verified
- [Phase 3] `git describe HEAD` / `make kernelversion`: v7.3-rc4 /
7.3.0-rc4
- [Phase 3] `git merge-base --is-ancestor 7178817f19049 HEAD`: yes; not
in v7.2; in v7.3-rc1
- [Phase 3] Blame: mapping 279b7e8a62cc (2019); enable_corner
e3e56c050ab6 (2021); MXC skip ddab91f4b2de5 (2024)
- [Phase 3] Pre-image: `skip_retention_level` only on mxc/mxc_ao; mmcx
had no flag
- [Phase 4] `b4 dig -c 7178817f19049`: lore/msgid `20260629-rpmhpd-skip-
ret-v1-1-d94180ad9a29@oss.qualcomm.com`
- [Phase 4] `b4 dig -a`: v1 only; Ulf “Applied for next”; no NAK
- [Phase 4] `b4 dig -w`: Ulf, Bjorn, Konrad, Taniya Das, linux-arm-msm,
linux-pm
- [Phase 4] UNVERIFIED: lore.kernel.org/stable (403)
- [Phase 5] `rpmhpd_update_level_mapping()` called only from
`rpmhpd_probe`
- [Phase 6] Post-fix code already in this checkout; related MXC skip
present and insufficient
- [Phase 8] Failure mode: GDSC stuck on — HIGH hang, Qualcomm-specific
**YES**
drivers/pmdomain/qcom/rpmhpd.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/pmdomain/qcom/rpmhpd.c b/drivers/pmdomain/qcom/rpmhpd.c
index 4c3cbf3abc750..fc61ec45755c3 100644
--- a/drivers/pmdomain/qcom/rpmhpd.c
+++ b/drivers/pmdomain/qcom/rpmhpd.c
@@ -41,7 +41,6 @@
* @addr: Resource address as looped up using resource name from
* cmd-db
* @state_synced: Indicator that sync_state has been invoked for the rpmhpd resource
- * @skip_retention_level: Indicate that retention level should not be used for the power domain
*/
struct rpmhpd {
struct device *dev;
@@ -58,7 +57,6 @@ struct rpmhpd {
const char *res_name;
u32 addr;
bool state_synced;
- bool skip_retention_level;
};
struct rpmhpd_desc {
@@ -176,7 +174,6 @@ static struct rpmhpd mxc = {
.pd = { .name = "mxc", },
.peer = &mxc_ao,
.res_name = "mxc.lvl",
- .skip_retention_level = true,
};
static struct rpmhpd mxc_ao = {
@@ -184,7 +181,6 @@ static struct rpmhpd mxc_ao = {
.active_only = true,
.peer = &mxc,
.res_name = "mxc.lvl",
- .skip_retention_level = true,
};
static struct rpmhpd nsp = {
@@ -974,7 +970,15 @@ static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd)
return -EINVAL;
for (i = 0; i < rpmhpd->level_count; i++) {
- if (rpmhpd->skip_retention_level && buf[i] == RPMH_REGULATOR_LEVEL_RETENTION)
+ /*
+ * Most HW won't function properly at Retention. The minimum
+ * operational level is the first level above Retention. The
+ * small subset of HW that can operate at Retention isn't
+ * controlled by HLOS. Skip the Retention level to avoid HW
+ * failures when the PD is enabled without first having an
+ * explicit OPP level set.
+ */
+ if (buf[i] == RPMH_REGULATOR_LEVEL_RETENTION)
continue;
rpmhpd->level[i] = buf[i];
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH AUTOSEL 6.18-6.6] thermal: intel: int340x: clean up RFIM groups on DVFS failure
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-6.12] powercap: intel_rapl: Fix kernel panic during PMU unbind Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.6] pmdomain: qcom: rpmhpd: Skip retention by default Sasha Levin
@ 2026-10-19 13:48 ` Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.1] opp: Use clk_get_optional() to avoid leaving opp_table->clk as an error pointer Sasha Levin
` (3 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2026-10-19 13:48 UTC (permalink / raw)
To: patches, stable
Cc: Pengpeng Hou, Rafael J. Wysocki, Sasha Levin, rafael,
daniel.lezcano, linux-pm, linux-kernel
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
[ Upstream commit 28f34d7dafa6dfa657efd6760b7c78e98c9b2f6a ]
proc_thermal_rfim_add() can create the FIVR and DLVR sysfs groups
before creating the DVFS group.
If DVFS group creation fails while both earlier groups are enabled, the
current error handling removes only one group: the FIVR branch returns
before the DLVR cleanup branch can run. This leaves the DLVR group
behind even though RFIM setup fails.
Use one DVFS failure path that removes all previously created RFIM
groups before returning the error.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Link: https://patch.msgid.link/20260615065245.84252-1-pengpeng@iscas.ac.cn
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit message forensics
**Step 1.1: PARSE THE SUBJECT LINE**
Record: `[thermal/intel/int340x] [clean up] incomplete RFIM sysfs-group
unwind when DVFS group creation fails`
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>` (author)
- `Link:
https://patch.msgid.link/20260615065245.84252-1-pengpeng@iscas.ac.cn`
- `Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>`
(thermal maintainer)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
by:`, or `Cc: stable` (absence of the last two is expected for this
review)
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- Bug: `proc_thermal_rfim_add()` may create FIVR and DLVR sysfs groups,
then fail creating DVFS.
- Symptom: on DVFS failure with both earlier groups enabled, the FIVR
cleanup branch returns immediately, so the DLVR group is never
removed.
- Failure mode: leftover sysfs group after RFIM setup reports failure
(resource leak; probe of the Intel processor thermal PCI device
fails).
- Root cause: two sequential `if (ret && FEATURE_*) { remove; return; }`
blocks instead of one failure path that removes every group that was
created.
- Version info in the message: none.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Yes. The subject says “clean up”, but the body and the diff are
an error-path leak fix (missing `sysfs_remove_group` for DLVR).
## Phase 2: Diff analysis
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/thermal/intel/int340x_thermal/processor_thermal_rfim.c`
(+5 / −6)
- Function: `proc_thermal_rfim_add()`
- Scope: single-file surgical error-path change
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record: Before, DVFS `sysfs_create_group()` failure did:
```c
if (ret && FIVR) { sysfs_remove_group(fivr); return ret; }
if (ret && DLVR) { sysfs_remove_group(dlvr); return ret; }
```
If both FIVR and DLVR bits are set, the first branch returns and DLVR is
never removed. After, a single `if (ret)` removes DLVR and/or FIVR
according to the feature mask, then returns.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: `[error path / resource leak]` Incomplete unwind.
`sysfs_create_group(&pdev->dev.kobj, &dvfs_attribute_group)` failure
leaked `dlvr_attribute_group` when FIVR+DLVR+DVFS were all enabled.
**Step 2.4: ASSESS THE FIX QUALITY**
Record: Obviously correct. Groups are only removed if the corresponding
feature bit is set, and those bits are the same ones that caused the
groups to be created earlier in this function. No API, locking, or data-
structure changes. Regression risk is very low: `sysfs_remove_group` on
a group that this function just created.
## Phase 3: Git history
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Original FIVR-only DVFS unwind: `473be51142adba` (“Add RFIM driver”,
2020; first in v5.11-rc1).
- Incomplete DLVR branch: `5bc6b1df65c87f` (“Add DLVR support for RFIM
control”, 2023-04-13; first in v6.4-rc1). That commit added the second
`if (ret && DLVR) return` after the existing FIVR early-return, which
is the bug.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Not applicable beyond that. The introducing
commit is `5bc6b1df65c87f`, and `git merge-base --is-ancestor
5bc6b1df65c87f HEAD` succeeded.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- `15176b818e048c` (“Fix sysfs group leak on DLVR registration failure”)
— same class of bug for the *DLVR create* path leaking FIVR; already
in this tree (v7.0-rc1).
- This patch is standalone, not `patch X/Y`. Same author later did a
similar ODVP probe-unwind fix (`d83dc9ce57a74`) in a different file;
not a prerequisite.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Pengpeng Hou is not the subsystem maintainer. Thermal maintainer
Rafael J. Wysocki applied the patch. Driver author Srinivas Pandruvada
was on the original To/Cc list.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: None. The fix only reorders existing `sysfs_remove_group()`
calls on `fivr_attribute_group` / `dlvr_attribute_group`, both present
in this tree. Can apply standalone.
## Phase 4: Mailing list and external research
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 28f34d7dafa6d` →
https://patch.msgid.link/20260615065245.84252-1-pengpeng@iscas.ac.cn
- `b4 dig -a`: v1 only
- Thread (mbox): no NAKs. Rafael: “Applied as 7.3 material, thanks!”
- No reviewer `Cc: stable` suggestion
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc included Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Srinivas Pandruvada, linux-pm. Applied by the thermal
maintainer. No separate Reviewed-by.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No `Reported-by` / bugzilla / syzbot. This is a code-inspection
leak, not a user crash report.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Standalone v1. Sibling leak fix is `15176b818e048c` (already
here). Author’s ODVP cleanup is a different driver file.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org HTTP 403. `b4 dig` thread has no stable-list
discussion. UNVERIFIED: whether linux-stable@ ever discussed this
subject independently.
## Phase 5: Code semantic analysis
**Step 5.1: KEY FUNCTIONS**
Record: `proc_thermal_rfim_add()` (modified). Uses `sysfs_create_group`
/ `sysfs_remove_group`.
**Step 5.2: TRACE CALLERS**
Record:
- `proc_thermal_mmio_add()` in `processor_thermal_device.c` (only
caller)
- That is called from `proc_thermal_pci_probe()` and
`proc_thermal_pci_legacy` probe
- Probe / device-enumeration path, not IRQ or a hot syscall path
**Step 5.3: TRACE CALLEES**
Record: `sysfs_create_group`, `sysfs_remove_group` on the PCI device
kobject.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: PCI probe → `proc_thermal_mmio_add()` →
`proc_thermal_rfim_add()`. On RFIM failure, `proc_thermal_mmio_add()`
does `goto err_rem_ptc` and **does not** call
`proc_thermal_rfim_remove()`. `proc_thermal_pci_probe()` then `return
ret` without `proc_thermal_mmio_remove()`. The leaked DLVR group
therefore stays on `pdev->dev.kobj`. Reachable whenever this Intel
thermal PCI device is probed (common on Intel laptops/SoCs with
INT340x). Trigger still requires `sysfs_create_group(dvfs)` to fail.
Verified hardware with all three bits (the only way both groups exist at
DVFS failure): Meteor Lake `MTLP_THERMAL` is `FIVR | DVFS | DLVR`
(`processor_thermal_device_pci.c`). `5bc6b1df65c87f` is what OR’d
`PROC_THERMAL_FEATURE_DLVR` onto that ID.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same incomplete-unwind pattern was already fixed for DLVR-
create-fails-after-FIVR (`15176b818e048c`). `fs/sysfs/group.c` returns
`-EEXIST` (and warns) if the named group already exists, so a leaked
DLVR group can make a later re-bind fail even after the original
transient error is gone.
## Phase 6: Cross-referencing against the local tree
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Local tree: `v7.3-rc4-27-gf0100363d8c37`, `make kernelversion` →
7.3.0-rc4
- RFIM, DLVR, FIVR, DVFS, and MTLP FIVR+DLVR+DVFS are all present
- Bug introduced in v6.4 (`5bc6b1df65c87f`), well before this 7.3 branch
- Parent of the candidate still has the two early-return blocks
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Clean apply. Surrounding `proc_thermal_rfim_add()` in this tree
matches the hunk (Nova Lake DLVR switch and the earlier FIVR leak fix
are already here).
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: `15176b818e048c` is in this tree and fixes a *different* leak
(DLVR create vs FIVR). It does not fix DVFS-create leaking DLVR. No
other substitute for this patch.
## Phase 7: Subsystem and maintainer context
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: `[drivers/thermal/intel/int340x] [IMPORTANT]` — Intel laptop/SoC
processor thermal / RFIM sysfs, not core mm/fs, but used on shipping
Meteor Lake platforms. `CONFIG_INT340X_THERMAL`.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively maintained (Nova Lake, Wildcat Lake, Panther Lake IDs
and DLVR tables in recent history). Maintainer applied this patch.
## Phase 8: Impact and risk
**Step 8.1: WHO IS AFFECTED**
Record: Driver-specific / config-specific: Intel INT340x processor
thermal PCI, specifically IDs with FIVR+DLVR+DVFS (Meteor Lake in this
tree).
**Step 8.2: TRIGGER CONDITIONS**
Record: Device probe, after FIVR and DLVR groups succeed, when DVFS
`sysfs_create_group()` fails (ENOMEM, duplicate name, etc.). Not
unprivileged-syscall reachable except via driver bind. Uncommon, but
deterministic when it happens. Re-probe can then hit `-EEXIST` on the
leaked DLVR group (`internal_create_group()` in `fs/sysfs/group.c`).
**Step 8.3: FAILURE MODE SEVERITY**
Record: `[sysfs group leak on PCI device after failed probe; possible
sticky re-bind failure] [MEDIUM]` — not a panic or filesystem
corruption. Stale `dlvr` attributes can remain on the PCI device kobject
after the driver has reported probe failure.
**Step 8.4: RISK-BENEFIT RATIO**
Record: Benefit medium (correct unwind on a real shipping platform’s
probe error path; avoids leftover sysfs and sticky `-EEXIST`). Risk very
low (5-line error-path-only change, no locking/API). Ratio favors taking
the fix.
## Phase 9: Final synthesis
**Step 9.1: EVIDENCE**
FOR:
- Real, deterministic incomplete unwind; DLVR group leaked when
FIVR+DLVR+DVFS are set
- Meteor Lake actually sets all three feature bits
- Caller does not call `proc_thermal_rfim_remove()` on
`proc_thermal_rfim_add()` failure
- Same bug class as `15176b818e048c`, which this tree already contains
- Tiny, obviously correct, no new API
- Thermal maintainer applied it; right people were CCed; no NAKs
- Bug present since v6.4; code exists in this 7.3 tree
AGAINST:
- Trigger is a failed `sysfs_create_group()` (rare)
- No user/syzbot report
- Not a crash, UAF, or data-corruption bug
- Rafael queued it as 7.3 material, not as a stable-cc’d hotfix
UNRESOLVED:
- Direct lore.kernel.org / stable-list HTTP search returned 403; thread
content recovered via `b4 dig -m` instead
**Step 9.2: STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — control flow is local and
clear; maintainer applied v1 with no further revisions.
2. Fixes a real bug that affects users? **PASS** — leftover sysfs on the
PCI device after failed RFIM setup on Meteor Lake.
3. Important issue? **PASS** (medium) — resource leak with a possible
sticky re-bind failure, not a panic; still the error-path leak class
stable regularly takes.
4. Small and contained? **PASS** — one function, +5/−6 lines.
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — file and both sysfs groups
exist; hunk matches.
**Step 9.3: EXCEPTION CATEGORIES**
Record: None (not a device ID, quirk, DT, build, or docs exception). It
is a normal small leak fix.
**Step 9.4: DECISION**
This tree is 7.3.0-rc4. The buggy unwind, DLVR/FIVR/DVFS groups, and
Meteor Lake FIVR+DLVR+DVFS mask are all here. The patch is a minimal,
correct error-path leak fix with negligible regression risk. It should
be in this tree.
Verification:
- [Phase 1] Tags: author SOB, Rafael SOB, lore Link; no Fixes/Reported-
by/Cc: stable
- [Phase 2] Diff: `proc_thermal_rfim_add()` DVFS failure path; two
early-returns collapsed into one unwind of DLVR then FIVR
- [Phase 3] `git blame` on parent: FIVR unwind from `473be51142adba`
(v5.11); DLVR early-return from `5bc6b1df65c87f` (v6.4-rc1)
- [Phase 3] `git merge-base --is-ancestor 5bc6b1df65c87f HEAD` → yes;
related leak fix `15176b818e048c` also in HEAD
- [Phase 3] `git log --author='Pengpeng Hou'` on thermal: this RFIM fix
plus a later independent ODVP unwind
- [Phase 4] `b4 dig -c 28f34d7dafa6d`:
https://patch.msgid.link/20260615065245.84252-1-pengpeng@iscas.ac.cn
- [Phase 4] `b4 dig -a`: v1 only; `b4 dig -w`: linux-pm +
Pandruvada/Wysocki/Lezcano
- [Phase 4] mbox: Rafael “Applied as 7.3 material”; no NAK, no stable
nomination
- [Phase 4] lore.kernel.org WebFetch: 403; stable-list search UNVERIFIED
via HTTP
- [Phase 5] Callers: `proc_thermal_mmio_add` → PCI probe; RFIM failure
jumps to `err_rem_ptc`, skipping `proc_thermal_rfim_remove`
- [Phase 5] MTLP ID: `FIVR | DVFS | DLVR` in
`processor_thermal_device_pci.c`; DLVR bit added in `5bc6b1df65c87f`
- [Phase 5] `sysfs_create_group` / `internal_create_group`: `-EEXIST` if
named group already exists
- [Phase 6] `git describe HEAD` = v7.3-rc4-27-gf0100363d8c37; `make
kernelversion` = 7.3.0-rc4
- [Phase 6] Parent function still has the buggy two-branch return; this
tree has the needed groups and IDs
- [Phase 8] Failure mode: leaked `dlvr` sysfs group after probe failure;
severity MEDIUM
The fix is small, surgical, and meets the stable criteria for an error-
path resource leak on hardware this tree already supports.
**YES**
.../intel/int340x_thermal/processor_thermal_rfim.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_rfim.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_rfim.c
index 635bc8fded1e9..2f32b230a4b15 100644
--- a/drivers/thermal/intel/int340x_thermal/processor_thermal_rfim.c
+++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_rfim.c
@@ -460,12 +460,11 @@ int proc_thermal_rfim_add(struct pci_dev *pdev, struct proc_thermal_device *proc
if (proc_priv->mmio_feature_mask & PROC_THERMAL_FEATURE_DVFS) {
ret = sysfs_create_group(&pdev->dev.kobj, &dvfs_attribute_group);
- if (ret && proc_priv->mmio_feature_mask & PROC_THERMAL_FEATURE_FIVR) {
- sysfs_remove_group(&pdev->dev.kobj, &fivr_attribute_group);
- return ret;
- }
- if (ret && proc_priv->mmio_feature_mask & PROC_THERMAL_FEATURE_DLVR) {
- sysfs_remove_group(&pdev->dev.kobj, &dlvr_attribute_group);
+ if (ret) {
+ if (proc_priv->mmio_feature_mask & PROC_THERMAL_FEATURE_DLVR)
+ sysfs_remove_group(&pdev->dev.kobj, &dlvr_attribute_group);
+ if (proc_priv->mmio_feature_mask & PROC_THERMAL_FEATURE_FIVR)
+ sysfs_remove_group(&pdev->dev.kobj, &fivr_attribute_group);
return ret;
}
}
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH AUTOSEL 6.18-6.1] opp: Use clk_get_optional() to avoid leaving opp_table->clk as an error pointer
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (2 preceding siblings ...)
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.6] thermal: intel: int340x: clean up RFIM groups on DVFS failure Sasha Levin
@ 2026-10-19 13:49 ` Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.10] power: reset: piix4-poweroff: add missing MODULE_DEVICE_TABLE() Sasha Levin
` (2 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2026-10-19 13:49 UTC (permalink / raw)
To: patches, stable
Cc: Praveen Talari, Sebastian Reichel, Viresh Kumar, Sasha Levin,
vireshk, nm, sboyd, linux-pm, linux-kernel
From: Praveen Talari <praveen.talari@oss.qualcomm.com>
[ Upstream commit 84f05af0975c9f9c8ffee0361d6f7cbb1f289231 ]
_update_opp_table_clk() uses clk_get(dev, NULL) to acquire the
device's clock. On platforms where the perf domain device has no
Linux clock and is instead managed entirely by firmware via
devm_pm_opp_of_add_table() (through
of_genpd_add_provider_simple()/onecell()), clk_get() returns
-ENOENT. That case is treated as valid (the OPP table can still
have entries sourced from firmware), but opp_table->clk is left
holding ERR_PTR(-ENOENT) rather than being reset to NULL:
opp_table->clk = clk_get(dev, NULL);
ret = PTR_ERR_OR_ZERO(opp_table->clk);
...
if (ret == -ENOENT) {
opp_table->clk_count = 1;
return opp_table; /* opp_table->clk is still ERR_PTR(-ENOENT) */
}
Consumers that only check IS_ERR(opp_table->clk) treat this as a
valid clk and pass it straight into the clk consumer API. In
particular, dev_pm_opp_set_rate() calls
clk_round_rate(opp_table->clk, target_freq), and clk_round_rate()
only guards against a NULL clk, so it dereferences the error pointer
to read clk->exclusive_count and crashes:
Unable to handle kernel NULL pointer dereference at virtual
address 000000000000002e
...
pc : clk_round_rate+0x3c/0x188
...
Call trace:
clk_round_rate+0x3c/0x188 (P)
dev_pm_opp_set_rate+0x114/0x33c
Rather than teaching every clk consumer API to special-case
ERR_PTR(-ENOENT), fix it at the source: use clk_get_optional()
instead of clk_get() in _update_opp_table_clk(), which already
translates -ENOENT into a NULL clk. This documents that the clock is
genuinely optional for such devices, and keeps opp_table->clk holding
either a valid clk or NULL, never a lingering -ENOENT error pointer.
_opp_config_clk_single() is only wired up via opp_table->config_clks
when a clk was actually found, and every other opp_table->clk
consumer already tolerates NULL through the standard clk API (which
treats a NULL clk as a no-op), so no other call site needs to change.
Suggested-by: Sebastian Reichel <sre@kernel.org>
Reviewed-by: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I have enough to decide: the crash path is in this 6.12 tree, the fix is
small, and it should apply cleanly. Full analysis follows.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[opp] [Use] Use `clk_get_optional()` so `opp_table->clk` is
never left as `ERR_PTR(-ENOENT)`.** Subsystem is the OPP core
(`drivers/opp/`). “Use X to avoid Y” is a correctness fix, not a
feature.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Suggested-by:** Sebastian Reichel `<sre@kernel.org>` (linux-pm /
power maintainer)
- **Reviewed-by:** Sebastian Reichel `<sre@kernel.org>`
- **Signed-off-by:** Praveen Talari `<praveen.talari@oss.qualcomm.com>`
(author)
- **Signed-off-by:** Viresh Kumar `<viresh.kumar@linaro.org>` (OPP
maintainer; applied the patch)
- **No Fixes:** tag (expected for this review pipeline)
- **No Cc: stable@vger.kernel.org** (expected; not a negative)
- **No Reported-by / Tested-by / Link / Acked-by**
- Pipeline markers (`[Upstream commit …]`, Sasha Levin SOB) ignored per
instructions
Upstream SHA (from public history, **not** in this tree):
`84f05af0975c9f9c8ffee0361d6f7cbb1f289231`.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `_update_opp_table_clk()` treats `clk_get()` returning
`-ENOENT` as valid (firmware-managed / clockless OPP tables) but
leaves `opp_table->clk = ERR_PTR(-ENOENT)`.
- **Symptom:** kernel oops. `dev_pm_opp_set_rate()` calls
`clk_round_rate(opp_table->clk, …)`. `clk_round_rate()` only NULL-
checks, then reads `clk->exclusive_count`. Error pointer wraps to a
low address (`000000000000002e` in the report).
- **Stack:** `clk_round_rate` ← `dev_pm_opp_set_rate`
- **Trigger:** platforms whose perf-domain / OPP device has no Linux
clock (`devm_pm_opp_of_add_table()` via
`of_genpd_add_provider_simple()`/`onecell()`), then a consumer calls
`dev_pm_opp_set_rate()`.
- **Root cause:** consumers use `IS_ERR()` (true for `-ENOENT`) vs clk
APIs that only accept NULL as “no clock”.
- **Fix:** `clk_get_optional()` maps `-ENOENT` → NULL at the source.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not hidden.** Explicit crash fix with a stack trace. Not
cleanup/refactor-only.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `drivers/opp/core.c` only (`+24 / -30` upstream)
- **Function:** `_update_opp_table_clk()`
- **Scope:** single-file surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record (this tree’s `_update_opp_table_clk` at
`drivers/opp/core.c:1511`):
**Before:**
1. `opp_table->clk = clk_get(dev, NULL)`
2. Success (`ret == 0`): set `config_clks = _opp_config_clk_single`,
`clk_count = 1`
3. `-ENOENT`: set `clk_count = 1`, **leave `clk` as
`ERR_PTR(-ENOENT)`**, return the table
4. Other errors: `dev_pm_opp_put_opp_table()`, `dev_err_probe()`, return
`ERR_PTR(ret)`
**After:**
1. `opp_table->clk = clk_get_optional(dev, NULL)` (`-ENOENT` becomes
NULL)
2. `IS_ERR(clk)`: put table, `dev_err_probe()`, return `ERR_CAST(clk)`
(real errors, e.g. `-EPROBE_DEFER`)
3. If `clk` is non-NULL: set `config_clks`
4. Always `clk_count = 1` and return the table (clock present **or**
optional-absent)
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Memory safety / NULL-vs-ERR_PTR] [Logic / correctness]**
Mechanism: `ERR_PTR(-ENOENT)` is not NULL. `clk_round_rate()` in this
tree (`drivers/clk/clk.c:1799`) does `if (!clk) return 0;` then
`clk->exclusive_count`. `ERR_PTR(-2)` plus `offsetof(struct clk,
exclusive_count)` wraps to `0x2e`, matching the oops.
`_find_current_opp` and table release already use
`!IS_ERR(opp_table->clk)` and therefore skip the error pointer;
`dev_pm_opp_set_rate()` does not.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** obviously correct. `clk_get_optional()` is the documented
helper for this case (`include/linux/clk.h:1195`). `config_clks` is
still only set when a real clk exists — same as before.
- **Surgical:** one helper, no API change.
- **Regression risk (low):**
- After NULL, early-return `!IS_ERR(opp_table->clk)` is true, so later
lookups do not retry `clk_get`. Previously `-ENOENT` was `IS_ERR`
and would retry. Correct for “no clock” devices.
- `_find_current_opp` may now call `clk_get_rate(NULL)` (returns 0)
instead of skipping. Lowest available OPP vs first list entry;
acceptable.
- Error path uses `opp_table->clk` **after**
`dev_pm_opp_put_opp_table()`. Old code saved `ret` first.
Theoretical UAF if the last ref frees the table before the
`PTR_ERR`/`ERR_CAST`. Mainline accepted this; not the common
`-ENOENT` success path.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- `_update_opp_table_clk` introduced in `32439ac7535a8` (“opp: Defer
acquiring the clk until OPPs are added”, 2021-02-02).
- `-ENOENT` treated as valid in `d4a4c7a41153d` (“opp: Don't ignore
clk_get() errors other than -ENOENT”, 2021-02-02) — **this is the
commit that left the error pointer in the table**.
- ENOENT comment / `clk_count = 1` / `config_clks` wiring from
`2083da24eb56ce` (“OPP: Allow multiple clocks for a device”,
2022-06-10).
- `dev_pm_opp_set_rate()` dropped its `IS_ERR(clk)` guard in
`1d3c42cabbd35` (“opp: No need to check clk for errors”, 2021-01-20) —
“Clock is not optional for users who call into
`dev_pm_opp_set_rate()`”, which is false for firmware-managed clocks.
Bug present since ~v5.12, definitely in 6.12.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag.** Introducers `d4a4c7a41153d` and
`1d3c42cabbd35` **are both ancestors of HEAD**.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Recent `drivers/opp/core.c` work is other OPP fixes (level,
bandwidth, genpd). No later local rewrite of `_update_opp_table_clk`.
**Standalone**, not “patch X/Y”.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Praveen Talari has **no** prior commits in `drivers/opp/` on
this tree. Viresh Kumar (OPP maintainer) signed off and applied it.
Sebastian Reichel suggested and reviewed.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record:
- `clk_get_optional()` exists here: `60b8f0ddf1a92` (2019-02-06),
`include/linux/clk.h:1203`.
- Function body in this tree **matches** the patch’s “before” hunk.
**Standalone apply.**
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 84f05af0975c9f9c8ffee0361d6f7cbb1f289231` → https://patch.m
sgid.link/20260727-fix_ptr_check_on_clk-v4-1-
97969fa7826a@oss.qualcomm.com
- **v1:** guard `clk_round_rate()` against error pointers (and a
`drivers/clk/clk.c` change)
- **v2:** per Reichel, fix OPP core with `clk_get_optional()`; drop clk-
core change
- **v3:** add OPP maintainers
- **v4:** rebase; drop local `ret` (applied)
- Applied version is v4 (latest). No NAK. Viresh: “Applied. Thanks.”
(only nit: extra `---` in the changelog)
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): Viresh Kumar, Nishanth Menon, Stephen Boyd (OPP
maintainers), Konrad Dybcio, Sebastian Reichel, linux-pm, linux-kernel,
clk maintainer Boyd. Right audience.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot/bugzilla. Concrete oops in the message from a Qualcomm
engineer. Real hardware, not theoretical.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Single-patch series. v1’s clk-core change was dropped; this does
not depend on other patches.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: **UNVERIFIED** for lore.kernel.org/stable (HTTP 403). No
evidence this was rejected for stable.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: `_update_opp_table_clk()` only.
**Step 5.2: TRACE CALLERS**
Record:
- Only caller: `_add_opp_table_indexed()` (`core.c:1623`)
- DT path `_of_add_table_indexed()` uses `getclk=true` (`of.c:1150`) —
**every** `dev_pm_opp_of_add_table()` / `devm_pm_opp_of_add_table()` /
genpd `of_genpd_add_provider_simple()` that adds an OPP table
- Also `dev_pm_opp_add_dynamic()` (`core.c:2894`, getclk=true)
- Crash consumer: `dev_pm_opp_set_rate()` (`core.c:1352`) — used by
Qualcomm UART/SPI/MMC/GPU/Venus, Tegra, cpufreq-dt,
panfrost/panthor/lima, etc.
**Step 5.3: TRACE CALLEES**
Record: `clk_get` → `clk_get_optional`; on hard error
`dev_pm_opp_put_opp_table` + `dev_err_probe`. No new locks/allocs.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Device probe (`dev_pm_opp_of_add_table`) →
`_update_opp_table_clk` stores `ERR_PTR(-ENOENT)` → later
`dev_pm_opp_set_rate` from driver/cpufreq/devfreq → oops. **Reachable
from normal device operation**, not a debug-only path.
Verified clk NULL-safety in this tree:
- `clk_round_rate`: NULL → 0 (`clk.c:1804`)
- `clk_get_rate`: NULL → 0 (`clk.c:2005`)
- `clk_set_rate`: NULL → 0 (`clk.c:2600`)
- `__clk_put`: NULL returns immediately (`clk.c:4748`)
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Other `opp_table->clk` users already use `!IS_ERR()`
(`_find_current_opp`, kref release). `set_rate` is the one that passes
the error pointer into clk APIs. Fixing storage is the right place (as
Reichel requested).
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** Local tree is **6.12.111** (`pending-6.12`,
`p-6.12-2-gd38d58a4e0359`). `_update_opp_table_clk` still uses
`clk_get(dev, NULL)` and the `-ENOENT` early return
(`core.c:1526-1551`). `git merge-base --is-ancestor
84f05af0975c9f9c8ffee0361d6f7cbb1f289231 HEAD` failed (not present). Bug
since ~2021.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply.** Function text matches the patch context; only
surrounding line numbers differ. `clk_get_optional()` already in
`include/linux/clk.h`.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: `d4a4c7a41153d` (ENOENT allowed) and `1d3c42cabbd35` (set_rate
dropped IS_ERR) are here. No `clk_get_optional` OPP commit. **No
alternate fix.**
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **OPP core** (`drivers/opp/`) — **IMPORTANT**. Shared by
cpufreq, devfreq, DRM, MMC, serial, genpd performance states. Not
staging.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Mature (OPP since well before 6.12) with ongoing correctness
fixes. This bug is old, not a brand-new subsystem.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Config/driver-specific:** `CONFIG_PM_OPP` plus devices with an
OPP table and **no** unnamed Linux clock, that then call
`dev_pm_opp_set_rate()`. Includes firmware-managed perf domains
(Qualcomm-class, genpd OPP). Not every box, but production SoCs.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Probe adds OPP table (`getclk=true`); `clk_get` → `-ENOENT`;
later `dev_pm_opp_set_rate()`. Common on clockless firmware DVFS, not
every boot on x86. Not a syzbot/unprivileged syscall primitive;
driver/cpufreq context.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Kernel oops / NULL deref** in `clk_round_rate`. **Severity:
CRITICAL** (crash; can strand DVFS/device).
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit: HIGH** — prevents a real oops on clockless OPP devices;
6.12 LTS includes those platforms.
- **Risk: LOW** — ~50-line rewrite of one helper; `clk_get_optional`
long-standing; success path equivalent except NULL vs ERR_PTR; clk
APIs already NULL-safe.
- **Ratio: strongly favors backport.**
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real oops with stack trace (`clk_round_rate` ← `dev_pm_opp_set_rate`)
- Buggy pattern present in **this** 6.12 tree
- Small, obviously correct, no new API
- `clk_get_optional` already here
- Reviewed by Reichel; applied by Viresh
- Clean apply; no missing prerequisites
**AGAINST:**
- Affects firmware-managed / clockless OPP devices, not every machine
- Error-path `opp_table->clk` after put is slightly sloppier than the
old local `ret` (not the crash path being fixed)
- No Fixes:/Cc:stable (irrelevant per pipeline rules)
**UNRESOLVED:**
- Could not fetch lore.kernel.org/stable (403)
- Could not fetch v1 body (403/404); changelog is enough
- Exact Qualcomm SoC in the oops not named in the v4 mail
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — helper does exactly this;
maintainer review + apply
2. Fixes a real bug that affects users? **PASS** — oops on clockless OPP
+ `set_rate`
3. Important issue? **PASS** — kernel crash (CRITICAL)
4. Small and contained? **PASS** — one function, one file
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — body matches;
`clk_get_optional` present
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/build/docs exception. Ordinary crash
fix; does not need an exception.
**Step 9.4: MAKE YOUR DECISION**
This is a real NULL-deref oops in OPP core that 6.12 still has. The fix
maps “no clock” to NULL the way the clk API already documents, is tiny,
and applies cleanly here. That is appropriate for this stable tree.
---
### What the commit solves (for this tree)
On 6.12.111, `_update_opp_table_clk()` still does `clk_get(dev, NULL)`
and, on `-ENOENT`, keeps `opp_table->clk` as `ERR_PTR(-ENOENT)` while
treating the table as valid. `dev_pm_opp_set_rate()` then passes that
into `clk_round_rate()`, which only rejects NULL, and the kernel oopses.
That is exactly the path firmware-managed / genpd OPP devices take when
they have no Linux clock.
### Stable rules / risk
Meets the stable criteria: real crash, small, no new API, prerequisites
already in 6.12. Benefit is avoiding an oops; risk is a one-function
change whose success path is equivalent except storing NULL instead of
an error pointer (what clk consumers already expect).
### Verification
- **[Phase 1]** Parsed tags: Suggested-by/Reviewed-by Reichel; SOB
Talari + Viresh; no Fixes/Cc:stable
- **[Phase 1]** Body: oops at `clk_round_rate` ← `dev_pm_opp_set_rate`,
address `0x2e`
- **[Phase 2]** Diff: only `_update_opp_table_clk`; `clk_get` →
`clk_get_optional`; `-ENOENT` → NULL; `config_clks` only if clk non-
NULL
- **[Phase 2]** `clk_round_rate` in this tree: NULL check only, then
`clk->exclusive_count` (`clk.c:1804-1809`)
- **[Phase 3]** Blame: `-ENOENT` keep-table from `d4a4c7a41153d` (in
tree); set_rate dropped IS_ERR in `1d3c42cabbd35` (in tree)
- **[Phase 3]** `clk_get_optional` from `60b8f0ddf1a92` (2019), present
- **[Phase 3]** `git merge-base --is-ancestor 84f05af HEAD` → not in
tree
- **[Phase 4]** `b4 dig -c 84f05af` → v4 msgid
`20260727-fix_ptr_check_on_clk-v4-1-97969fa7826a@oss.qualcomm.com`
- **[Phase 4]** `b4 dig -a`: v1→v4; v2 switched to source fix per
Reichel
- **[Phase 4]** `b4 dig -w`: OPP maintainers + linux-pm on To/Cc
- **[Phase 4]** mbox: Viresh “Applied. Thanks.”; no NAK
- **[Phase 5]** Caller: `_add_opp_table_indexed` ← DT add with
`getclk=true`; crash via `dev_pm_opp_set_rate`
- **[Phase 5]** clk_get_rate/set_rate/put NULL-safe in this tree
- **[Phase 6]** `git describe` / Makefile: 6.12.111; buggy `clk_get` +
`-ENOENT` block at `core.c:1526-1551`
- **[Phase 8]** Failure mode: oops, CRITICAL
- **UNVERIFIED:** stable@ lore (403); exact crash SoC; whether later
mainline fixed the post-put `opp_table->clk` read
**YES**
drivers/opp/core.c | 54 +++++++++++++++++++++-------------------------
1 file changed, 24 insertions(+), 30 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index a179d57a5a150..a41ad136e90e0 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1579,8 +1579,6 @@ static struct opp_table *_update_opp_table_clk(struct device *dev,
struct opp_table *opp_table,
bool getclk)
{
- int ret;
-
/*
* Return early if we don't need to get clk or we have already done it
* earlier.
@@ -1589,39 +1587,35 @@ static struct opp_table *_update_opp_table_clk(struct device *dev,
opp_table->clks)
return opp_table;
- /* Find clk for the device */
- opp_table->clk = clk_get(dev, NULL);
+ /*
+ * There are few platforms which don't want the OPP core to manage
+ * device's clock settings. In such cases neither the platform
+ * provides the clks explicitly to us, nor the DT contains a valid
+ * clk entry. The OPP nodes in DT may still contain "opp-hz" property
+ * though, which we need to parse and allow the platform to find an
+ * OPP based on freq later on.
+ *
+ * This is a simple solution to take care of such corner cases, i.e.
+ * make the clk_count 1, which lets us allocate space for frequency
+ * in opp->rates and also parse the entries in DT. Use
+ * clk_get_optional() instead of clk_get() so opp_table->clk stays
+ * NULL for such devices, instead of holding an ERR_PTR(-ENOENT) that
+ * consumers must remember to special-case.
+ */
+ opp_table->clk = clk_get_optional(dev, NULL);
- ret = PTR_ERR_OR_ZERO(opp_table->clk);
- if (!ret) {
- opp_table->config_clks = _opp_config_clk_single;
- opp_table->clk_count = 1;
- return opp_table;
+ if (IS_ERR(opp_table->clk)) {
+ dev_pm_opp_put_opp_table(opp_table);
+ dev_err_probe(dev, PTR_ERR(opp_table->clk), "Couldn't find clock\n");
+ return ERR_CAST(opp_table->clk);
}
- if (ret == -ENOENT) {
- /*
- * There are few platforms which don't want the OPP core to
- * manage device's clock settings. In such cases neither the
- * platform provides the clks explicitly to us, nor the DT
- * contains a valid clk entry. The OPP nodes in DT may still
- * contain "opp-hz" property though, which we need to parse and
- * allow the platform to find an OPP based on freq later on.
- *
- * This is a simple solution to take care of such corner cases,
- * i.e. make the clk_count 1, which lets us allocate space for
- * frequency in opp->rates and also parse the entries in DT.
- */
- opp_table->clk_count = 1;
-
- dev_dbg(dev, "%s: Couldn't find clock: %d\n", __func__, ret);
- return opp_table;
- }
+ if (opp_table->clk)
+ opp_table->config_clks = _opp_config_clk_single;
- dev_pm_opp_put_opp_table(opp_table);
- dev_err_probe(dev, ret, "Couldn't find clock\n");
+ opp_table->clk_count = 1;
- return ERR_PTR(ret);
+ return opp_table;
}
/*
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] power: reset: piix4-poweroff: add missing MODULE_DEVICE_TABLE()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (3 preceding siblings ...)
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.1] opp: Use clk_get_optional() to avoid leaving opp_table->clk as an error pointer Sasha Levin
@ 2026-10-19 13:53 ` Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-6.1] tools/lib/thermal: Fix misplaced extern "C" closing brace Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.15] power: supply: cros_pchg: unregister EC notifier Sasha Levin
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2026-10-19 13:53 UTC (permalink / raw)
To: patches, stable
Cc: Pengpeng Hou, Sebastian Reichel, Sasha Levin, sre, linux-pm,
linux-kernel
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
[ Upstream commit ffefc5d9a052f67bc87b57d1107fd7cc1fa1a3d4 ]
The driver has a match table for the pci bus wired into its driver
structure, but the table is not exported with MODULE_DEVICE_TABLE().
Add the missing MODULE_DEVICE_TABLE() entry so module alias information
is generated for automatic module loading.
This is a source-level fix. It does not claim dynamic hardware
reproduction; the evidence is the driver-owned match table, its use by
the driver registration structure, and the missing module alias
publication.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Link: https://patch.msgid.link/20260704152336.51384-1-pengpeng@iscas.ac.cn
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
This tree is **7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`). The driver
has been present since v4.10; the change is a one-line
`MODULE_DEVICE_TABLE()` addition so the existing PCI ID can autoload the
module.
## Phase 1: Commit message forensics
**Step 1.1: Subject line**
Record: **[power/reset: piix4-poweroff] [add] [export the existing PCI
match table via MODULE_DEVICE_TABLE() so the module can autoload]**
**Step 1.2: Tags**
Record:
- **Fixes:** none (expected for this review path)
- **Reported-by / Tested-by / Reviewed-by / Acked-by:** none
- **Link:**
`https://patch.msgid.link/20260704152336.51384-1-pengpeng@iscas.ac.cn`
- **Cc: stable:** none (expected)
- **Signed-off-by:** Pengpeng Hou (author); Sebastian Reichel (power-
supply maintainer)
- No syzbot, no CVE, no kernel test robot
**Step 1.3: Body**
Record:
- **Bug:** `piix4_poweroff_ids[]` is wired into `pci_driver.id_table`
but not published with `MODULE_DEVICE_TABLE()`, so modpost does not
emit PCI aliases.
- **Symptom:** when `CONFIG_POWER_RESET_PIIX4_POWEROFF=m`, userspace
(udev/modprobe) cannot match PCI `8086:7113` to the module, so the
driver is not autoloaded and `pm_power_off` is never installed.
- **Version info:** none in the message. Driver added in 2016
(`29676833df1d9`, first in v4.10-rc1).
- **Root cause:** missing module-alias publication, not a runtime logic
error. Author states this is a **source-level** finding, not a
hardware repro.
**Step 1.4: Hidden bug fix?**
Record: **Not hidden.** The message names the autoload failure directly.
It is a real functional bug even without a crash report.
## Phase 2: Diff analysis
**Step 2.1: Inventory**
Record:
- **File:** `drivers/power/reset/piix4-poweroff.c` — **+1 / −0**
- **Function:** none; one statement after `piix4_poweroff_ids[]`
- **Scope:** single-file, surgical
**Step 2.2: Code flow**
Record:
- **Before:** PCI ID table exists and is used for matching **after** the
module is loaded (`id_table = piix4_poweroff_ids`). No
`MODULE_DEVICE_TABLE`, so no `modules.alias` entry.
- **After:** `MODULE_DEVICE_TABLE(pci, piix4_poweroff_ids)` creates the
alias symbol that `scripts/mod/file2alias.c` (`do_pci_entry`) turns
into `MODULE_ALIAS("pci:v00008086d00007113...")`.
- **Path affected:** module metadata / autoload only. Probe, remove, and
`piix4_poweroff()` are unchanged.
**Step 2.3: Bug mechanism**
Record: **[Hardware enablement / missing module alias]** — match table
present, alias not exported. Same class as adding a PCI ID for autoload.
Not UAF, leak, race, or crash.
**Step 2.4: Fix quality**
Record: **Obviously correct.** Canonical one-macro fix used across
`drivers/power/reset/`. No API, locking, or control-flow change.
**Regression risk: negligible** (build-time alias only).
## Phase 3: Git history
**Step 3.1: Blame**
Record: ID table introduced in **`29676833df1d9`** (“power: reset: Add
Intel PIIX4 poweroff driver”, Paul Burton, 2016-09-28). **`git describe
--contains`:** `v4.10-rc1~120^2~15`. The table never had
`MODULE_DEVICE_TABLE` until this commit. Bug is old, not recent churn.
**Step 3.2: Fixes: tag**
Record: **No Fixes: tag.** Buggy pattern is the original driver, not a
later regression. Introducing commit is an ancestor of this tree.
**Step 3.3: File history**
Record: Only a handful of commits on this file (`29676833df1d9` add,
SPDX, email, `MODULE_DESCRIPTION`, then this). **Standalone**, not patch
2/N. Sibling same-day commit `9da4152d376ca` (cw2015) is a parallel
autoload fix, not a prerequisite.
**Step 3.4: Author**
Record: Pengpeng Hou is not the subsystem maintainer; they have a series
of missing-`MODULE_DEVICE_TABLE` fixes (rtc, vdpa, watchdog, hwmon,
fbdev, this driver). Applied by **Sebastian Reichel**, power-
supply/reset maintainer.
**Step 3.5: Dependencies**
Record: **None.** Only needs `piix4_poweroff_ids` and
`MODULE_DEVICE_TABLE()`, both present here for years. Applies
standalone.
## Phase 4: Mailing list / external research
**Step 4.1: Original discussion**
Record:
- **Lore/msgid:**
`https://patch.msgid.link/20260704152336.51384-1-pengpeng@iscas.ac.cn`
- **`b4 dig -a`:** **v1 only** (2026-07-04). No v2/v3.
- Maintainer reply 2026-07-21: **“Applied, thanks!”** (no requested
changes).
- **No NAK, no stable nomination, no flagged risk.**
- lore.kernel.org HTTP fetch returned **403**; thread recovered via
**`b4 dig -m`**.
**Step 4.2: Recipients (`b4 dig -w`)**
Record: To Sebastian Reichel (`sre@kernel.org`); Cc `linux-
pm@vger.kernel.org`, `linux-kernel@vger.kernel.org`. Right maintainer
and list.
**Step 4.3: Bug report**
Record: **No user/syzbot/bugzilla report.** Evidence is the source
mismatch (table used, not exported).
**Step 4.4: Series**
Record: Standalone 1/1. Parallel same-author autoload patches in other
drivers; this patch does not depend on them.
**Step 4.5: Stable list**
Record: **`site:lore.kernel.org/stable` search returned no hits.**
WebFetch to lore/stable also 403. **No evidence this was rejected for
stable.**
## Phase 5: Semantic / call-chain analysis
**Step 5.1: Functions**
Record: **No function body changed.** New use of `MODULE_DEVICE_TABLE`
next to `piix4_poweroff_ids`. Related runtime functions:
`piix4_poweroff_probe`, `piix4_poweroff_remove`, `piix4_poweroff`.
**Step 5.2: Callers**
Record: `piix4_poweroff_probe` / `remove` only via `module_pci_driver`.
`piix4_poweroff` assigned to `pm_power_off` in probe. No other in-tree
callers (`git grep`).
**Step 5.3: Callees**
Record: Probe uses `pci_request_region`, `pci_resource_start`. Poweroff
path uses PIO and `pci_bus_write_config_dword`. Unchanged by this patch.
**Step 5.4: Reachability**
Record: Userspace-reachable: PCI enumerate → autoload → probe →
`pm_power_off`. `reboot(LINUX_REBOOT_CMD_POWER_OFF)` then runs the
handler. **Without aliases, the module never loads**, so power-off via
this southbridge never registers. Config: `POWER_RESET_PIIX4_POWEROFF`
is **tristate**, `depends on PCI && HAS_IOPORT && (MIPS ||
COMPILE_TEST)` — mainly **MIPS Malta / QEMU Malta**.
**Step 5.5: Similar patterns**
Record: Same one-line fix already used in this directory (`gpio-
poweroff`, `keystone-reset`, `regulator-poweroff`, `at91-poweroff`, `zx-
reboot`, etc.). Established, not novel.
## Phase 6: This checkout
**Step 6.1: Buggy code in this tree?**
Record: **Yes.** File exists; pre-image at `ffefc5d9a052f^` has the ID
table and **no** `MODULE_DEVICE_TABLE`. Bug since **v4.10**. Current
tree already contains the fix (`git merge-base --is-ancestor
ffefc5d9a052f HEAD` succeeded). That is expected for a mainline checkout
of the originating commit, not a reason to treat the bug as absent from
this lineage.
**Step 6.2: Backport difficulty**
Record: **Clean apply** onto any tree that still has the original ID-
table block (verbatim in this file since 2016 aside from nearby module
macros). No refactor of this driver.
**Step 6.3: Related fixes already here?**
Record: **No earlier autoload fix** on this file. `b498ddb6f283c` only
added `MODULE_DESCRIPTION()`.
## Phase 7: Subsystem context
**Step 7.1:**
Record: **[drivers/power/reset] [PERIPHERAL]** — platform power-off, not
mm/VFS/net core. Still the whole purpose of this driver.
**Step 7.2:**
Record: Subsystem is active overall; **this file is mature and almost
untouched**. Long-standing omission, not a new-code fix.
## Phase 8: Impact and risk
**Step 8.1: Who is affected**
Record: **Config- and platform-specific:**
`CONFIG_POWER_RESET_PIIX4_POWEROFF=m` on MIPS (Malta/QEMU) with Intel
PIIX4 PM **8086:7113**. Built-in (`=y`) already works without aliases.
Distros that ship this as a module are the users who benefit.
**Step 8.2: Trigger**
Record: Boot with the PCI device present and the driver as a module,
without a manual `modprobe`. Common for modular distro kernels; **not**
a race. Unprivileged users do not need to trigger it for it to matter —
missing autoload means **power-off never gets hooked**.
**Step 8.3: Failure mode**
Record: **[Module never loads → `pm_power_off` not set by this driver →
`poweroff` does not cut board/QEMU power]**
**Severity: MEDIUM** (functional, not oops/corruption). For this driver
that *is* the user-visible bug.
**Step 8.4: Risk vs benefit**
Record:
- **Benefit:** medium for the small MIPS Malta/QEMU population (driver
actually loads).
- **Risk:** very low (one metadata macro).
- **Ratio:** favorable. Same bar as PCI ID additions, which stable
explicitly allows.
## Phase 9: Synthesis
**Step 9.1: Evidence**
**For:**
- Real autoload bug: ID table used, not exported.
- One line, canonical, maintainer-applied v1.
- Driver in this tree since v4.10; no extra commits required.
- Device-ID / hardware-enablement exception: userspace cannot bind the
existing PCI ID.
- Zero runtime behavior change once the module is loaded.
- Same fix already used throughout `drivers/power/reset/`.
**Against:**
- Not a crash, UAF, deadlock, or security hole.
- Niche (`MIPS || COMPILE_TEST`), old PIIX4/Malta.
- No hardware repro, no `Reported-by`.
- Only matters for `=m`, not `=y`.
**Unresolved:** lore.kernel.org HTTP 403 (thread still obtained via `b4
dig`); no stable-list thread found.
**Step 9.2: Stable rules**
1. Obviously correct and tested? **PASS** — one well-known macro;
maintainer applied v1.
2. Real bug affecting users? **PASS** — modular autoload/power-off fails
on matching PCI.
3. Important (crash/security/corruption/deadlock)? **PASS via device-ID
exception** — enables existing hardware; failure mode is “power-off
driver never attaches,” which is this driver’s job. Not a panic.
4. Small and contained? **PASS** — +1 line, one file.
5. No new features/APIs? **PASS** — publishes an ID table that already
exists.
6. Can apply to this tree? **PASS** — file and table match the pre-
image; no missing helpers.
**Step 9.3: Exception category**
Record: **Device ID / module autoload (hardware enablement).** Closest
documented exception: new device IDs on an existing driver. Here the ID
is already in the driver; only the userspace alias was missing.
**Step 9.4: Decision**
The local 7.3-rc4 tree has this driver and the long-standing omission.
The patch is a zero-risk, one-line autoload fix that makes the existing
PCI match actually load the module. That is appropriate to take into
this tree’s stable-style line.
### What it solves for users of this tree
With `CONFIG_POWER_RESET_PIIX4_POWEROFF=m`, udev cannot load
`piix4-poweroff` for Intel 82371AB PM (`8086:7113`). Probe never runs,
`pm_power_off` is not set, and Malta/QEMU-style power-off does not work
unless the module is loaded by hand. `MODULE_DEVICE_TABLE(pci,
piix4_poweroff_ids)` is the required alias export.
### Risk vs benefit
Benefit is restoring the driver’s purpose on modular MIPS configs. Risk
is essentially none: no runtime path, lock, or ABI change.
---
**Verification:**
- **[Phase 1]** Parsed message/tags: no Fixes/Reported-by/Cc:stable;
Link msgid; SOBs Hou + Reichel
- **[Phase 2]** Diff: +1 line `MODULE_DEVICE_TABLE(pci,
piix4_poweroff_ids)` in `piix4-poweroff.c`
- **[Phase 2]** Confirmed `MODULE_DEVICE_TABLE` in
`include/linux/module.h` creates the alias symbol;
`scripts/mod/file2alias.c` `do_pci_entry` emits `pci:v...d...` aliases
- **[Phase 3]** `git blame`: ID table from `29676833df1d9` (Paul Burton,
2016); MDT line from `ffefc5d9a052f`
- **[Phase 3]** `git describe --contains 29676833df1d9` →
`v4.10-rc1~120^2~15`
- **[Phase 3]** `git log --oneline --
drivers/power/reset/piix4-poweroff.c`: no other autoload fix
- **[Phase 3]** `git merge-base --is-ancestor ffefc5d9a052f HEAD` → yes
(commit is in this tree)
- **[Phase 3]** Pre-image `git show ffefc5d9a052f^:...` matches the
patch context exactly
- **[Phase 4]** `b4 dig -c ffefc5d9a052f`: msgid
`20260704152336.51384-1-pengpeng@iscas.ac.cn`
- **[Phase 4]** `b4 dig -a`: v1 only
- **[Phase 4]** `b4 dig -w`: To Sebastian Reichel; Cc linux-pm, linux-
kernel
- **[Phase 4]** `b4 dig -m /tmp/piix4-mdt.mbox`: maintainer “Applied,
thanks!”; no NAK/stable discussion
- **[Phase 4]** lore HTTP 403; stable web search: no hits —
**UNVERIFIED: full lore HTML; verified via mbox**
- **[Phase 5]** `git grep piix4_poweroff`: only this file; probe sets
`pm_power_off`
- **[Phase 6]** `git describe HEAD` → `v7.3-rc4-27-gf0100363d8c37`;
`make kernelversion` → `7.3.0-rc4`
- **[Phase 6]** Kconfig: tristate, `depends on PCI && HAS_IOPORT &&
(MIPS || COMPILE_TEST)`
- **[Phase 6]** `PCI_DEVICE_ID_INTEL_82371AB_3` = `0x7113` in
`include/linux/pci_ids.h`
- **[Phase 8]** Failure mode: missing autoload → power-off handler never
registered; MEDIUM, not oops
**UNVERIFIED:** Direct lore.kernel.org page content (403). Does not
change the verdict; mbox has the full v1 + apply mail.
**YES**
drivers/power/reset/piix4-poweroff.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/power/reset/piix4-poweroff.c b/drivers/power/reset/piix4-poweroff.c
index e6822c0210000..1a6f3d22d2974 100644
--- a/drivers/power/reset/piix4-poweroff.c
+++ b/drivers/power/reset/piix4-poweroff.c
@@ -96,6 +96,7 @@ static const struct pci_device_id piix4_poweroff_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_3) },
{ 0 },
};
+MODULE_DEVICE_TABLE(pci, piix4_poweroff_ids);
static struct pci_driver piix4_poweroff_driver = {
.name = "piix4-poweroff",
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH AUTOSEL 6.18-6.1] tools/lib/thermal: Fix misplaced extern "C" closing brace
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (4 preceding siblings ...)
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.10] power: reset: piix4-poweroff: add missing MODULE_DEVICE_TABLE() Sasha Levin
@ 2026-10-19 13:53 ` Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.15] power: supply: cros_pchg: unregister EC notifier Sasha Levin
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2026-10-19 13:53 UTC (permalink / raw)
To: patches, stable
Cc: Andreas Haufler, Daniel Lezcano, Sasha Levin, rafael, linux-pm,
linux-kernel
From: Andreas Haufler <andreas@haufler.info>
[ Upstream commit cb0ec27efac2aaf92393ad563a35fb6f00d0a6cd ]
The public libthermal header opens the C++ 'extern "C" {' block inside
the __LIBTHERMAL_H include guard, but places the closing brace after
the guard has already ended:
#endif /* __LIBTHERMAL_H */
#ifdef __cplusplus
}
#endif
On a single inclusion the braces still balance, so the problem is
invisible. On the second inclusion of the header in the same C++
translation unit the include guard skips the opening 'extern "C" {',
while the closing '}' lives outside the guard and is emitted anyway.
This leaves a stray '}' and breaks compilation for any C++ consumer
that includes the header more than once.
Move the closing block inside the include guard so both halves of the
'extern "C"' declaration are guarded consistently.
Signed-off-by: Andreas Haufler <andreas@haufler.info>
Signed-off-by: Daniel Lezcano <daniel.lezcano@kernel.org>
Link: https://patch.msgid.link/20260721083230.91246-1-andreas@haufler.info
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Local tree
This checkout is **pending-6.18**, kernel **6.18.53**
(`p-6.18-1-g03b85e06ca9bf`). The verdict is only about this tree.
Mainline commit: `cb0ec27efac2aaf92393ad563a35fb6f00d0a6cd` (landed
after v6.18; `git describe` reports `v7.2-rc2-24-gcb0ec27efac2a`). It is
**not** an ancestor of HEAD.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: PARSE THE SUBJECT LINE
- **Subsystem:** `tools/lib/thermal` (userspace libthermal, not
`drivers/thermal`)
- **Action verb:** Fix
- **Claimed intent:** Put the C++ `extern "C"` closing brace inside the
include guard
**Record:** `[tools/lib/thermal] [fix] [move misplaced extern "C" }
inside include guard]`
### Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
From `git show cb0ec27efac2a` and the lore thread:
| Tag | Value |
|-----|--------|
| Signed-off-by | Andreas Haufler \<andreas@haufler.info\> (author) |
| Signed-off-by | Daniel Lezcano \<daniel.lezcano@kernel.org\> (thermal
maintainer) |
| Link |
https://patch.msgid.link/20260721083230.91246-1-andreas@haufler.info |
**Absent (expected):** Fixes, Reported-by, Tested-by, Reviewed-by,
Acked-by, Cc: stable.
**Record:** Author + maintainer SOB; one lore Link. No syzbot, no Fixes,
no stable tag.
### Step 1.3: ANALYZE THE COMMIT BODY TEXT
The header opens `extern "C" {` **inside** `#ifndef __LIBTHERMAL_H`, but
closes `}` **after** `#endif`.
- First include: braces balance, so C++ compiles.
- Second include in the same C++ TU: the guard skips the opening brace,
but the closing `}` is still emitted → stray `}` → **compile error**.
**Record:**
- **Bug:** include-guard / `extern "C"` mismatch
- **Symptom:** C++ compile failure on the second include of `thermal.h`
- **Versions:** none stated; intro commit is in this tree since v5.19
- **Root cause:** closing `}` lives outside the guard
### Step 1.4: DETECT HIDDEN BUG FIXES
Not disguised. Subject and body describe a real compile break.
**Record:** Not a hidden fix. Explicit userspace header compile bug.
---
## PHASE 2: DIFF ANALYSIS
### Step 2.1: INVENTORY THE CHANGES
```
tools/lib/thermal/include/thermal.h | 4 ++--
```
One file. Two lines moved: `#endif /* __LIBTHERMAL_H */` is placed after
the `extern "C"` close. No functions changed (header-only).
**Record:** `[thermal.h +2/-2] [no functions] [single-file surgical]`
### Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
**Before** (this tree, lines 178–182):
```178:182:tools/lib/thermal/include/thermal.h
#endif /* __LIBTHERMAL_H */
#ifdef __cplusplus
}
#endif
```
**After:** `#ifdef __cplusplus` / `}` / `#endif` stay inside
`__LIBTHERMAL_H`, then the guard ends.
**Record:** `[header guard close moved after extern "C" close; second
C++ include no longer emits a stray }]`
### Step 2.3: IDENTIFY THE BUG MECHANISM
**Category:** (g) logic/correctness + (h) not hardware; this is a
**build / include-file** defect.
**Mechanism:** On re-include, the guard suppresses `extern "C" {` but
not `}`.
**Record:** `[build/include-guard] [closing brace outside guard]`
### Step 2.4: ASSESS THE FIX QUALITY
Matches the correct pattern in `tools/lib/bpf/libbpf.h` (close `extern
"C"` **then** `#endif /* __LIBBPF_LIBBPF_H */`). Minimal, no API change,
no runtime code. Regression risk is effectively none.
**Record:** `[obviously correct] [no meaningful regression risk]`
---
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: BLAME THE CHANGED LINES
```
47c4b0de080adc (Daniel Lezcano 2022-04-20) #endif /* __LIBTHERMAL_H */
47c4b0de080adc (Daniel Lezcano 2022-04-20) #ifdef __cplusplus
47c4b0de080adc (Daniel Lezcano 2022-04-20) }
```
Introduced with the library itself. Present since v5.19 (`git merge-base
--is-ancestor 47c4b0de080ad v5.19`; not in v5.18).
**Record:** `[47c4b0de080ad "tools/lib/thermal: Add a thermal library"]
[v5.19]`
### Step 3.2: FOLLOW THE FIXES: TAG
No Fixes tag. The buggy code is the original header layout from
47c4b0de080ad, which **is** in this tree.
**Record:** `[no Fixes tag; intro commit is in this tree]`
### Step 3.3: CHECK FILE HISTORY
Only two commits on this file in this tree:
- `47c4b0de080ad` — add library
- `a262672486287` — threshold netlink ABI (does not touch the
guard/`extern "C"`)
No later local fix. `git log v6.18..HEAD --
tools/lib/thermal/include/thermal.h` is empty. File at v6.18 and HEAD
still has the misplaced `}`.
**Record:** `[no related local fixes] [standalone] [not a series]`
### Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS
`git log --author="Andreas Haufler" -- tools/` is empty in this tree.
Author is an external contributor. Maintainer Daniel Lezcano committed
it (`CommitDate: Fri Aug 7 17:18:51 2026`) after “Applied, thanks for
the fix”.
**Record:** `[external author; thermal maintainer applied]`
### Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS
Diff assumes only `#endif` / `#ifdef __cplusplus` / `}`. Those lines
exist unchanged. Standalone.
**Record:** `[no dependencies] [applies standalone]`
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
### Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION
`b4 dig -c cb0ec27efac2a` matched by patch-id:
- **Lore:**
https://patch.msgid.link/20260721083230.91246-1-andreas@haufler.info
- **v1 only** (`b4 dig -a`): no v2/v3
- Thread: patch + Daniel Lezcano reply: **“Applied, thanks for the
fix”**
- No NAK, no stable nomination, no requested rework
WebFetch to lore returned 403; thread was recovered via `b4 mbox`.
**Record:** `[v1 accepted as-is] [maintainer Applied] [no stable
mention] [no NAKs]`
### Step 4.2: CHECK WHO REVIEWED THE PATCH
`b4 dig -w` recipients: Rafael Wysocki, Daniel Lezcano, Zhang Rui,
Lukasz Luba, linux-pm, linux-kernel. Those are the THERMAL
maintainers/reviewers in MAINTAINERS.
**Record:** `[thermal maintainers on To/Cc; Lezcano applied]`
### Step 4.3: SEARCH FOR THE BUG REPORT
No Reported-by, bugzilla, or syzbot. Author found it by inspecting the
public header.
**Record:** `[no external bug report] [author-discovered compile
defect]`
### Step 4.4: CHECK FOR RELATED PATCHES AND SERIES
Standalone `[PATCH]`, msgid `...91246-1`, no `-2`. `git log
origin/master cb0ec27efac2a^.. -- tools/lib/thermal/include/thermal.h`
shows only this commit.
**Record:** `[standalone; no follow-up on the header]`
### Step 4.5: CHECK STABLE MAILING LIST HISTORY
lore.kernel.org/stable WebFetch returned 403. No Cc: stable on the
patch. No local `git log --grep=extern -- tools/lib/thermal/` hits.
**Record:** `[no stable-list discussion verified; lore/stable fetch
failed with 403]`
---
## PHASE 5: CODE SEMANTIC ANALYSIS
semcode MCP is not available. Grep/read used instead.
### Step 5.1: IDENTIFY KEY FUNCTIONS
No functions modified. Change is preprocessor layout at the bottom of
`thermal.h`.
**Record:** `[none; include guard / extern "C" only]`
### Step 5.2: TRACE CALLERS
In-tree includes of this header:
- `tools/lib/thermal/{thermal,events,commands,thermal_nl,sampling}.c`
- `tools/thermal/thermal-engine/thermal-engine.c` (`thermal_init`,
`thermal_events_handle`)
All **C**. `find tools/thermal tools/lib/thermal -name '*.cpp'` is
empty. The header is still a **public installed API** (`install_headers`
installs `include/thermal.h`; `libthermal.pc`).
**Record:** `[in-tree users are C; public header is C++-aware and
installed]`
### Step 5.3: TRACE CALLEES
N/A for a brace move.
**Record:** `[N/A]`
### Step 5.4: FOLLOW THE CALL CHAIN
Trigger is **userspace C++** including `thermal.h` twice (direct or via
two headers). Not a syscall/kernel path. Reachable for anyone who
installs libthermal and uses it from C++.
**Record:** `[userspace C++ re-include] [not kernel runtime]`
### Step 5.5: SEARCH FOR SIMILAR PATTERNS
`tools/lib/bpf/libbpf.h` and `libbpf_legacy.h` close `extern "C"`
**inside** the guard. This patch matches that.
**Record:** `[libbpf already uses the correct pattern; libthermal was
the outlier]`
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
### Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?
Yes. `tools/lib/thermal/include/thermal.h` lines 178–182 still have
`#endif` before the closing `}`. Introduced in v5.19; present at v6.18
and HEAD.
`git merge-base --is-ancestor cb0ec27efac2a HEAD` → **not in this
tree**.
**Record:** `[bug present at thermal.h:178-182] [since v5.19] [fix not
applied]`
### Step 6.2: CHECK FOR BACKPORT COMPLICATIONS
Context around the hunk is identical to the mainline diff. No churn on
this file after v6.18. Clean apply.
**Record:** `[clean apply]`
### Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE
No. File history has no other extern-C/guard fix.
**Record:** `[no existing equivalent fix]`
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
### Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY
`tools/lib/thermal/` — userspace thermal netlink library (THERMAL in
MAINTAINERS: Rafael Wysocki, Daniel Lezcano).
**Criticality:** PERIPHERAL for kernel runtime; IMPORTANT as a **public
installed userspace API** for thermal tools. Not mm/fs/net core.
**Record:** `[tools/lib/thermal] [PERIPHERAL kernel / public userspace
header]`
### Step 7.2: ASSESS SUBSYSTEM ACTIVITY
Low churn: library add (2022), threshold ABI (2024), a few
build/leak/symbol fixes. Mature header layout, not a moving target.
**Record:** `[low activity; header layout stable since 2022]`
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: DETERMINE WHO IS AFFECTED
C++ programs that include the installed `thermal.h` more than once. In-
tree C tools (`thermal-engine`) are unaffected. Kernel image/build is
unaffected.
**Record:** `[config/userspace-specific: C++ libthermal consumers]`
### Step 8.2: DETERMINE THE TRIGGER CONDITIONS
Second `#include <thermal.h>` in one C++ translation unit (common with
umbrella headers). Unprivileged userspace compile-time only. Not a race.
**Record:** `[double include from C++] [uncommon but valid] [compile-
time only]`
### Step 8.3: DETERMINE THE FAILURE MODE SEVERITY
Compile failure (`stray '}'`), not oops/corruption/UAF. Stable rules
still list **build errors** and **include-file fixes** as in-scope.
**Record:** `[C++ compile break] [MEDIUM as a build bug; HIGH as
include-file correctness; not a kernel crash]`
### Step 8.4: CALCULATE RISK-BENEFIT RATIO
- **Benefit:** C++ consumers of the public header compile; long-standing
defect (v5.19+).
- **Risk:** 4-line preprocessor move, no runtime, no ABI change.
**Record:** `[moderate benefit for C++ libthermal users] [near-zero
risk] [favorable]`
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: COMPILE THE EVIDENCE
**FOR:**
- Real, well-explained compile break for a public C++-aware header
- Obviously correct; matches libbpf
- 4 lines; zero runtime/API risk
- Buggy code is in this 6.18.53 tree; fix is not
- Applies cleanly; no prerequisites
- Maintainer applied v1 with “thanks for the fix”
- Fits the **build / include-file** exception in stable-kernel-rules
**AGAINST:**
- Not a kernel oops, UAF, deadlock, or security hole
- In-tree tools are C and already build
- No multi-user Reported-by
- libthermal is a small audience
**UNRESOLVED:**
- Could not search lore.kernel.org/stable (HTTP 403). Unverified whether
anyone asked for this on the stable list.
### Step 9.2: APPLY THE STABLE RULES CHECKLIST
1. Obviously correct and tested? **PASS** — mechanical brace move;
maintainer applied
2. Fixes a real bug that affects users? **PASS** — C++ double-include of
the installed header fails
3. Important issue? **PASS** — build/include-file error (explicitly
allowed); not a crash, but in-scope
4. Small and contained? **PASS** — 4 lines, one header
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — context matches HEAD
### Step 9.3: CHECK FOR EXCEPTION CATEGORIES
**Build / include-file fix.** Not a device ID, quirk, DT, or docs-only
change.
### Step 9.4: DECISION
This tree ships the buggy public header. The fix is a 4-line, zero-risk
compile repair that mainline already took. That meets stable rules for
this 6.18 tree.
---
## What the commit solves for this tree
`thermal.h` advertises C++ support (`extern "C"`) and is installed
(`make install` → `include/thermal.h`). The closing `}` sits outside
`__LIBTHERMAL_H`, so a second include in one C++ file emits a stray
brace and fails the build. Moving `#endif` below the close matches every
other well-formed kernel userspace header. Kernel runtime is untouched.
Risk vs benefit: benefit is restoring a header that this tree already
ships; risk is none.
---
## Verification
- **[Phase 1]** `git show cb0ec27efac2a`: tags are author SOB, Lezcano
SOB, Link; no Fixes/Reported-by/Cc: stable
- **[Phase 1]** Body describes second-include stray `}` — matches the
current header
- **[Phase 2]** Diff is `#endif` moved after `#ifdef __cplusplus` / `}`
/ `#endif`
- **[Phase 2]** `tools/lib/bpf/libbpf.h` uses the same close-then-guard-
end order
- **[Phase 3]** `git blame tools/lib/thermal/include/thermal.h` lines
178–182 → `47c4b0de080ad`
- **[Phase 3]** `git merge-base --is-ancestor 47c4b0de080ad v5.19` yes;
`v5.18` no
- **[Phase 3]** `git log --oneline --
tools/lib/thermal/include/thermal.h`: only add + threshold ABI
- **[Phase 3]** `git merge-base --is-ancestor cb0ec27efac2a HEAD` → not
in this tree
- **[Phase 4]** `b4 dig -c cb0ec27efac2a`: lore URL + v1 only
- **[Phase 4]** `b4 dig -a`: single revision 2026-07-21
- **[Phase 4]** `b4 dig -w`: Rafael, Lezcano, Zhang Rui, Lukasz Luba,
linux-pm, linux-kernel
- **[Phase 4]** `b4 mbox`: Lezcano “Applied, thanks for the fix”
(2026-08-07)
- **[Phase 4]** WebFetch lore/all, patch.msgid.link, lore/linux-pm,
lore/stable → 403
- **[Phase 5]** No `*.cpp` under `tools/thermal` or `tools/lib/thermal`
- **[Phase 5]** In-tree include sites are C; `thermal-engine.c` calls
`thermal_init`
- **[Phase 5]** Makefile `install_headers` installs `include/thermal.h`
- **[Phase 6]** `make kernelversion` / Makefile → 6.18.53; `git describe
HEAD` → `p-6.18-1-g03b85e06ca9bf`
- **[Phase 6]** HEAD `thermal.h` still has `#endif` before `}`
- **[Phase 6]** `git log v6.18..HEAD --
tools/lib/thermal/include/thermal.h` empty
- **[Phase 7]** MAINTAINERS THERMAL covers `tools/lib/thermal/`
- **[Phase 8]** Failure mode verified as C++ compile-time only, not
kernel oops
- **UNVERIFIED:** stable@ mailing-list archive (HTTP 403); does not
change the verdict
The local 6.18.53 tree has the bug, the fix is small and correct, and it
applies cleanly.
**YES**
tools/lib/thermal/include/thermal.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/lib/thermal/include/thermal.h b/tools/lib/thermal/include/thermal.h
index 818ecdfb46e50..d9097271d9fad 100644
--- a/tools/lib/thermal/include/thermal.h
+++ b/tools/lib/thermal/include/thermal.h
@@ -175,8 +175,8 @@ LIBTHERMAL_API thermal_error_t thermal_sampling_handle(struct thermal_handler *t
LIBTHERMAL_API int thermal_sampling_fd(struct thermal_handler *th);
-#endif /* __LIBTHERMAL_H */
-
#ifdef __cplusplus
}
#endif
+
+#endif /* __LIBTHERMAL_H */
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH AUTOSEL 6.18-5.15] power: supply: cros_pchg: unregister EC notifier
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (5 preceding siblings ...)
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-6.1] tools/lib/thermal: Fix misplaced extern "C" closing brace Sasha Levin
@ 2026-10-19 13:53 ` Sasha Levin
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2026-10-19 13:53 UTC (permalink / raw)
To: patches, stable
Cc: Pengpeng Hou, Tzung-Bi Shih, Sebastian Reichel, Sasha Levin, sre,
linux-pm, linux-kernel
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
[ Upstream commit 631adfa03595c63b2a621cdc62db60bb1ce5f91c ]
cros_pchg_probe() registers an EC event notifier whose callback uses the
devm-allocated charger_data via container_of(). The driver has no
remove callback and does not unregister the notifier, so the notifier
chain can retain a pointer to freed driver state after unbind or probe
cleanup.
Register a devm cleanup action immediately after the notifier is
installed so the notifier is unregistered before the driver state is
released. Also fail probe if the notifier cannot be registered, instead
of leaving a charger device that cannot receive EC events.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
Link: https://patch.msgid.link/20260615063105.39152-1-pengpeng@iscas.ac.cn
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit message forensics
**Step 1.1: Subject line**
Record: `[power: supply: cros_pchg]` `[unregister]` `[stop leaving an EC
event notifier pointing at freed charger state]`
**Step 1.2: Tags**
Record:
- `Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>` (author)
- `Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>` (Chrome OS platform
maintainer)
- `Link:
https://patch.msgid.link/20260615063105.39152-1-pengpeng@iscas.ac.cn`
- `Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>`
(power-supply maintainer; applied the patch)
- No `Fixes:`, no `Cc: stable`, no `Reported-by`, no `Tested-by`, no
syzbot/CVE tags (absence is expected)
**Step 1.3: Body**
Record:
- **Bug:** `cros_pchg_probe()` registers `charger->notifier` on the
parent Chrome OS EC `event_notifier` chain. The callback uses
`container_of()` to recover `struct charger_data`, which is
`devm_kzalloc()`’d. The platform driver has no `.remove` and never
unregisters the notifier.
- **Symptom:** After unbind/module unload (and, as claimed, probe
unwind), the parent EC chain still holds a pointer into freed driver
state. The next EC MKBP event can call into that freed object.
- **Root cause:** Notifier lifetime is tied to the parent
`cros_ec_device`, not to the child pchg platform device.
- **Fix:** `devm_add_action_or_reset()` immediately after successful
register; also fail probe if register fails instead of leaving a
charger that cannot receive events.
- **Versions:** none stated in the message.
**Step 1.4: Hidden bug fix?**
Record: Not hidden. This is an explicit use-after-free / dangling-
notifier fix. The “fail probe on register failure” part is a real
correctness fix, not a feature.
## Phase 2: Diff analysis
**Step 2.1: Inventory**
Record:
- File: `drivers/power/supply/cros_peripheral_charger.c` (`+11 / -2`)
- Functions: new `cros_pchg_unregister_notifier()`; `cros_pchg_probe()`
changed
- Scope: single-file surgical fix
**Step 2.2: Code flow**
Record, from the pre-fix file (`631adfa03595c^`):
- **Before:** After registering ports, probe does
`blocking_notifier_chain_register(&ec_dev->ec_dev->event_notifier,
nb)`. On failure it only logs and still `return 0`. There is no
`.remove`. `charger` is released by devm on unbind.
- **After:** Register failure returns `dev_err_probe(...)`. On success,
`devm_add_action_or_reset(dev, cros_pchg_unregister_notifier,
charger)` unregisters from `charger->ec_device->event_notifier` before
other devm resources (including `charger`) are freed.
`devm_add_action_or_reset()` also runs the unregister callback if
adding the action itself fails.
**Step 2.3: Bug mechanism**
Record: **Use-after-free / dangling notifier (memory safety + ref-
lifetime).** Secondary logic fix: probe no longer succeeds without a
working notifier. Not a leak-only cleanup.
Verified mechanism:
- `cros_ec_notify()` does `container_of(nb, struct charger_data,
notifier)` then `cros_pchg_event(charger)`.
- Parent EC IRQ path `cros_ec_handle_event()` /
`cros_ec_report_events_during_suspend()` call
`blocking_notifier_call_chain(&ec_dev->event_notifier, ..., ec_dev)`
in `drivers/platform/chrome/cros_ec.c`.
- The pchg device is an MFD child (`cros-ec-pchg` in
`drivers/mfd/cros_ec_dev.c`), added when `EC_CMD_PCHG_COUNT` reports
ports. The chain lives on the parent `cros_ec_device`, which outlives
a child unbind / `rmmod` of `CHARGER_CROS_PCHG`.
**Step 2.4: Fix quality**
Record: Obviously correct and the same pattern already used by
`cros_usbpd-charger.c`. Minimal. Regression risk is low: `devm` LIFO
order unregisters before `charger` is freed; unregister is only
scheduled after a successful register. Failing probe when notifier
registration fails is stricter but matches sibling Chrome EC drivers
(`rtc-cros-ec`, `cros_ec_keyb`, `extcon-usbc-cros-ec`).
`blocking_notifier_chain_register()` rarely fails.
## Phase 3: Git history
**Step 3.1: Blame**
Record: Notifier registration and the missing remove hook come from
`56d629af09b9d` (“power: supply: PCHG: Peripheral device charger”,
2021-07-16). That commit is an ancestor of `v5.15` (`git merge-base
--is-ancestor` exit 0 for v5.15, 1 for v5.14). The bug is old, not last-
month code.
**Step 3.2: Fixes: tag**
Record: None. Introducer is `56d629af09b9d`, present in this tree.
**Step 3.3: File history**
Record (recent, this file): `631adfa03595c` (this fix), `d6486a13665e9`
(ID table), `97dd69b1ade16` (resume sync). No prerequisite refactor.
Standalone, not `patch X/Y`.
**Step 3.4: Author**
Record: Pengpeng Hou has one other power-supply commit here
(`cw2015_battery` MODULE_DEVICE_TABLE). Not the subsystem maintainer.
Weight comes from reviewer Tzung-Bi Shih (Chrome platform) and applier
Sebastian Reichel (power-supply).
**Step 3.5: Dependencies**
Record: Uses `devm_add_action_or_reset()` and `dev_err_probe()`, both
long-standing. No new helpers. Can apply standalone. No revert of this
commit found.
## Phase 4: Mailing list
**Step 4.1: Original discussion**
Record: `b4 dig -c 631adfa03595c` matched
https://patch.msgid.link/20260615063105.39152-1-pengpeng@iscas.ac.cn
(v1 only; subject used `cros_peripheral_charger`, maintainer retitled to
`cros_pchg`). Thread saved via `b4 dig -m`.
**Step 4.2: Recipients / review**
Record (`b4 dig -w`): To/Cc included Sebastian Reichel, linux-pm, linux-
kernel, Benson Leung, Tzung-Bi Shih, Guenter Roeck, chrome-platform.
Tzung-Bi’s only comment was a subject-line nit; `Reviewed-by` “with or
without this”. Sebastian applied it (“Applied, thanks!”). No NAKs, no
stable nomination in-thread.
**Step 4.3: Bug report**
Record: No user crash/syzbot report. A later duplicate submission
(Hongyan Xu / Patchew) says the same hole was found by static analysis.
That still describes a real UAF, not a style nit.
**Step 4.4: Series**
Record: Single standalone patch (`[1/1]`).
**Step 4.5: Stable list**
Record: lore.kernel.org/stable search returned HTTP 403. UNVERIFIED
whether anyone already asked for this on stable@. No evidence it was
rejected.
## Phase 5: Semantic / call graph
**Step 5.1: Functions**
Record: `cros_pchg_unregister_notifier` (new), `cros_pchg_probe`
(modified). Callback still `cros_ec_notify` → `cros_pchg_event`.
**Step 5.2: Callers**
Record: `cros_pchg_probe` is the platform `.probe` for `cros-ec-pchg`.
Instantiated from `ec_device_probe()` in `cros_ec_dev.c` when the EC
reports PCHG ports. `cros_ec_notify` is invoked from the parent EC
IRQ/event path (`cros_ec_irq_thread` → `cros_ec_handle_event` →
`blocking_notifier_call_chain`). That is a live interrupt/event path,
not init-only.
**Step 5.3: Callees**
Record: `blocking_notifier_chain_register/unregister`,
`devm_add_action_or_reset`, `dev_err_probe`. No new allocations or locks
beyond the notifier API.
**Step 5.4: Reachability**
Record: Userspace can trigger unbind (`/sys/bus/platform/drivers/cros-
ec-pchg/unbind`) or `rmmod` when `CONFIG_CHARGER_CROS_PCHG=m`. After
that, any EC MKBP event on the still-live parent walks the dangling
notifier. Unprivileged users cannot usually unbind, but module unload,
sysfs unbind, and parent teardown are real. After UAF,
`cros_pchg_event()` walks `charger->ports[]` and may call
`power_supply_changed()` — crash or memory corruption.
**Step 5.5: Similar patterns**
Record: Sibling `cros_usbpd-charger.c` already uses
`devm_add_action_or_reset(..., cros_usbpd_charger_unregister_notifier,
...)`. Other Chrome EC clients (`rtc-cros-ec`, `cros_ec_keyb`, `extcon-
usbc-cros-ec`) unregister in `.remove`. This driver was the outlier.
## Phase 6: This checkout
**Step 6.1: Buggy code in this tree?**
Record: Local tree is **v7.3-rc4** (`git describe HEAD` =
`v7.3-rc4-27-gf0100363d8c37`, `VERSION=7 PATCHLEVEL=3 SUBLEVEL=0
EXTRAVERSION=-rc4`). File exists. Driver introduced in v5.15; notifier
bug present from that introduction. Pre-fix probe (shown via `git show
631adfa03595c^`) matches the candidate diff. `git merge-base --is-
ancestor 631adfa03595c HEAD` succeeds (the fix is already on this
mainline branch, as expected for a candidate sourced from it).
**Step 6.2: Backport difficulty**
Record: Clean apply on this file. Surrounding probe logic is the
original 2021 structure plus later resume/ID-table work; this hunk does
not depend on those.
**Step 6.3: Related fixes already here**
Record: This is the fix. No earlier alternate fix on the file. No
revert.
## Phase 7: Subsystem context
**Step 7.1:**
Record: `drivers/power/supply/` Chrome OS EC peripheral charger.
**IMPORTANT** (not core mm/vfs, but default-on for
`CONFIG_MFD_CROS_EC_DEV` Chromebooks with stylus/WLC ports).
**Step 7.2:**
Record: File is mature; few commits since 2021. Not a newly landed
subsystem.
## Phase 8: Impact and risk
**Step 8.1: Who is affected**
Record: Chrome OS EC machines with peripheral charge ports
(`EC_CMD_PCHG_COUNT` > 0). `CONFIG_CHARGER_CROS_PCHG` defaults to
`MFD_CROS_EC_DEV`. Driver-specific, not universal.
**Step 8.2: Trigger**
Record: Successful probe, then unbind/rmmod/MFD child removal while the
parent EC device (and its IRQ) remain. Not every boot. Timing-dependent
once the dangling notifier is left on the chain: the next PCHG (or any)
EC event walks the chain. `cros_ec_notify` returns `NOTIFY_DONE` for
non-PCHG events but **after** `container_of()` — so *any* EC event after
unbind is already a UAF.
**Step 8.3: Failure mode**
Record: Use-after-free in notifier callback → kernel oops or memory
corruption. **Severity: HIGH.**
**Step 8.4: Risk/benefit**
Record: **Benefit: high** for Chromebooks that unbind or unload this
driver (and for teardown races). **Risk: very low** (13 lines,
established `devm` pattern, Chrome maintainer review). Ratio favors
backport.
## Phase 9: Synthesis
**Step 9.1: Evidence**
For:
- Real dangling-notifier UAF: parent EC chain keeps `notifier_block`
inside `devm`-freed `charger_data`
- Any post-unbind EC event is UAF (even events this driver ignores)
- Small, obviously correct, matches `cros_usbpd-charger.c`
- Reviewed by Chrome platform maintainer; applied by power-supply
maintainer
- Driver present since v5.15; no extra dependencies
- Fail-probe-on-register-failure avoids a half-working charger
Against:
- No `Reported-by` / syzbot / production crash (static analysis)
- Affects only Chrome OS PCHG hardware
- Unbind is not the common path on always-on Chromebooks
- Slightly stricter probe (register failure now fails the device)
Unresolved:
- Stable@ discussion (lore 403)
- How often users actually unbind this device in the field
**Step 9.2: Stable rules**
1. Obviously correct and tested? **PASS** — pattern is standard;
maintainer Reviewed-by + applied
2. Real bug affecting users? **PASS** — UAF after unbind/unload while
parent EC is live
3. Important (crash/security/corruption)? **PASS** — UAF →
oops/corruption, HIGH
4. Small and contained? **PASS** — 13 lines, one file
5. No new features/APIs? **PASS**
6. Can apply to this tree? **PASS** — file and APIs exist; hunk is self-
contained
**Step 9.3: Exception category**
Record: None needed; this is a normal UAF bugfix.
**Step 9.4: Decision**
This is a classic stable-worthy UAF: the child’s notifier is left on a
parent-owned chain. The local 7.3 tree has this driver; the fix is small
and matches existing Chrome EC practice. Trigger is unbind/teardown
rather than every boot, but the failure mode is a kernel UAF, which
stable takes.
What it solves for stable users: Chromebooks with peripheral chargers no
longer keep a freed `charger_data` on the EC event chain after driver
unbind, module unload, or MFD child removal, so a later EC interrupt
cannot oops or corrupt memory in `cros_ec_notify()`.
Risk vs benefit: very low risk, high benefit on the machines that hit
teardown. No prerequisites beyond the driver itself.
---
Verification:
- [Phase 1] Parsed tags: Reviewed-by Tzung-Bi Shih; Link to msgid; no
Fixes/stable/syzbot
- [Phase 1] Body describes dangling notifier after unbind; verified
against pre-fix source
- [Phase 2] Diff: +11/-2 in `cros_pchg_probe` plus
`cros_pchg_unregister_notifier`; fail probe on register error
- [Phase 2] Pre-fix file from `git show 631adfa03595c^`: register then
`return 0`; no `.remove`
- [Phase 2] Post-fix file: `devm_add_action_or_reset` after successful
register
- [Phase 3] `git blame`: notifier register from `56d629af09b9d` (2021);
`git merge-base --is-ancestor` → in v5.15, not v5.14
- [Phase 3] `git log --oneline -20 --
drivers/power/supply/cros_peripheral_charger.c`: standalone, not a
series
- [Phase 3] No revert of this commit
- [Phase 4] `b4 dig -c 631adfa03595c`: lore thread found; v1 only
- [Phase 4] `b4 dig -w`: Chrome/power-supply maintainers CC’d
- [Phase 4] `b4 dig -m`: Tzung-Bi Reviewed-by (subject nit only);
Sebastian applied; no NAK
- [Phase 4] Duplicate later patch (Hongyan Xu) notes static-analysis
origin
- [Phase 4] lore.kernel.org WebFetch 403 — stable@ history UNVERIFIED
- [Phase 5] `cros_ec_handle_event` / resume path call
`blocking_notifier_call_chain` on the same `event_notifier`
- [Phase 5] MFD cell `cros-ec-pchg` in `drivers/mfd/cros_ec_dev.c` when
`pchg_count.port_count != 0`
- [Phase 5] Sibling `cros_usbpd-charger.c` already uses the same `devm`
unregister pattern
- [Phase 6] `git describe HEAD` = v7.3-rc4-27-gf0100363d8c37; `make
kernelversion` = 7.3.0-rc4
- [Phase 6] Driver and APIs present; fix is self-contained on this tree
- [Phase 8] UAF on any EC event after unbind: `container_of` happens
before the PCHG event-type check
- UNVERIFIED: field frequency of unbind; whether stable@ already
discussed it
The fix is small, surgical, and meets the stable criteria for a real
UAF.
**YES**
drivers/power/supply/cros_peripheral_charger.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/power/supply/cros_peripheral_charger.c b/drivers/power/supply/cros_peripheral_charger.c
index 962a6fd298323..48e10e872d1bd 100644
--- a/drivers/power/supply/cros_peripheral_charger.c
+++ b/drivers/power/supply/cros_peripheral_charger.c
@@ -259,6 +259,14 @@ static int cros_ec_notify(struct notifier_block *nb,
return cros_pchg_event(charger);
}
+static void cros_pchg_unregister_notifier(void *data)
+{
+ struct charger_data *charger = data;
+
+ blocking_notifier_chain_unregister(&charger->ec_device->event_notifier,
+ &charger->notifier);
+}
+
static int cros_pchg_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -346,9 +354,10 @@ static int cros_pchg_probe(struct platform_device *pdev)
ret = blocking_notifier_chain_register(&ec_dev->ec_dev->event_notifier,
nb);
if (ret < 0)
- dev_err(dev, "Failed to register notifier (err:%d)\n", ret);
+ return dev_err_probe(dev, ret, "Failed to register notifier\n");
- return 0;
+ return devm_add_action_or_reset(dev, cros_pchg_unregister_notifier,
+ charger);
}
#ifdef CONFIG_PM_SLEEP
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-19 14:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-6.12] powercap: intel_rapl: Fix kernel panic during PMU unbind Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.6] pmdomain: qcom: rpmhpd: Skip retention by default Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.6] thermal: intel: int340x: clean up RFIM groups on DVFS failure Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.1] opp: Use clk_get_optional() to avoid leaving opp_table->clk as an error pointer Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.10] power: reset: piix4-poweroff: add missing MODULE_DEVICE_TABLE() Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-6.1] tools/lib/thermal: Fix misplaced extern "C" closing brace Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.15] power: supply: cros_pchg: unregister EC notifier Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox