[PATCH 0/1] rust: cpufreq: Fix temporary write in Registration::bios_limit_callback

[PATCH 0/1] rust: cpufreq: Fix temporary write in Registration::bios_limit_callback

[Priya Bala Govindasamy]

Dear Linux Kernel Maintainers,

We are developing a tool called FerroLens to detect potential unsound 
behavior in Rust code in the Linux kernel.
The tool internally uses an LLM to detect bugs. 
We then perform manual analysis to verify these reports. 
FerroLens reported the following issues in rust/kernel/cpufreq.rs.

In `Registration::bios_limit_callback`, the expression 
`&mut (unsafe { *limit })` creates a reference to a temporary copy 
of the value pointed to by `limit` on the stack.
Therefore, writes made by `T::bios_limit` go to this temporary 
instead of the memory location pointed to by `limit`.

Additionally, `limit` may be uninitialized, such as when
`Registration::bios_limit_callback` is invoked by `show_bios_limit`
in drivers/cpufreq/cpufreq.c. Therefore creating a reference from 
`limit` is unsound.

Here is a PoC that demonstrates that writes made by `T::bios_limit` 
are not persistent:

  // SPDX-License-Identifier: GPL-2.0

  //! Rust cpufreq proof-of-concept driver for the `bios_limit` callback bug.

  use core::sync::atomic::{AtomicU32, Ordering};

  use kernel::{
      bindings,
      clk::Hertz,
      cpufreq,
      error::code::*,
      macros::vtable,
      prelude::*,
      sync::Arc,
  };

  const LOW_FREQ: Hertz = Hertz::from_mhz(1000);
  const HIGH_FREQ: Hertz = Hertz::from_mhz(2000);
  const BIOS_LIMIT_KHZ: u32 = 424_242;

  module! {
      type: CpuFreqPocModule,
      name: "cpufreq_poc",
      authors: ["Priya Govindasamy"],
      description: "PoC cpufreq driver for the Rust bios_limit callback bug",
      license: "GPL",
  }

  struct CpuFreqPocModule {
      _driver: cpufreq::Registration<CpuFreqPocDriver>,
  }

  struct CpuFreqPocPolicyData {
      table: KVec<bindings::cpufreq_frequency_table>,
      current_freq_khz: AtomicU32,
  }

  #[derive(Default)]
  struct CpuFreqPocDriver;

  #[vtable]
  impl cpufreq::Driver for CpuFreqPocDriver {
      const NAME: &'static CStr = c"cpufreq-poc";
      const FLAGS: u16 = 0;
      const BOOST_ENABLED: bool = false;

      type PData = Arc<CpuFreqPocPolicyData>;

      fn init(policy: &mut cpufreq::Policy) -> Result<Self::PData> {
          
          let mut table = KVec::new();
          table.push(
              bindings::cpufreq_frequency_table {
                  flags: 0,
                  driver_data: 0,
                  frequency: LOW_FREQ.as_khz() as u32,
              },
              GFP_KERNEL,
          )?;
          table.push(
              bindings::cpufreq_frequency_table {
                  flags: 0,
                  driver_data: 1,
                  frequency: HIGH_FREQ.as_khz() as u32,
              },
              GFP_KERNEL,
          )?;
          table.push(
              bindings::cpufreq_frequency_table {
                  flags: 0,
                  driver_data: 0,
                  frequency: bindings::CPUFREQ_TABLE_END as u32,
              },
              GFP_KERNEL,
          )?;
          
          let cpu = policy.cpu();

          policy.cpus().set(cpu);
          policy
              .set_min(LOW_FREQ)
              .set_max(HIGH_FREQ)
              .set_cpuinfo_min_freq(LOW_FREQ)
              .set_cpuinfo_max_freq(HIGH_FREQ)
              .set_suspend_freq(LOW_FREQ)
              .set_transition_latency_ns(cpufreq::DEFAULT_TRANSITION_LATENCY_NS);

          unsafe { policy.set_freq_table(cpufreq::Table::from_raw(table.as_ptr())) };

          pr_info!(
              "cpufreq_poc: init cpu{} expected bios_limit={} kHz\n",
              cpu.as_u32(),
              BIOS_LIMIT_KHZ
          );

          Ok(Arc::new(
              CpuFreqPocPolicyData {
                  table,
                  current_freq_khz: AtomicU32::new(HIGH_FREQ.as_khz() as u32),
              },
              GFP_KERNEL,
          )?)
      }

      fn verify(data: &mut cpufreq::PolicyData) -> Result {
          data.generic_verify()
      }

      fn target_index(policy: &mut cpufreq::Policy, index: cpufreq::TableIndex) -> Result {
          let Some(data) = policy.data::<Self::PData>() else {
              return Err(ENOENT);
          };

          let table = unsafe { cpufreq::Table::from_raw(data.table.as_ptr()) };
          let freq = table.freq(index)?;
          let freq_khz = freq.as_khz() as u32;

          data.current_freq_khz.store(freq_khz, Ordering::Relaxed);
          Ok(())
      }

      fn get(policy: &mut cpufreq::Policy) -> Result<u32> {
          let Some(data) = policy.data::<Self::PData>() else {
              return Err(ENOENT);
          };

          let freq = data.current_freq_khz.load(Ordering::Relaxed);

          Ok(freq)
      }

      fn bios_limit(policy: &mut cpufreq::Policy, limit: &mut u32) -> Result {
          *limit = BIOS_LIMIT_KHZ;

          pr_info!(
              "cpufreq_poc: bios_limit cpu{} wrote {} kHz\n",
              policy.cpu().as_u32(),
              BIOS_LIMIT_KHZ
          );

          Ok(())
      }

      fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result {
          Ok(())
      }
  }

  impl kernel::Module for CpuFreqPocModule {
      fn init(_module: &'static ThisModule) -> Result<Self> {
          pr_info!(
              "cpufreq_poc: loading, expected sysfs bios_limit value is {} kHz\n",
              BIOS_LIMIT_KHZ
          );

          let driver = match cpufreq::Registration::<CpuFreqPocDriver>::new() {
              Ok(driver) => driver,
              Err(err) => {
                  pr_err!(
                      "cpufreq_poc: cpufreq driver registration failed: errno={}\n",
                      err.to_errno()
                  );
                  return Err(err);
              }
          };

          Ok(Self { _driver: driver })
      }
  }

  impl Drop for CpuFreqPocModule {
      fn drop(&mut self) {
          pr_info!("cpufreq_poc: unloaded\n");
      }
  }

