rust: device: make lifetime on `Core` and `CoreInternal` invariant
Subsystems (6)
PATCH 1/1 rust: device: make lifetime on `Core` and `CoreInternal` invariant
From: Gary Guo <[email protected]>
Currently the lifetime on Core and CoreInternal is covariant. This means that they can be coerced into shorter living lifetimes. On probe function, signature has &'bound Device<Core<'a>>; the type's wellformness would imply 'a: 'bound and thus the type can be coerced &'bound Device<Core<'bound>>, defeating the purpose of having the lifetime bound to prevent users of the Core type to escape the function.
Fix this by making the lifetime invariant, so the coercion is impossible. The lifetime here only needs to be "branded" so it does not coerce or unify with other lifetimes, so we do not need to ensure 'bound: 'a.
This requires modifying nova-core which relies on this implied bound due to pre-2024 capture rule. The "use" bound can be removed if built with edition 2024.
--- drivers/gpu/nova-core/gpu.rs | 6 +++--- rust/kernel/device.rs | 10 ++++++++-- 2 files changed, 11 insertions(+), 5 deletions(-)
drivers/gpu/nova-core/gpu.rs +3 -3
@@ -285,10 +285,10 @@ pub(crate) struct Gpu<'gpu> { } impl<'gpu> Gpu<'gpu> { - pub(crate) fn new( - pdev: &'gpu pci::Device<device::Core<'_>>, + pub(crate) fn new<'a>( + pdev: &'gpu pci::Device<device::Core<'a>>, bar: Bar0<'gpu>, - ) -> impl PinInit<Self, Error> + 'gpu { + ) -> impl PinInit<Self, Error> + use<'gpu, 'a> { try_pin_init!(Self { device: pdev.as_ref(), spec: Spec::new(pdev.as_ref(), bar).inspect(|spec| {
rust/kernel/device.rs +8 -2
@@ -511,7 +511,11 @@ pub trait DeviceContext: private::Sealed {}/// callback it appears in. It is intended to be used for synchronization purposes. Bus device /// implementations can implement methods for [`Device<Core>`], such that they can only be called /// from bus callbacks. -pub struct Core <'a>(PhantomData <&'a ()>); +/// +/// The lifetime `'a` is for "lifetime branding" purpose. Callbacks need to polymorphic over this +/// lifetime so the `&'bound Device<Core<'_>>` provided to them cannot outlive the scope of the +/// function. For this reason, it needs to be invariant. +pub struct Core <'a>(PhantomData <fn (&'a ()) -> &'a ()>);/// Semantically the same as [`Core`], but reserved for internal usage of the corresponding bus /// abstraction.
@@ -522,7 +526,9 @@ pub trait DeviceContext: private::Sealed {}/// /// This context mainly exists to share generic [`Device`] infrastructure that should only be called /// from bus callbacks with bus abstractions, but without making them accessible for drivers. -pub struct CoreInternal <'a>(PhantomData <&'a ()>); +/// +/// Lifetime `'a` is invariant for the same reason as [`Core`]. +pub struct CoreInternal <'a>(PhantomData <fn (&'a ()) -> &'a ()>);/// The [`Bound`] context is the [`DeviceContext`] of a bus specific device when it is guaranteed to /// be bound to a driver.
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
Signature · 1 line
2.54.0
Gary Guo
reviewerQuoted 9 lines · Gary Guo
From: Gary Guo <[email protected]>
Currently the lifetime on
CoreandCoreInternalis covariant. This means that they can be coerced into shorter living lifetimes. Onprobefunction, signature has&'bound Device<Core<'a>>; the type's wellformness would imply'a: 'boundand thus the type can be coerced&'bound Device<Core<'bound>>, defeating the purpose of having the lifetime bound to prevent users of theCoretype to escape the function.
Quoted 7 lines
Fix this by making the lifetime invariant, so the coercion is impossible. The lifetime here only needs to be "branded" so it does not coerce or unify with other lifetimes, so we do not need to ensure
'bound: 'a.This requires modifying
nova-corewhich relies on this implied bound due to pre-2024 capture rule. The "use" bound can be removed if built with edition 2024.
Actually, a more proper fix is to make Device invariant over their context. It would require touching all Device, though.
Danilo, let me know if you'd prefer that approach instead.
Best,
Gary
Quoted 56 lines
Fixes 24799831d631 ("rust: device: make Core and CoreInternal lifetime-parameterized")Signed-off-by Gary Guo <[email protected]>--- drivers/gpu/nova-core/gpu.rs | 6 +++--- rust/kernel/device.rs | 10 ++++++++-- 2 files changed, 11 insertions(+), 5 deletions(-)drivers/gpu/nova-core/gpu.rs +3
-3@@ -285,10 +285,10 @@ pub(crate) struct Gpu<'gpu> { } impl<'gpu> Gpu<'gpu> { - pub(crate) fn new( - pdev: &'gpu pci::Device<device::Core<'_>>, + pub(crate) fn new<'a>( + pdev: &'gpu pci::Device<device::Core<'a>>, bar: Bar0<'gpu>, - ) -> impl PinInit<Self, Error> + 'gpu { + ) -> impl PinInit<Self, Error> + use<'gpu, 'a> { try_pin_init!(Self { device: pdev.as_ref(), spec: Spec::new(pdev.as_ref(), bar).inspect(|spec| {rust/kernel/device.rs +8
-2@@ -511,7 +511,11 @@ pub trait DeviceContext: private::Sealed {}/// callback it appears in. It is intended to be used for synchronization purposes. Bus device /// implementations can implement methods for [`Device<Core>`], such that they can only be called /// from bus callbacks. -pub struct Core <'a>(PhantomData <&'a ()>); +/// +/// The lifetime `'a` is for "lifetime branding" purpose. Callbacks need to polymorphic over this +/// lifetime so the `&'bound Device<Core<'_>>` provided to them cannot outlive the scope of the +/// function. For this reason, it needs to be invariant. +pub struct Core <'a>(PhantomData <fn (&'a ()) -> &'a ()>);/// Semantically the same as [`Core`], but reserved for internal usage of the corresponding bus /// abstraction. @@ -522,7 +526,9 @@ pub trait DeviceContext: private::Sealed {}/// /// This context mainly exists to share generic [`Device`] infrastructure that should only be called /// from bus callbacks with bus abstractions, but without making them accessible for drivers. -pub struct CoreInternal <'a>(PhantomData <&'a ()>); +/// +/// Lifetime `'a` is invariant for the same reason as [`Core`]. +pub struct CoreInternal <'a>(PhantomData <fn (&'a ()) -> &'a ()>);/// The [`Bound`] context is the [`DeviceContext`] of a bus specific device when it is guaranteed to /// be bound to a driver. base-commit:
a13c140cc289c0b7b3770bce5b3ad42ab35074aa
Danilo Krummrich
maintainerQuoted 4 lines · Gary Guo
Actually, a more proper fix is to make
Deviceinvariant over their context. It would require touching allDevice, though.Danilo, let me know if you'd prefer that approach instead.
The bigger change is not a concern, but requiring every bus device implementor to consider this is a bit of a concern. If we can enforce it in the driver core code I'd prefer that.
Danilo Krummrich
maintainerQuoted 5 lines · Gary Guo
drivers/gpu/nova-core/gpu.rs +0
-0
drivers/gpu/nova-core/gpu.rs pub(crate) struct Gpu<'gpu> { · line 291
Quoted 10 lines
drivers/gpu/nova-core/gpu.rs +3
-3@@ -285,10 +285,10 @@ pub(crate) struct Gpu<'gpu> { }impl <'gpu>Gpu <'gpu> { -pub (crate )fn new ( - pdev: &'gpu pci::Device <device::Core <'_>>, +pub (crate )fn new <'a>( + pdev: &'gpu pci::Device <device::Core <'a>>, bar:Bar0 <'gpu>, - ) ->impl PinInit <Self ,Error > + 'gpu { + ) ->impl PinInit <Self ,Error > +use <'gpu, 'a> {
Isn't that edition 2024 only?
drivers/gpu/nova-core/gpu.rs pub(crate) struct Gpu<'gpu> { · line 294
Quoted 3 lines
drivers/gpu/nova-core/gpu.rs +0
-0try_pin_init! (Self { device: pdev.as_ref (), spec:Spec ::new (pdev.as_ref (), bar).inspect (|spec| {
I think you also need this hunk:
samples/rust/rust_debugfs.rs +1 -1
@@ -147,7 +147,7 @@ fn build_inner(dir: &Dir) -> impl PinInit<File<Mutex<Inner>>> + '_ { dir.read_write_file (c"pair" ,new_mutex! (Inner { x:3 , y:10 })) } -fn new <'a>(pdev: &'a platform::Device <Core <'_>>) ->impl PinInit <Self ,Error > + 'a { +fn new <'a, 'b>(pdev: &'a platform::Device <Core <'b>>) ->impl PinInit <Self ,Error > +use <'a, 'b> {let debugfs =Dir ::new (c"sample_debugfs" );let dev = pdev.as_ref ();
Gary Guo
reviewerQuoted 8 lines · Danilo Krummrich
Quoted 7 lines · Gary Guo
drivers/gpu/nova-core/gpu.rs +0
-0@@ -285,10 +285,10 @@ pub(crate) struct Gpu<'gpu> { }
Quoted 10 lines
drivers/gpu/nova-core/gpu.rs pub(crate) struct Gpu<'gpu> { · line 291
Quoted 8 lines
drivers/gpu/nova-core/gpu.rs +3
-3impl <'gpu>Gpu <'gpu> { -pub (crate )fn new ( - pdev: &'gpu pci::Device <device::Core <'_>>, +pub (crate )fn new <'a>( + pdev: &'gpu pci::Device <device::Core <'a>>, bar:Bar0 <'gpu>, - ) ->impl PinInit <Self ,Error > + 'gpu { + ) ->impl PinInit <Self ,Error > +use <'gpu, 'a> {Isn't that edition 2024 only?
The syntax is available to all editions. Pre-2024 only type parameters are captured, so the use would be needed to capture 'a.
In 2024 the capture rule is changed so that all lifetimes are captured by default, removing the inconsistency between lifetime and type parameters, so this can just be
pub(crate) fn new(
pdev: &'gpu pci::Device<device::Core<'_>>,
bar: Bar0<'gpu>,
) -> impl PinInit<Self, Error> {The equivalent code without using use is
pub(crate) fn new<'init, 'a>(
pdev: &'gpu pci::Device<device::Core<'a>>,
bar: Bar0<'gpu>,
) -> impl PinInit<Self, Error> + 'init where 'init: 'gpu, 'init: 'a {but use is stable in 1.82+ so we can just use it instead.
Quoted 12 lines
drivers/gpu/nova-core/gpu.rs pub(crate) struct Gpu<'gpu> { · line 294
Quoted 4 lines
drivers/gpu/nova-core/gpu.rs +0
-0try_pin_init! (Self { device: pdev.as_ref (), spec:Spec ::new (pdev.as_ref (), bar).inspect (|spec| {I think you also need this hunk:
samples/rust/rust_debugfs.rs +0
-0
samples/rust/rust_debugfs.rs fn build_inner(dir: &Dir) -> impl PinInit<File<Mutex<Inner>>> + '_ { · line 152
Quoted 7 lines
samples/rust/rust_debugfs.rs +1
-1@@ -147,7 +147,7 @@ fn build_inner(dir: &Dir) -> impl PinInit<File<Mutex<Inner>>> + '_ { dir.read_write_file (c"pair" ,new_mutex! (Inner { x:3 , y:10 })) } -fn new <'a>(pdev: &'a platform::Device <Core <'_>>) ->impl PinInit <Self ,Error > + 'a { +fn new <'a, 'b>(pdev: &'a platform::Device <Core <'b>>) ->impl PinInit <Self ,Error > +use <'a, 'b> {let debugfs =Dir ::new (c"sample_debugfs" );let dev = pdev.as_ref ();
Right, somehow debugfs is disabled in my test build that I didn't notice.
Best,
Gary
Danilo Krummrich
maintainerQuoted 1 line · Gary Guo
but
useis stable in 1.82+ so we can just use it instead.
Ah, right. I was confusing this with the situation before the version bump up from 1.78.
kernel test robot
build botkernel test robot noticed the following build errors:
Hi Gary,
kernel test robot noticed the following build errors:
[auto build test ERROR on a13c140cc289c0b7b3770bce5b3ad42ab35074aa]
url: https://github.com/intel-lab-lkp/linux/commits/Gary-Guo/rust-device-make-lifetime-on-Core-and-CoreInternal-invariant/20260714-041724 base: a13c140cc289c0b7b3770bce5b3ad42ab35074aa patch link: https://lore.kernel.org/r/20260713201455.640151-1-gary%40kernel.org patch subject: [PATCH] rust: device: make lifetime on `Core` and `CoreInternal` invariant config: x86_64-buildonly-randconfig-002-20260714 (https://download.01.org/0day-ci/archive/20260714/[email protected]/config) compiler: clang version 22.1.3 (https://github.com/llvm/llvm-project e9846648fd6183ee6d8cbdb4502213fcf902a211) rustc: rustc 1.96.0 (ac68faa20 2026-05-25) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260714/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> error[E0700]: hidden type for `impl pin_init::PinInit<RustDebugFs, kernel::error::Error> + 'a` captures lifetime that does not appear in bounds
--> samples/rust/rust_debugfs.rs:154:9
|
150 | fn new<'a>(pdev: &'a platform::Device<Core<'_>>) -> impl PinInit<Self, Error> + 'a {
| -------- ------------------------------ opaque type defined here
| |
>> | hidden type `impl pin_init::PinInit<RustDebugFs, kernel::error::Error>` captures the anonymous lifetime defined here
...
154 | / try_pin_init! {
155 | | Self {
156 | | _compatible <- debugfs.read_only_file(
157 | | c"compatible",
... |
176 | | }
| |_________^
| = note: this error originates in the macro ::pin_init::pin_init which comes from the expansion of the macro try_pin_init (in Nightly builds, run with -Z macro-backtrace for more info)
help: add a use<...> bound to explicitly capture '_
|
150 | fn new<'a>(pdev: &'a platform::Device<Core<'_>>) -> impl PinInit<Self, Error> + 'a + use<'a, '_> {
| +++++++++++++
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Quoted 1 line · Gary Guo
[PATCH] rust: device: make lifetime on
CoreandCoreInternalinvariant
Applied, thanks!
Branch: driver-core-testing
Tree: git://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git
[1/1] rust: device: make lifetime on Core and CoreInternal invariant
commit: f7acb19abcd8
[ Fixup the debugfs sample to use an explicit lifetime instead of
Core<'_>. - Danilo ]
The patch will appear in the next linux-next integration (typically within 24 hours on weekdays).
The patch is in the driver-core-testing branch and will be promoted to driver-core-next after validation.