Re: [PATCH v4 21/27] rust: types: add `ForLt` trait for higher-ranked lifetime support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] The `ForLt!` macro's covariance check can be bypassed when a lifetime is hidden inside a nested macro invocation, leading to unsound implementations of the `ForLt` trait.
- [Critical] The safety architecture of `ForLt` is flawed because it allows safe code to bypass the macro's covariance proofs entirely, directly leading to soundness vulnerabilities.
- [Low] Incorrect usage of `#[inline(always)]` on `cast_ref`.
- [Low] `expand_elided_lifetime` fails to replace elided lifetimes (`'_`) hidden inside macro invocations.
--

commit a372a3dda5030ca04860c4057bb0215647faa34b
Author: Gary Guo <gary@garyguo.net>

rust: types: add ForLt trait for higher-ranked lifetime support

This commit introduces the ForLt trait and a corresponding macro to
support types that are generic over lifetimes. The macro attempts to
syntactically prove covariance or emits compiler checks to ensure the
soundness of lifetime shortening casts.

Link: https://docs.rs/higher-kinded-types/ [1]

> diff --git a/rust/kernel/types/for_lt.rs b/rust/kernel/types/for_lt.rs
> new file mode 100644
> --- /dev/null
> +++ b/rust/kernel/types/for_lt.rs

[ ... ]

> +pub unsafe trait ForLt {
> +    /// The type parameterized by the lifetime.
> +    type Of<'a>: 'a;
> +
> +    /// Cast a reference to a shorter lifetime.
> +    #[inline(always)]
> +    fn cast_ref<'r, 'short: 'r, 'long: 'short>(long: &'r Self::Of<'long>) -> &'r Self::Of<'short> {

[Severity: Low]
This isn't a bug, but according to the rust subsystem coding guidelines,
#[inline(always)] should only be used for functions using build_assert!()
that depend on function parameters. Should this be changed to #[inline]?

> +        // SAFETY: This is sound as this trait guarantees covariance.
> +        unsafe { core::mem::transmute(long) }
> +    }
> +}

[ ... ]

> +#[doc(hidden)]
> +pub struct UnsafeForLtImpl<T: ?Sized, WF, const N: usize>(PhantomData<(WF, T)>);
> +
> +// This is a helper trait for implementation `ForLt` to be able to use HRTB.
> +#[doc(hidden)]
> +pub trait WithLt<'a> {
> +    type Of: 'a;
> +}
> +
> +// SAFETY: In `ForLt!` macro, a covariance proof is generated when naming `UnsafeForLtImpl`
> +// and it will fail to evaluate if the type is not covariant.
> +unsafe impl<T: ?Sized + for<'a> WithLt<'a>, WF> ForLt for UnsafeForLtImpl<T, WF, 0> {
> +    type Of<'a> = <T as WithLt<'a>>::Of;
> +}

[Severity: Critical]
Can safe downstream code implement the public WithLt trait for an
invariant type (like Cell<&'a u32>), and then name
<UnsafeForLtImpl<MyStruct, (), 0> as ForLt> to access ForLt::cast_ref?

If so, does this allow shortening lifetimes of invariant types entirely
in safe code, bypassing the macro's covariance checks and breaking
safety guarantees?

> diff --git a/rust/macros/for_lt.rs b/rust/macros/for_lt.rs
> new file mode 100644
> --- /dev/null
> +++ b/rust/macros/for_lt.rs

[ ... ]

> +    fn expand_elided_lifetime(&self, explicit_lt: &Lifetime) -> Type {
> +        struct ElidedLifetimeExpander<'a>(&'a Lifetime);
> +
> +        impl VisitMut for ElidedLifetimeExpander<'_> {
> +            fn visit_lifetime_mut(&mut self, lifetime: &mut Lifetime) {
> +                // Expand explicit `'_`
> +                if lifetime.ident == "_" {
> +                    *lifetime = self.0.clone();
> +                }
> +            }
> +
> +            fn visit_type_reference_mut(&mut self, reference: &mut syn::TypeReference) {
> +                syn::visit_mut::visit_type_reference_mut(self, reference);
> +
> +                if reference.lifetime.is_none() {
> +                    reference.lifetime = Some(self.0.clone());
> +                }
> +            }
> +        }
> +
> +        let mut ret = self.clone();
> +        ElidedLifetimeExpander(explicit_lt).visit_type_mut(&mut ret);
> +        ret
> +    }

[Severity: Low]
If an elided lifetime is passed inside a macro invocation, such as
ForLt!(Wrapper<SomeMacro!(<'_>)>), does VisitMut fail to traverse the
TokenStream inside the Type::Macro?

Will this leave the elided lifetime unmodified and cause a missing
lifetime specifier compiler error?

[ ... ]

> +    fn prove(&mut self, ty: &'a Type) {
> +        match ty {

[ ... ]

> +            // Macro invocations are opaque to proc macros; conservatively require
> +            // a compiler proof since we cannot determine lifetime usage.
> +            Type::Macro(_) => self.1.push(ty),
> +
> +            // No mention of lifetime at all, no need to perform compiler check.
> +            ty if !ty.has_lifetime(self.0) => (),
> +
> +            // Otherwise, we need to emit checks so that compiler can determine if the types are
> +            // actually covariant.
> +            ty => self.1.push(ty),
> +        }
> +    }

[Severity: Critical]
If a lifetime is hidden within a macro invocation that is nested inside
another type, for example ForLt!(for<'a> Cell<Mac!('a)>), will
has_lifetime falsely return false because it doesn't traverse into
the macro's TokenStream?

Since the Type::Macro arm only matches top-level macros, does the nested
Type::Path (Cell) fall through to the ty if !ty.has_lifetime(self.0) arm?

If so, does this bypass the compiler-assisted covariance proof and
allow unsound lifetime shortening casts?

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