Unverified Commit 8121353a authored by Alice Ryhl's avatar Alice Ryhl Committed by Mark Brown
Browse files

rust: regulator: do not assume that regulator_get() returns non-null 



The Rust `Regulator` abstraction uses `NonNull` to wrap the underlying
`struct regulator` pointer. When `CONFIG_REGULATOR` is disabled, the C
stub for `regulator_get` returns `NULL`. `from_err_ptr` does not treat
`NULL` as an error, so it was passed to `NonNull::new_unchecked`,
causing undefined behavior.

Fix this by using a raw pointer `*mut bindings::regulator` instead of
`NonNull`. This allows `inner` to be `NULL` when `CONFIG_REGULATOR` is
disabled, and leverages the C stubs which are designed to handle `NULL`
or are no-ops.

Fixes: 9b614cea ("rust: regulator: add a bare minimum regulator abstraction")
Reported-by: default avatarMiguel Ojeda <ojeda@kernel.org>
Closes: https://lore.kernel.org/r/20260322193830.89324-1-ojeda@kernel.org


Signed-off-by: default avatarAlice Ryhl <aliceryhl@google.com>
Reviewed-by: default avatarDaniel Almeida <daniel.almeida@collabora.com>
Link: https://patch.msgid.link/20260324-regulator-fix-v1-1-a5244afa3c15@google.com


Signed-off-by: default avatarMark Brown <broonie@kernel.org>
parent c3692998

rust/kernel/regulator.rs

+18 −15
Original line number Diff line number Diff line
@@ -23,7 +23,10 @@ 
    prelude::*,

};



use core::{marker::PhantomData, mem::ManuallyDrop, ptr::NonNull};

use core::{

    marker::PhantomData,

    mem::ManuallyDrop, //

};



mod private {

    pub trait Sealed {}
@@ -229,15 +232,17 @@ pub fn devm_enable_optional(dev: &Device<Bound>, name: &CStr) -> Result { 
///

/// # Invariants

///

/// - `inner` is a non-null wrapper over a pointer to a `struct

///   regulator` obtained from [`regulator_get()`].

/// - `inner` is a pointer obtained from a successful call to

///   [`regulator_get()`]. It is treated as an opaque token that may only be

///   accessed using C API methods (e.g., it may be `NULL` if the C API returns

///   `NULL`).

///

/// [`regulator_get()`]: https://docs.kernel.org/driver-api/regulator.html#c.regulator_get

pub struct Regulator<State>

where

    State: RegulatorState,

{

    inner: NonNull<bindings::regulator>,

    inner: *mut bindings::regulator,

    _phantom: PhantomData<State>,

}
@@ -249,7 +254,7 @@ pub fn set_voltage(&self, min_voltage: Voltage, max_voltage: Voltage) -> Result 
        // SAFETY: Safe as per the type invariants of `Regulator`.

        to_result(unsafe {

            bindings::regulator_set_voltage(

                self.inner.as_ptr(),

                self.inner,

                min_voltage.as_microvolts(),

                max_voltage.as_microvolts(),

            )
@@ -259,7 +264,7 @@ pub fn set_voltage(&self, min_voltage: Voltage, max_voltage: Voltage) -> Result 
    /// Gets the current voltage of the regulator.

    pub fn get_voltage(&self) -> Result<Voltage> {

        // SAFETY: Safe as per the type invariants of `Regulator`.

        let voltage = unsafe { bindings::regulator_get_voltage(self.inner.as_ptr()) };

        let voltage = unsafe { bindings::regulator_get_voltage(self.inner) };



        to_result(voltage).map(|()| Voltage::from_microvolts(voltage))

    }
@@ -270,10 +275,8 @@ fn get_internal(dev: &Device, name: &CStr) -> Result<Regulator<T>> { 
            // received from the C code.

            from_err_ptr(unsafe { bindings::regulator_get(dev.as_raw(), name.as_char_ptr()) })?;



        // SAFETY: We can safely trust `inner` to be a pointer to a valid

        // regulator if `ERR_PTR` was not returned.

        let inner = unsafe { NonNull::new_unchecked(inner) };



        // INVARIANT: `inner` is a pointer obtained from `regulator_get()`, and

        // the call was successful.

        Ok(Self {

            inner,

            _phantom: PhantomData,
@@ -282,12 +285,12 @@ fn get_internal(dev: &Device, name: &CStr) -> Result<Regulator<T>> { 


    fn enable_internal(&self) -> Result {

        // SAFETY: Safe as per the type invariants of `Regulator`.

        to_result(unsafe { bindings::regulator_enable(self.inner.as_ptr()) })

        to_result(unsafe { bindings::regulator_enable(self.inner) })

    }



    fn disable_internal(&self) -> Result {

        // SAFETY: Safe as per the type invariants of `Regulator`.

        to_result(unsafe { bindings::regulator_disable(self.inner.as_ptr()) })

        to_result(unsafe { bindings::regulator_disable(self.inner) })

    }

}
@@ -349,7 +352,7 @@ impl<T: IsEnabled> Regulator<T> { 
    /// Checks if the regulator is enabled.

    pub fn is_enabled(&self) -> bool {

        // SAFETY: Safe as per the type invariants of `Regulator`.

        unsafe { bindings::regulator_is_enabled(self.inner.as_ptr()) != 0 }

        unsafe { bindings::regulator_is_enabled(self.inner) != 0 }

    }

}
@@ -359,11 +362,11 @@ fn drop(&mut self) { 
            // SAFETY: By the type invariants, we know that `self` owns a

            // reference on the enabled refcount, so it is safe to relinquish it

            // now.

            unsafe { bindings::regulator_disable(self.inner.as_ptr()) };

            unsafe { bindings::regulator_disable(self.inner) };

        }

        // SAFETY: By the type invariants, we know that `self` owns a reference,

        // so it is safe to relinquish it now.

        unsafe { bindings::regulator_put(self.inner.as_ptr()) };

        unsafe { bindings::regulator_put(self.inner) };

    }

}