From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Lifeng Zheng <zhenglifeng1@huawei.com>,
"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
Sasha Levin <sashal@kernel.org>,
rafael@kernel.org, viresh.kumar@linaro.org,
linux-pm@vger.kernel.org
Subject: [PATCH AUTOSEL 6.16 24/85] cpufreq: Exit governor when failed to start old governor
Date: Sun, 3 Aug 2025 20:22:33 -0400 [thread overview]
Message-ID: <20250804002335.3613254-24-sashal@kernel.org> (raw)
In-Reply-To: <20250804002335.3613254-1-sashal@kernel.org>
From: Lifeng Zheng <zhenglifeng1@huawei.com>
[ Upstream commit 0ae204405095abfbc2d694ee0fbb49bcbbe55c57 ]
Detect the result of starting old governor in cpufreq_set_policy(). If it
fails, exit the governor and clear policy->governor.
Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
Link: https://patch.msgid.link/20250709104145.2348017-5-zhenglifeng1@huawei.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:
**Backport Status: YES**
This commit should be backported to stable kernel trees. Here's my
extensive analysis:
## Bug Analysis
The commit fixes a **resource leak and inconsistent state bug** in the
cpufreq governor switching code. Looking at the code changes in
`cpufreq_set_policy()`:
### The Bug:
In the original code (lines 2716-2721), when switching governors fails
and we need to restore the old governor:
```c
if (old_gov) {
policy->governor = old_gov;
if (cpufreq_init_governor(policy))
policy->governor = NULL;
else
cpufreq_start_governor(policy); // Bug: no error handling here
}
```
If `cpufreq_start_governor()` fails, the code doesn't handle the error.
This leaves the system in an **inconsistent state** where:
1. The governor is initialized (`cpufreq_init_governor` succeeded)
2. But the governor failed to start (`cpufreq_start_governor` failed)
3. The policy still points to a non-functional governor
4. Resources allocated during `cpufreq_init_governor` are **leaked**
(module reference count, governor's init() allocations)
### The Fix:
```c
if (cpufreq_init_governor(policy)) {
policy->governor = NULL;
} else if (cpufreq_start_governor(policy)) {
cpufreq_exit_governor(policy); // NEW: Clean up on failure
policy->governor = NULL; // NEW: Clear the governor pointer
}
```
## Why This Should Be Backported:
1. **Fixes a Real Bug**: This addresses a resource leak where
`cpufreq_init_governor()` acquires resources (notably
`try_module_get()` at line 2442 and potential governor->init()
allocations) that aren't cleaned up if `cpufreq_start_governor()`
fails.
2. **Small and Contained Fix**: The change is minimal - just 4 lines
modified to add proper error handling. It doesn't change any APIs or
introduce new functionality.
3. **Prevents System Instability**: Leaving the cpufreq subsystem in an
inconsistent state (initialized but not started governor) could lead
to:
- Module reference count leaks
- Memory leaks from governor init allocations
- Potential crashes if the partially-initialized governor is accessed
later
4. **Error Path Fix**: This is clearly an error handling path that was
incorrectly implemented. The pattern of calling
`cpufreq_exit_governor()` after a failed `cpufreq_start_governor()`
is already used elsewhere in the code (line 2711).
5. **No Architectural Changes**: The fix simply adds missing cleanup
code in an error path. It doesn't change the normal operation flow or
introduce new features.
6. **Critical Subsystem**: The cpufreq subsystem is critical for power
management and system stability. Bugs here can affect system
reliability.
The commit follows the stable kernel rules perfectly - it's a clear bug
fix that prevents resource leaks and system instability, with minimal
risk of regression since it only affects an error path that was already
broken.
drivers/cpufreq/cpufreq.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index d7426e1d8bdd..10a22c6f5519 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2715,10 +2715,12 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
pr_debug("starting governor %s failed\n", policy->governor->name);
if (old_gov) {
policy->governor = old_gov;
- if (cpufreq_init_governor(policy))
+ if (cpufreq_init_governor(policy)) {
policy->governor = NULL;
- else
- cpufreq_start_governor(policy);
+ } else if (cpufreq_start_governor(policy)) {
+ cpufreq_exit_governor(policy);
+ policy->governor = NULL;
+ }
}
return ret;
--
2.39.5
next prev parent reply other threads:[~2025-08-04 0:24 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20250804002335.3613254-1-sashal@kernel.org>
2025-08-04 0:22 ` [PATCH AUTOSEL 6.16 05/85] cpufreq: CPPC: Mark driver with NEED_UPDATE_LIMITS flag Sasha Levin
2025-08-04 0:22 ` Sasha Levin [this message]
2025-08-04 0:22 ` [PATCH AUTOSEL 6.16 25/85] cpufreq: intel_pstate: Add Granite Rapids support in no-HWP mode Sasha Levin
2025-08-04 0:22 ` [PATCH AUTOSEL 6.16 27/85] PM / devfreq: governor: Replace sscanf() with kstrtoul() in set_freq_store() Sasha Levin
2025-08-04 0:22 ` [PATCH AUTOSEL 6.16 30/85] thermal/drivers/qcom-spmi-temp-alarm: Enable stage 2 shutdown when required Sasha Levin
2025-08-04 0:22 ` [PATCH AUTOSEL 6.16 43/85] PM: runtime: Clear power.needs_force_resume in pm_runtime_reinit() Sasha Levin
2025-08-04 0:22 ` [PATCH AUTOSEL 6.16 44/85] thermal: sysfs: Return ENODATA instead of EAGAIN for reads Sasha Levin
2025-08-04 0:22 ` [PATCH AUTOSEL 6.16 45/85] PM: sleep: console: Fix the black screen issue Sasha Levin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250804002335.3613254-24-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=patches@lists.linux.dev \
--cc=rafael.j.wysocki@intel.com \
--cc=rafael@kernel.org \
--cc=stable@vger.kernel.org \
--cc=viresh.kumar@linaro.org \
--cc=zhenglifeng1@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).