Re: [PATCH v2] rust: cpufreq: Fix temporary write in Registration::bios_limit_callback

["Gary Guo" <gary@garyguo.net>]

On Sat Jul 18, 2026 at 12:35 AM BST, Priya Bala Govindasamy wrote:
> 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 dereferencing `limit` is
> unsound.
>
> Fix this by changing the signature of `T::bios_limit` to return the limit
> value.
> `Registration::bios_limit_callback` can then update `limit` directly.
>
> 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

You should move this line above your S-o-b. This tag should be covered by your
S-o-b, not by whoever applies this patch.

>
> changes in v2:
>  - Change the signature of `T::bios_limit` and have
>    `Registration::bios_limit_callback` write to `limit` directly instead of initializing `limit` to zero first and passing a mutable reference of `limit` to `T::bios_limit`
>  - v1 Link: https://lore.kernel.org/rust-for-linux/cover.1783456063.git.pgovind2@uci.edu/T/#t
> ---
>  rust/kernel/cpufreq.rs | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
> index 58ac04c650a1..5dc09063f4e9 100644
> --- a/rust/kernel/cpufreq.rs
> +++ b/rust/kernel/cpufreq.rs
> @@ -817,7 +817,9 @@ fn update_limits(_policy: &mut Policy) {
>      }
>  
>      /// Driver's `bios_limit` callback.
> -    fn bios_limit(_policy: &mut Policy, _limit: &mut u32) -> Result {
> +    ///
> +    /// Returns the limit

The exisitng documentation is pointless. This really should just get proper
documentation, e.g.

    /// Returns HW/BIOS max frequency limitations for the CPU.

> +    fn bios_limit(_policy: &mut Policy) -> Result<u32> {
>          build_error!(VTABLE_DEFAULT_ERROR)
>      }
>  
> @@ -1352,9 +1354,10 @@ impl<T: Driver> Registration<T> {
>  
>          from_result(|| {
>              let mut policy = PolicyCpu::from_cpu(cpu_id)?;
> -
> +            let val = T::bios_limit(&mut policy)?;
>              // SAFETY: `limit` is guaranteed by the C code to be valid.
> -            T::bios_limit(&mut policy, &mut (unsafe { *limit })).map(|()| 0)
> +            unsafe { core::ptr::write(limit, val); }

This is primitive so there's no dropping of old value, so this can just be

    unsafe { *limit = val; }

Best,
Gary

> +            Ok(0)
>          })
>      }
>