Archive-only list for patches
 help / color / mirror / Atom feed
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 5.10 10/39] cpufreq: Exit governor when failed to start old governor
Date: Sun,  3 Aug 2025 20:40:12 -0400	[thread overview]
Message-ID: <20250804004041.3628812-10-sashal@kernel.org> (raw)
In-Reply-To: <20250804004041.3628812-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 6294e10657b4..2f1e8669a15b 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2544,10 +2544,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


  parent reply	other threads:[~2025-08-04  0:41 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-04  0:40 [PATCH AUTOSEL 5.10 01/39] usb: xhci: print xhci->xhc_state when queue_command failed Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 02/39] cpufreq: CPPC: Mark driver with NEED_UPDATE_LIMITS flag Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 03/39] selftests/futex: Define SYS_futex on 32-bit architectures with 64-bit time_t Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 04/39] usb: typec: ucsi: psy: Set current max to 100mA for BC 1.2 and Default Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 05/39] regulator: core: repeat voltage setting request for stepped regulators Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 06/39] usb: xhci: Avoid showing warnings for dying controller Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 07/39] usb: xhci: Set avg_trb_len = 8 for EP0 during Address Device Command Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 08/39] usb: xhci: Avoid showing errors during surprise removal Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 09/39] gpio: wcd934x: check the return value of regmap_update_bits() Sasha Levin
2025-08-04  0:40 ` Sasha Levin [this message]
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 11/39] ARM: rockchip: fix kernel hang during smp initialization Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 12/39] PM / devfreq: governor: Replace sscanf() with kstrtoul() in set_freq_store() Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 13/39] ASoC: soc-dapm: set bias_level if snd_soc_dapm_set_bias_level() was successed Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 14/39] gpio: tps65912: check the return value of regmap_update_bits() Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 15/39] ARM: tegra: Use I/O memcpy to write to IRAM Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 16/39] selftests: tracing: Use mutex_unlock for testing glob filter Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 17/39] PM: runtime: Clear power.needs_force_resume in pm_runtime_reinit() Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 18/39] thermal: sysfs: Return ENODATA instead of EAGAIN for reads Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 19/39] PM: sleep: console: Fix the black screen issue Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 20/39] ACPI: processor: fix acpi_object initialization Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 21/39] mmc: sdhci-msm: Ensure SD card power isn't ON when card removed Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 22/39] ACPI: APEI: GHES: add TAINT_MACHINE_CHECK on GHES panic path Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 23/39] reset: brcmstb: Enable reset drivers for ARCH_BCM2835 Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 24/39] mmc: rtsx_usb_sdmmc: Fix error-path in sd_set_power_mode() Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 25/39] x86/bugs: Avoid warning when overriding return thunk Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 26/39] ASoC: hdac_hdmi: Rate limit logging on connection and disconnection Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 27/39] ALSA: intel8x0: Fix incorrect codec index usage in mixer for ICH4 Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 28/39] ASoC: core: Check for rtd == NULL in snd_soc_remove_pcm_runtime() Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 29/39] usb: typec: intel_pmc_mux: Defer probe if SCU IPC isn't present Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 30/39] usb: core: usb_submit_urb: downgrade type check Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 31/39] usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 32/39] pm: cpupower: Fix the snapshot-order of tsc,mperf, clock in mperf_stop() Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 33/39] platform/x86: thinkpad_acpi: Handle KCOV __init vs inline mismatches Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 34/39] platform/chrome: cros_ec_typec: Defer probe on missing EC parent Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 35/39] ALSA: hda/ca0132: Fix buffer overflow in add_tuning_control Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 36/39] ALSA: pcm: Rewrite recalculate_boundary() to avoid costly loop Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 37/39] ALSA: usb-audio: Avoid precedence issues in mixer_quirks macros Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 38/39] iio: adc: ad7768-1: Ensure SYNC_IN pulse minimum timing requirement Sasha Levin
2025-08-04  0:40 ` [PATCH AUTOSEL 5.10 39/39] ASoC: codecs: rt5640: Retry DEVICE_ID verification 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=20250804004041.3628812-10-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