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/[email protected]/.
>
> 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>>,
> }