Output:

cat /sys/devices/system/cpu/cpu0/cpufreq/bios_limit
0

[  327.756814] cpufreq_poc: loading out-of-tree module taints kernel.
[  327.756852] cpufreq_poc: module verification failed: signature and/or required key missing - tainting kernel
[  327.761432] cpufreq_poc: cpufreq_poc: loading, expected sysfs bios_limit value is 424242 kHz
[  327.763857] cpufreq_poc: cpufreq_poc: init cpu0 expected bios_limit=424242 kHz
[  327.770173] cpufreq_poc: cpufreq_poc: init cpu1 expected bios_limit=424242 kHz
[  327.777130] cpufreq_poc: cpufreq_poc: init cpu2 expected bios_limit=424242 kHz
[  327.778584] tsc: Marking TSC unstable due to cpufreq changes on SMP
[  327.781279] cpufreq_poc: cpufreq_poc: init cpu3 expected bios_limit=424242 kHz
[  327.782682] cpufreq_poc: cpufreq_poc: init cpu4 expected bios_limit=424242 kHz
[  327.785466] cpufreq_poc: cpufreq_poc: init cpu5 expected bios_limit=424242 kHz
[  327.787647] cpufreq_poc: cpufreq_poc: init cpu6 expected bios_limit=424242 kHz
[  327.789458] cpufreq_poc: cpufreq_poc: init cpu7 expected bios_limit=424242 kHz
[  327.792583] cpufreq_poc: cpufreq_poc: init cpu8 expected bios_limit=424242 kHz
[  327.794484] cpufreq_poc: cpufreq_poc: init cpu9 expected bios_limit=424242 kHz
[  327.797198] cpufreq_poc: cpufreq_poc: init cpu10 expected bios_limit=424242 kHz
[  327.799800] cpufreq_poc: cpufreq_poc: init cpu11 expected bios_limit=424242 kHz
[  327.801306] cpufreq_poc: cpufreq_poc: init cpu12 expected bios_limit=424242 kHz
[  327.804141] cpufreq_poc: cpufreq_poc: init cpu13 expected bios_limit=424242 kHz
[  327.806432] cpufreq_poc: cpufreq_poc: init cpu14 expected bios_limit=424242 kHz
[  327.809162] cpufreq_poc: cpufreq_poc: init cpu15 expected bios_limit=424242 kHz
[  335.494129] cpufreq_poc: cpufreq_poc: bios_limit cpu0 wrote 424242 kHz

Priya Bala Govindasamy (1):
  Fix temporary write in Registration::bios_limit_callback

 rust/kernel/cpufreq.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH 1/1] rust: cpufreq: Fix temporary write in Registration::bios_limit_callback

[Priya Bala Govindasamy]

Writes made by `T::bios_limit` go to a temporary on the stack instead of the 
limit pointer passed by C. 
Fix by creating a mutable reference to the actual memory location of
the limit pointer.

Additionally there is a creation of a reference from the 
uninitialized limit pointer which is unsound. 
Fix by initializing the limit pointer to 0 before 
calling `T::bios_limit`.

---
Fixes: c6af9a1191d042839e56abff69e8b0302d117988 ("rust: cpufreq: Extend abstractions for driver registration")
Reported-by: Dylan Zueck<dzueck@uci.edu>
Reported-by: Yuan Tan<ytan089@ucr.edu>
Signed-off-by: Priya Bala Govindasamy<pgovind2@uci.edu>
Assisted-by: ChatGPT:gpt-5.4

 rust/kernel/cpufreq.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index 58ac04c650a1..0f9385d41b64 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -1349,12 +1349,12 @@ impl<T: Driver> Registration<T> {
     unsafe extern "C" fn bios_limit_callback(cpu: c_int, limit: *mut c_uint) -> c_int {
         // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.
         let cpu_id = unsafe { CpuId::from_i32_unchecked(cpu) };
-
+        unsafe { core::ptr::write(limit, 0); }
         from_result(|| {
             let mut policy = PolicyCpu::from_cpu(cpu_id)?;
 
             // SAFETY: `limit` is guaranteed by the C code to be valid.
-            T::bios_limit(&mut policy, &mut (unsafe { *limit })).map(|()| 0)
+            T::bios_limit(&mut policy, unsafe { &mut *limit }).map(|()| 0)
         })
     }
 
-- 
2.34.1