[PATCH] cpufreq/amd-pstate: Fix TOCTOU when changing driver mode via sysfs

Mario Limonciello posted 1 patch 2 days, 13 hours ago
drivers/cpufreq/amd-pstate.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
[PATCH] cpufreq/amd-pstate: Fix TOCTOU when changing driver mode via sysfs
Posted by Mario Limonciello 2 days, 13 hours ago
amd_pstate_update_status() evaluates mode_state_machine[cppc_state]
[mode_idx] before taking amd_pstate_driver_lock, and then re-evaluates it
again once the lock is held.  cppc_state is global and only stable under
the lock, so concurrent sysfs writes race:

  CPU 0                               CPU 1
  -----                               -----
  // cppc_state == A, mode == B
  if (mode_state_machine[A][B])       // sees non-NULL
                                      if (mode_state_machine[A][B])
                                        guard(&amd_pstate_driver_lock);
                                        mode_state_machine[A][B](B);
                                          cppc_state = B;
    guard(&amd_pstate_driver_lock);
    // cppc_state is now B
    mode_state_machine[B][B](B);      // NULL -> NULL deref

The re-read under the lock can resolve to a self-transition (NULL
pointer, immediate NULL deref) or to an unexpected transition that
redundantly runs amd_pstate_driver_cleanup(), double-freeing
current_pstate_driver->attr.

Take the lock first, then read cppc_state and resolve the transition
function exactly once into a local before calling it, so the check and
the call observe the same state.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-f86ff3a6-55df-4eef-8e4d-62ac270dba25
Fixes: 6f0b13f16f7a ("cpufreq/amd-pstate: Overhaul locking")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 7a2174b5911e4..698b889a08d07 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1901,6 +1901,7 @@ EXPORT_SYMBOL_FOR_PSTATE_UT(amd_pstate_get_status);
 
 int amd_pstate_update_status(const char *buf, size_t size)
 {
+	cppc_mode_transition_fn fn;
 	int mode_idx;
 
 	if (size > strlen("passive") || size < strlen("active"))
@@ -1910,10 +1911,12 @@ int amd_pstate_update_status(const char *buf, size_t size)
 	if (mode_idx < 0)
 		return mode_idx;
 
-	if (mode_state_machine[cppc_state][mode_idx]) {
-		guard(mutex)(&amd_pstate_driver_lock);
-		return mode_state_machine[cppc_state][mode_idx](mode_idx);
-	}
+	guard(mutex)(&amd_pstate_driver_lock);
+
+	/* Resolve the transition under the lock; cppc_state is only stable while held. */
+	fn = mode_state_machine[cppc_state][mode_idx];
+	if (fn)
+		return fn(mode_idx);
 
 	return 0;
 }
-- 
2.55.0