From: Kaushlendra Kumar kaushlendra.kumar@intel.com
[ Upstream commit 23199d2aa6dcaf6dd2da772f93d2c94317d71459 ]
Fix incorrect size parameter passed to cpuidle_state_write_file() in cpuidle_state_disable().
The function was incorrectly using sizeof(disable) which returns the size of the unsigned int variable (4 bytes) instead of the actual length of the string stored in the 'value' buffer.
Since 'value' is populated with snprintf() to contain the string representation of the disable value, we should use the length returned by snprintf() to get the correct string length for writing to the sysfs file.
This ensures the correct number of bytes is written to the cpuidle state disable file in sysfs.
Link: https://lore.kernel.org/r/20250917050820.1785377-1-kaushlendra.kumar@intel.c... Signed-off-by: Kaushlendra Kumar kaushlendra.kumar@intel.com Signed-off-by: Shuah Khan skhan@linuxfoundation.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- What it fixes - The patch corrects the length passed to the sysfs write helper from a fixed, incorrect size to the actual string length: - Before: `cpuidle_state_disable()` used `sizeof(disable)` (always 4) when writing the textual value to sysfs, causing 4 bytes to be written regardless of the actual string length. - See old call site changed in `tools/power/cpupower/lib/cpuidle.c:247` (post‑patch numbering): `value, sizeof(disable)` → `value, len` - After: It computes `len = snprintf(value, SYSFS_PATH_MAX, "%u", disable);` and passes that `len` to the writer, ensuring only the intended number of bytes is written: - New local `int len;` added at `tools/power/cpupower/lib/cpuidle.c:236`. - `len = snprintf(...)` at `tools/power/cpupower/lib/cpuidle.c:245`. - Correct write length used at `tools/power/cpupower/lib/cpuidle.c:247-248`.
- Why it matters - The writer `cpuidle_state_write_file()` is a thin wrapper around `write(2)` that takes a `len` and writes it verbatim to the sysfs attribute `disable`: - See `tools/power/cpupower/lib/cpuidle.c:85-111`. - The backing kernel sysfs store for `stateX/disable` parses a NUL/newline-terminated string using `kstrtouint()` (see `drivers/cpuidle/sysfs.c:281-306`). While the kernfs write path guarantees NUL termination for the internal buffer, passing a length larger than the actual string (e.g., 4 for “0”) causes extra bytes beyond the first NUL to be sent. This is conceptually incorrect and can lead to surprising behavior (e.g., stray bytes copied into the sysfs buffer), even if parsing usually succeeds due to the early NUL. - In practice, cpupower only writes “0” or “1”, so `sizeof(unsigned int)` = 4 led to writing “1\0??” instead of just “1”. That typically works today because: - Kernfs appends a NUL at position `count` and `kstrtouint()` stops at the first NUL, so the extra bytes after the embedded NUL are ignored in most cases. - However, this is still a correctness bug and fragile across attribute implementations and future changes.
- Scope and risk - Scope is minimal and fully contained to the cpupower userspace library: - Only `tools/power/cpupower/lib/cpuidle.c` is changed. - No ABI changes; only a local variable and corrected length usage. - No architectural changes; no kernel/runtime impact beyond how many bytes userspace writes. - Regression risk is negligible: - Success semantics in `cpuidle_state_disable()` remain the same (returns 0 if any positive byte count is written; see `tools/power/cpupower/lib/cpuidle.c:247-251`). - Downstream callers (cpupower `cpuidle-set`, Python bindings, and `rtla`) do not depend on writing exactly 4 bytes and will continue to work; see callers such as `tools/power/cpupower/utils/cpuidle- set.c:117,130` and `tools/tracing/rtla/src/utils.c:596,661`.
- Affected history - The incorrect `sizeof(disable)` usage dates back to the introduction of the cpuidle library to cpupower in 2016 (commit `ac5a181d065d7`), so all stable series carrying cpupower’s cpuidle library are affected. - The fix is already in mainline as `23199d2aa6dca` (“tools/cpupower: Fix incorrect size in cpuidle_state_disable()”), authored 2025-09-17.
- Stable backport criteria - Important bugfix: Yes — corrects wrong write length to sysfs, eliminating stray bytes and making behavior robust and explicit. - Small and contained: Yes — one function, a few lines in a single file. - No new features or architectural changes: Correct. - Minimal regression risk: Correct. - Critical subsystem: It’s in `tools/` (userspace cpupower), which stable regularly backports fixes for.
Given the above, this is a low-risk correctness fix confined to cpupower, addressing a long-standing bug. It should be backported to stable trees.
tools/power/cpupower/lib/cpuidle.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/power/cpupower/lib/cpuidle.c b/tools/power/cpupower/lib/cpuidle.c index 0ecac009273ce..f2c1139adf716 100644 --- a/tools/power/cpupower/lib/cpuidle.c +++ b/tools/power/cpupower/lib/cpuidle.c @@ -233,6 +233,7 @@ int cpuidle_state_disable(unsigned int cpu, { char value[SYSFS_PATH_MAX]; int bytes_written; + int len;
if (cpuidle_state_count(cpu) <= idlestate) return -1; @@ -241,10 +242,10 @@ int cpuidle_state_disable(unsigned int cpu, idlestate_value_files[IDLESTATE_DISABLE])) return -2;
- snprintf(value, SYSFS_PATH_MAX, "%u", disable); + len = snprintf(value, SYSFS_PATH_MAX, "%u", disable);
bytes_written = cpuidle_state_write_file(cpu, idlestate, "disable", - value, sizeof(disable)); + value, len); if (bytes_written) return 0; return -3;