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

["Danilo Krummrich" <dakr@kernel.org>]

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)>,
 }
 
-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>>,
 }