Re: [PATCH v3 17/27] rust: auxiliary: generalize Registration over ForLt

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

On Wed May 20, 2026 at 1:33 AM BST, Danilo Krummrich wrote:
> On Tue May 19, 2026 at 6:45 PM CEST, Gary Guo wrote:
>> On Sun May 17, 2026 at 1:01 AM BST, Danilo Krummrich wrote:
>>> +    pub fn new<'bound, E>(
>>> +        parent: &'bound device::Device<device::Bound>,
>>>          name: &CStr,
>>>          id: u32,
>>>          modname: &CStr,
>>> -        data: impl PinInit<T, E>,
>>> +        data: impl PinInit<F::Of<'bound>, E>,
>>>      ) -> Result<Devres<Self>>
>>
>> I think this is unsound for the reason that I gave in another email
>> https://lore.kernel.org/rust-for-linux/DIMSJVKTYX6D.AEN6OPPC2898@garyguo.net/.
>
> Indeed, this has to be fixed. I looked into the options brought up in the linked
> reply, i.e. the "new type" approach and the "closure" approach. And after
> playing around with both of them, I'm not really satisfied with either of those.
>
> The "new type" one is simple and works, but has the disadvantage that, well, we
> need a new type and update all callsites where we would ever want to create
> registrations (mainly probe though).
>
> The "closure" one on the other hand creates a little bit of an odd API and by
> its nature does not allow to move pre-existing resources into the closure, which
> is a major limitation (maybe there is some way, but if so I didn't find it).
>
> I instead went with something else: Currently we return a
> Devres<auxiliary::Registration<_>>. However, since we're moving to lifetimes
> anyway, we can just return an auxiliary::Registration<'bound, _> instead, which
> makes the return type invariant over 'bound, which makes the problem go away
> naturally.
>
> Please find the diff below for reference.
>
> diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
> index 5acece8e369a..c784426b8092 100644
> --- a/rust/kernel/auxiliary.rs
> +++ b/rust/kernel/auxiliary.rs
> @@ -12,7 +12,7 @@
>          RawDeviceId,
>          RawDeviceIdIndex, //
>      },
> -    devres::Devres,
> +
>      driver,
>      error::{
>          from_result,
> @@ -408,12 +408,12 @@ struct RegistrationData<T> {
>  /// `self.adev` always holds a valid pointer to an initialized and registered
>  /// [`struct auxiliary_device`] whose `registration_data_rust` field points to a
>  /// valid `Pin<KBox<RegistrationData<F::Of<'static>>>>`.
> -pub struct Registration<F: ForLt + 'static> {
> +pub struct Registration<'bound, F: ForLt + 'static> {
>      adev: NonNull<bindings::auxiliary_device>,
> -    _data: PhantomData<F>,
> +    _phantom: PhantomData<(fn(&'bound ()) -> &'bound (), F)>,

The type itself may be invariant over the lifetime, but it would still accept a
shorter lifetime (which again, is a case where I think having 'bound becomes a
factor for confusion).

You can still pass in a shortened lifetime to `Registration::new` and then leak
a registration (yes, Leakpocalypse strikes again), so the invariance here isn't
actually serving purpose.

    fn bad_fn(dev: &device::Device<device::Bound>, buf: &[u8]) {
        forget(Box::pin_init(Registration::new(dev, c"foo", 0, c"bar", buf)))
    }

Best,
Gary

>  }
>  
> -impl<F: ForLt> Registration<F>
> +impl<'bound, F: ForLt> Registration<'bound, F>
>  where
>      for<'a> F::Of<'a>: Send + Sync,
>  {
> @@ -421,13 +421,13 @@ impl<F: ForLt> Registration<F>
>      ///
>      /// The `data` is owned by the registration and can be accessed through the auxiliary device
>      /// via [`Device::registration_data()`].
> -    pub fn new<'bound, E>(
> +    pub fn new<E>(
>          parent: &'bound device::Device<device::Bound>,
>          name: &CStr,
>          id: u32,
>          modname: &CStr,
>          data: impl PinInit<F::Of<'bound>, E>,
> -    ) -> Result<Devres<Self>>
> +    ) -> Result<Self>
>      where
>          Error: From<E>,
>      {
> @@ -439,8 +439,10 @@ pub fn new<'bound, E>(
>              GFP_KERNEL,
>          )?;
>  
> -        // SAFETY: Lifetimes are erased and do not affect layout, so RegistrationData<F::Of<'bound>>
> -        // and RegistrationData<F::Of<'static>> have identical representation.
> +        // SAFETY: `'bound` is invariant (via `Registration`'s `PhantomData`), guaranteeing it
> +        // represents the full binding scope. Lifetimes do not affect layout, so
> +        // RegistrationData<F::Of<'bound>> and RegistrationData<F::Of<'static>> have identical
> +        // representation.
>          let data: Pin<KBox<RegistrationData<F::Of<'static>>>> =
>              unsafe { core::mem::transmute(data) };
>  
> @@ -487,18 +489,16 @@ pub fn new<'bound, E>(
>  
>          // INVARIANT: The device will remain registered until `auxiliary_device_delete()` is
>          // called, which happens in `Self::drop()`.
> -        let reg = Self {
> +        Ok(Self {
>              // SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated
>              // successfully.
>              adev: unsafe { NonNull::new_unchecked(adev) },
> -            _data: PhantomData,
> -        };
> -
> -        Devres::new::<core::convert::Infallible>(parent, reg)
> +            _phantom: PhantomData,
> +        })
>      }
>  }
>  
> -impl<F: ForLt> Drop for Registration<F> {
> +impl<F: ForLt> Drop for Registration<'_, F> {
>      fn drop(&mut self) {
>          // SAFETY: By the type invariant of `Self`, `self.adev.as_ptr()` is a valid registered
>          // `struct auxiliary_device`.
> @@ -520,7 +520,7 @@ fn drop(&mut self) {
>  }
>  
>  // SAFETY: A `Registration` of a `struct auxiliary_device` can be released from any thread.
> -unsafe impl<F: ForLt> Send for Registration<F> where for<'a> F::Of<'a>: Send {}
> +unsafe impl<F: ForLt> Send for Registration<'_, F> where for<'a> F::Of<'a>: Send {}
>  
>  // SAFETY: `Registration` does not expose any methods or fields that need synchronization.
> -unsafe impl<F: ForLt> Sync for Registration<F> where for<'a> F::Of<'a>: Send {}
> +unsafe impl<F: ForLt> Sync for Registration<'_, F> where for<'a> F::Of<'a>: Send {}
> diff --git a/samples/rust/rust_driver_auxiliary.rs b/samples/rust/rust_driver_auxiliary.rs
> index 84d18bbfafc5..efb4d97b416b 100644
> --- a/samples/rust/rust_driver_auxiliary.rs
> +++ b/samples/rust/rust_driver_auxiliary.rs
> @@ -10,7 +10,6 @@
>          Bound,
>          Core, //
>      },
> -    devres::Devres,
>      driver,
>      pci,
>      prelude::*,
> @@ -58,29 +57,29 @@ struct Data<'bound> {
>  }
>  
>  #[allow(clippy::type_complexity)]
> -struct ParentDriver {
> -    _reg0: Devres<auxiliary::Registration<ForLt!(Data<'_>)>>,
> -    _reg1: Devres<auxiliary::Registration<ForLt!(Data<'_>)>>,
> +struct ParentDriver<'bound> {
> +    _reg0: auxiliary::Registration<'bound, ForLt!(Data<'_>)>,
> +    _reg1: auxiliary::Registration<'bound, ForLt!(Data<'_>)>,
>  }
>  
>  kernel::pci_device_table!(
>      PCI_TABLE,
>      MODULE_PCI_TABLE,
> -    <ParentDriver as pci::Driver>::IdInfo,
> +    <ParentDriver<'_> as pci::Driver>::IdInfo,
>      [(pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5), ())]
>  );
>  
> -impl pci::Driver for ParentDriver {
> +impl pci::Driver for ParentDriver<'_> {
>      type IdInfo = ();
> -    type Data<'bound> = Self;
> +    type Data<'bound> = ParentDriver<'bound>;
>  
>      const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
>  
>      fn probe<'bound>(
>          pdev: &'bound pci::Device<Core>,
>          _info: &'bound Self::IdInfo,
> -    ) -> impl PinInit<Self, Error> + 'bound {
> -        Ok(Self {
> +    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
> +        Ok(ParentDriver {
>              _reg0: auxiliary::Registration::new(
>                  pdev.as_ref(),
>                  AUXILIARY_NAME,
> @@ -105,7 +104,7 @@ fn probe<'bound>(
>      }
>  }
>  
> -impl ParentDriver {
> +impl ParentDriver<'_> {
>      fn connect(adev: &auxiliary::Device<Bound>) -> Result {
>          let data = adev.registration_data::<ForLt!(Data<'_>)>()?;
>          let pdev = data.parent;
> @@ -131,7 +130,7 @@ fn connect(adev: &auxiliary::Device<Bound>) -> Result {
>  #[pin_data]
>  struct SampleModule {
>      #[pin]
> -    _pci_driver: driver::Registration<pci::Adapter<ParentDriver>>,
> +    _pci_driver: driver::Registration<pci::Adapter<ParentDriver<'static>>>,
>      #[pin]
>      _aux_driver: driver::Registration<auxiliary::Adapter<AuxiliaryDriver>>,
>  }