Re: [PATCH v3 5/7] rust: devres: add DevresLt for ForLt-aware device resource access

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] `DevresGuard` implements `Deref` exposing the inner lifetime `'a` tied to `DevresLt` rather than the guard's lifetime (`'guard`), causing an API soundness issue that can lead to Use-After-Free.
- [Low] Missing `#[inline]` annotations on small forwarding abstraction functions.
--

commit 20febe7248eef3fe1ba711b99c2b480ed7284b88
Author: Danilo Krummrich <dakr@kernel.org>

rust: devres: add DevresLt for ForLt-aware device resource access

This commit introduces DevresLt, a wrapper around Devres that provides
lifetime-aware access to device resources. It adds accessors that correctly
shorten the stored static lifetime to the caller's borrow lifetime, preventing
unsound static lifetimes from being exposed.

> diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> index 11ce500e9b76f..e11deff3e1be5 100644
> --- a/rust/kernel/devres.rs
> +++ b/rust/kernel/devres.rs
[ ... ]
> @@ -365,6 +367,104 @@ fn drop(&mut self) {
>      }
>  }
>  
> +/// Guard returned by [`DevresLt::try_access`].
> +///
> +/// Dereferences to `F::Of<'a>`, shortening the lifetime of the stored data to the guard's borrow
> +/// lifetime.
> +pub struct DevresGuard<'a, F: CovariantForLt>(RevocableGuard<'a, F::Of<'static>>);
> +
> +impl<'a, F: CovariantForLt> core::ops::Deref for DevresGuard<'a, F> {
> +    type Target = F::Of<'a>;
> +
> +    fn deref(&self) -> &Self::Target {

[Severity: Low]
This isn't a bug, but shouldn't this trivial forwarding function be annotated
with #[inline]?

[Severity: High]
Could implementing Deref here expose the outer lifetime 'a instead of the
guard's lifetime, potentially leading to a use-after-free?

Because F::Of<'a> is exposed with its outer 'a lifetime, a caller can
trivially extract references with lifetime 'a from it.

When DevresGuard is dropped, the internal RCU guard is released, ending the
RCU read-side critical section. However, the extracted 'a reference is still
valid from the compiler's perspective because it is tied to DevresLt.

If the device is concurrently unbound, devres will revoke and drop the
internal data. Accessing the extracted 'a reference afterwards results in a
use-after-free.

Would it be safer to remove Deref and provide an explicit get method that
yields &'guard F::Of<'guard>?

> +        F::cast_ref(&*self.0)
> +    }
> +}
[ ... ]
> +    /// Return a reference of the [`Device`] this [`DevresLt`] instance has been created with.
> +    pub fn device(&self) -> &Device {

[Severity: Low]
This isn't a bug, but should this small forwarding function have an
#[inline] annotation as per the Rust subsystem guidelines?

This also applies to other trivial forwarding methods added in this commit,
such as access_with, try_access_with, and access.

> +        self.0.device()
> +    }
[ ... ]
> +    /// [`DevresLt`] accessor for [`Revocable::try_access`].
> +    pub fn try_access(&self) -> Option<DevresGuard<'_, F>> {

[Severity: High]
Is there a risk that tying the guard to the borrow lifetime of &self allows
references to outlive the RCU read-side critical section?

This method borrows &self for lifetime 'a and returns Option<DevresGuard<'a,
F>>. Combined with the Deref implementation, this exposes the longer lifetime
'a to the caller, bypassing the RCU lock's guard lifetime.

[Severity: Low]
This isn't a bug, but would it be appropriate to add an #[inline] annotation
to this forwarding function?

> +        self.0.data().try_access().map(DevresGuard)
> +    }
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618230834.812007-1-dakr@kernel.org?part=5