Rust for Linux List
 help / color / mirror / Atom feed
* [PATCH] rust: devres: add 'static bound to Devres<T>
@ 2026-05-26  0:04 Danilo Krummrich
  2026-05-26  0:09 ` Danilo Krummrich
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Danilo Krummrich @ 2026-05-26  0:04 UTC (permalink / raw)
  To: gregkh, rafael, dakr, ojeda, boqun, gary, bjorn3_gh, lossin,
	a.hindborg, aliceryhl, tmgross
  Cc: driver-core, linux-kernel, rust-for-linux

Devres::new() registers a callback with the C devres subsystem via
devres_node_add(). If the Devres is leaked (e.g. via
core::mem::forget(), which is safe), its Drop impl never runs, and the
devres release callback will revoke the inner Revocable on device
unbind, which drops T in place. If T contains non-'static references,
those may be dangling by that point.

Add a 'static bound to prevent storing types with borrowed data in
Devres.

Fixes: 76c01ded724b ("rust: add devres abstraction")
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/devres.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 9e5f93aed20c..2e258d31a45c 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -122,7 +122,7 @@ struct Inner<T> {
 /// # Ok(())
 /// # }
 /// ```
-pub struct Devres<T: Send> {
+pub struct Devres<T: Send + 'static> {
     dev: ARef<Device>,
     inner: Arc<Inner<T>>,
 }
@@ -184,7 +184,7 @@ pub(super) unsafe fn devres_node_remove(
     }
 }
 
-impl<T: Send> Devres<T> {
+impl<T: Send + 'static> Devres<T> {
     /// Creates a new [`Devres`] instance of the given `data`.
     ///
     /// The `data` encapsulated within the returned `Devres` instance' `data` will be
@@ -349,7 +349,7 @@ unsafe impl<T: Send> Send for Devres<T> {}
 // SAFETY: `Devres` can be shared with any task, if `T: Sync`.
 unsafe impl<T: Send + Sync> Sync for Devres<T> {}
 
-impl<T: Send> Drop for Devres<T> {
+impl<T: Send + 'static> Drop for Devres<T> {
     fn drop(&mut self) {
         // SAFETY: When `drop` runs, it is guaranteed that nobody is accessing the revocable data
         // anymore, hence it is safe not to wait for the grace period to finish.

base-commit: 56785dcb2ef6d3cff82ac33f2e34db94377416a3
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] rust: devres: add 'static bound to Devres<T>
  2026-05-26  0:04 [PATCH] rust: devres: add 'static bound to Devres<T> Danilo Krummrich
@ 2026-05-26  0:09 ` Danilo Krummrich
  2026-05-26  1:08 ` Eliot Courtney
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Danilo Krummrich @ 2026-05-26  0:09 UTC (permalink / raw)
  To: gregkh, rafael, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross
  Cc: driver-core, linux-kernel, rust-for-linux

On Tue May 26, 2026 at 2:04 AM CEST, Danilo Krummrich wrote:
> Devres::new() registers a callback with the C devres subsystem via
> devres_node_add(). If the Devres is leaked (e.g. via
> core::mem::forget(), which is safe), its Drop impl never runs, and the
> devres release callback will revoke the inner Revocable on device
> unbind, which drops T in place. If T contains non-'static references,
> those may be dangling by that point.
>
> Add a 'static bound to prevent storing types with borrowed data in
> Devres.
>
> Fixes: 76c01ded724b ("rust: add devres abstraction")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Cc: stable@vger.kernel.org

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] rust: devres: add 'static bound to Devres<T>
  2026-05-26  0:04 [PATCH] rust: devres: add 'static bound to Devres<T> Danilo Krummrich
  2026-05-26  0:09 ` Danilo Krummrich
@ 2026-05-26  1:08 ` Eliot Courtney
  2026-05-26  1:49 ` Alexandre Courbot
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Eliot Courtney @ 2026-05-26  1:08 UTC (permalink / raw)
  To: Danilo Krummrich, gregkh, rafael, ojeda, boqun, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross
  Cc: driver-core, linux-kernel, rust-for-linux

On Tue May 26, 2026 at 9:04 AM JST, Danilo Krummrich wrote:
> Devres::new() registers a callback with the C devres subsystem via
> devres_node_add(). If the Devres is leaked (e.g. via
> core::mem::forget(), which is safe), its Drop impl never runs, and the
> devres release callback will revoke the inner Revocable on device
> unbind, which drops T in place. If T contains non-'static references,
> those may be dangling by that point.
>
> Add a 'static bound to prevent storing types with borrowed data in
> Devres.
>
> Fixes: 76c01ded724b ("rust: add devres abstraction")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] rust: devres: add 'static bound to Devres<T>
  2026-05-26  0:04 [PATCH] rust: devres: add 'static bound to Devres<T> Danilo Krummrich
  2026-05-26  0:09 ` Danilo Krummrich
  2026-05-26  1:08 ` Eliot Courtney
@ 2026-05-26  1:49 ` Alexandre Courbot
  2026-05-27 14:44 ` Gary Guo
  2026-05-28 23:10 ` Danilo Krummrich
  4 siblings, 0 replies; 12+ messages in thread
From: Alexandre Courbot @ 2026-05-26  1:49 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross, driver-core, linux-kernel, rust-for-linux

On Tue May 26, 2026 at 9:04 AM JST, Danilo Krummrich wrote:
> Devres::new() registers a callback with the C devres subsystem via
> devres_node_add(). If the Devres is leaked (e.g. via
> core::mem::forget(), which is safe), its Drop impl never runs, and the
> devres release callback will revoke the inner Revocable on device
> unbind, which drops T in place. If T contains non-'static references,
> those may be dangling by that point.
>
> Add a 'static bound to prevent storing types with borrowed data in
> Devres.
>
> Fixes: 76c01ded724b ("rust: add devres abstraction")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] rust: devres: add 'static bound to Devres<T>
  2026-05-26  0:04 [PATCH] rust: devres: add 'static bound to Devres<T> Danilo Krummrich
                   ` (2 preceding siblings ...)
  2026-05-26  1:49 ` Alexandre Courbot
@ 2026-05-27 14:44 ` Gary Guo
  2026-05-27 18:04   ` Danilo Krummrich
  2026-05-28 23:10 ` Danilo Krummrich
  4 siblings, 1 reply; 12+ messages in thread
From: Gary Guo @ 2026-05-27 14:44 UTC (permalink / raw)
  To: Danilo Krummrich, gregkh, rafael, ojeda, boqun, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross
  Cc: driver-core, linux-kernel, rust-for-linux

On Tue May 26, 2026 at 1:04 AM BST, Danilo Krummrich wrote:
> Devres::new() registers a callback with the C devres subsystem via
> devres_node_add(). If the Devres is leaked (e.g. via
> core::mem::forget(), which is safe), its Drop impl never runs, and the
> devres release callback will revoke the inner Revocable on device
> unbind, which drops T in place. If T contains non-'static references,
> those may be dangling by that point.
>
> Add a 'static bound to prevent storing types with borrowed data in
> Devres.

The bound should be added on `Devres::new` instead.

Best,
Gary

>
> Fixes: 76c01ded724b ("rust: add devres abstraction")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/devres.rs | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> index 9e5f93aed20c..2e258d31a45c 100644
> --- a/rust/kernel/devres.rs
> +++ b/rust/kernel/devres.rs
> @@ -122,7 +122,7 @@ struct Inner<T> {
>  /// # Ok(())
>  /// # }
>  /// ```
> -pub struct Devres<T: Send> {
> +pub struct Devres<T: Send + 'static> {
>      dev: ARef<Device>,
>      inner: Arc<Inner<T>>,
>  }
> @@ -184,7 +184,7 @@ pub(super) unsafe fn devres_node_remove(
>      }
>  }
>  
> -impl<T: Send> Devres<T> {
> +impl<T: Send + 'static> Devres<T> {
>      /// Creates a new [`Devres`] instance of the given `data`.
>      ///
>      /// The `data` encapsulated within the returned `Devres` instance' `data` will be
> @@ -349,7 +349,7 @@ unsafe impl<T: Send> Send for Devres<T> {}
>  // SAFETY: `Devres` can be shared with any task, if `T: Sync`.
>  unsafe impl<T: Send + Sync> Sync for Devres<T> {}
>  
> -impl<T: Send> Drop for Devres<T> {
> +impl<T: Send + 'static> Drop for Devres<T> {
>      fn drop(&mut self) {
>          // SAFETY: When `drop` runs, it is guaranteed that nobody is accessing the revocable data
>          // anymore, hence it is safe not to wait for the grace period to finish.
>
> base-commit: 56785dcb2ef6d3cff82ac33f2e34db94377416a3



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] rust: devres: add 'static bound to Devres<T>
  2026-05-27 14:44 ` Gary Guo
@ 2026-05-27 18:04   ` Danilo Krummrich
  2026-05-27 18:07     ` Gary Guo
  0 siblings, 1 reply; 12+ messages in thread
From: Danilo Krummrich @ 2026-05-27 18:04 UTC (permalink / raw)
  To: Gary Guo
  Cc: gregkh, rafael, ojeda, boqun, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross, driver-core, linux-kernel, rust-for-linux

On Wed May 27, 2026 at 4:44 PM CEST, Gary Guo wrote:
> On Tue May 26, 2026 at 1:04 AM BST, Danilo Krummrich wrote:
>> Add a 'static bound to prevent storing types with borrowed data in
>> Devres.
>
> The bound should be added on `Devres::new` instead.

I did consider this as it is generally recommended to minimize bounds on
structs.

However, a Devres<T> with non-'static T is semantically nonsensical, not just
unconstructible, and I think type level bound represents that better.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] rust: devres: add 'static bound to Devres<T>
  2026-05-27 18:04   ` Danilo Krummrich
@ 2026-05-27 18:07     ` Gary Guo
  2026-05-27 18:13       ` Danilo Krummrich
  0 siblings, 1 reply; 12+ messages in thread
From: Gary Guo @ 2026-05-27 18:07 UTC (permalink / raw)
  To: Danilo Krummrich, Gary Guo
  Cc: gregkh, rafael, ojeda, boqun, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross, driver-core, linux-kernel, rust-for-linux

On Wed May 27, 2026 at 7:04 PM BST, Danilo Krummrich wrote:
> On Wed May 27, 2026 at 4:44 PM CEST, Gary Guo wrote:
>> On Tue May 26, 2026 at 1:04 AM BST, Danilo Krummrich wrote:
>>> Add a 'static bound to prevent storing types with borrowed data in
>>> Devres.
>>
>> The bound should be added on `Devres::new` instead.
>
> I did consider this as it is generally recommended to minimize bounds on
> structs.
>
> However, a Devres<T> with non-'static T is semantically nonsensical, not just
> unconstructible, and I think type level bound represents that better.

Technically `Devres` can contain references to the registration, which is known
to outlive the bound device.

Best,
Gary

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] rust: devres: add 'static bound to Devres<T>
  2026-05-27 18:07     ` Gary Guo
@ 2026-05-27 18:13       ` Danilo Krummrich
  2026-05-27 19:25         ` Gary Guo
  0 siblings, 1 reply; 12+ messages in thread
From: Danilo Krummrich @ 2026-05-27 18:13 UTC (permalink / raw)
  To: Gary Guo
  Cc: gregkh, rafael, ojeda, boqun, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross, driver-core, linux-kernel, rust-for-linux

On Wed May 27, 2026 at 8:07 PM CEST, Gary Guo wrote:
> On Wed May 27, 2026 at 7:04 PM BST, Danilo Krummrich wrote:
>> On Wed May 27, 2026 at 4:44 PM CEST, Gary Guo wrote:
>>> On Tue May 26, 2026 at 1:04 AM BST, Danilo Krummrich wrote:
>>>> Add a 'static bound to prevent storing types with borrowed data in
>>>> Devres.
>>>
>>> The bound should be added on `Devres::new` instead.
>>
>> I did consider this as it is generally recommended to minimize bounds on
>> structs.
>>
>> However, a Devres<T> with non-'static T is semantically nonsensical, not just
>> unconstructible, and I think type level bound represents that better.
>
> Technically `Devres` can contain references to the registration, which is known
> to outlive the bound device.

That's technically true, but how would you express "outlives the device" as a
lifetime bound on Devres?

Additionally, if the borrowed data outlives the device bound, you don't need
device-managed revocation for it in the first place.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] rust: devres: add 'static bound to Devres<T>
  2026-05-27 18:13       ` Danilo Krummrich
@ 2026-05-27 19:25         ` Gary Guo
  2026-05-27 19:49           ` Danilo Krummrich
  0 siblings, 1 reply; 12+ messages in thread
From: Gary Guo @ 2026-05-27 19:25 UTC (permalink / raw)
  To: Danilo Krummrich, Gary Guo
  Cc: gregkh, rafael, ojeda, boqun, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross, driver-core, linux-kernel, rust-for-linux

On Wed May 27, 2026 at 7:13 PM BST, Danilo Krummrich wrote:
> On Wed May 27, 2026 at 8:07 PM CEST, Gary Guo wrote:
>> On Wed May 27, 2026 at 7:04 PM BST, Danilo Krummrich wrote:
>>> On Wed May 27, 2026 at 4:44 PM CEST, Gary Guo wrote:
>>>> On Tue May 26, 2026 at 1:04 AM BST, Danilo Krummrich wrote:
>>>>> Add a 'static bound to prevent storing types with borrowed data in
>>>>> Devres.
>>>>
>>>> The bound should be added on `Devres::new` instead.
>>>
>>> I did consider this as it is generally recommended to minimize bounds on
>>> structs.
>>>
>>> However, a Devres<T> with non-'static T is semantically nonsensical, not just
>>> unconstructible, and I think type level bound represents that better.
>>
>> Technically `Devres` can contain references to the registration, which is known
>> to outlive the bound device.
>
> That's technically true, but how would you express "outlives the device" as a
> lifetime bound on Devres?

Just having the lifetime would mean it outlives the device (lifetimes that
cannot be guaranteed to outlive the device cannot be used in devres).

In a world with registration data, I would imagine all lifetime parameters on
registration data are also valid for `Devres`.

>
> Additionally, if the borrowed data outlives the device bound, you don't need
> device-managed revocation for it in the first place.

Resources that depend on both registration data and device resource perhaps?

Best,
Gary

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] rust: devres: add 'static bound to Devres<T>
  2026-05-27 19:25         ` Gary Guo
@ 2026-05-27 19:49           ` Danilo Krummrich
  2026-05-28 23:10             ` Danilo Krummrich
  0 siblings, 1 reply; 12+ messages in thread
From: Danilo Krummrich @ 2026-05-27 19:49 UTC (permalink / raw)
  To: Gary Guo
  Cc: gregkh, rafael, ojeda, boqun, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross, driver-core, linux-kernel, rust-for-linux

On Wed May 27, 2026 at 9:25 PM CEST, Gary Guo wrote:
> On Wed May 27, 2026 at 7:13 PM BST, Danilo Krummrich wrote:
>> On Wed May 27, 2026 at 8:07 PM CEST, Gary Guo wrote:
>>> On Wed May 27, 2026 at 7:04 PM BST, Danilo Krummrich wrote:
>>>> On Wed May 27, 2026 at 4:44 PM CEST, Gary Guo wrote:
>>>>> On Tue May 26, 2026 at 1:04 AM BST, Danilo Krummrich wrote:
>>>>>> Add a 'static bound to prevent storing types with borrowed data in
>>>>>> Devres.
>>>>>
>>>>> The bound should be added on `Devres::new` instead.
>>>>
>>>> I did consider this as it is generally recommended to minimize bounds on
>>>> structs.
>>>>
>>>> However, a Devres<T> with non-'static T is semantically nonsensical, not just
>>>> unconstructible, and I think type level bound represents that better.
>>>
>>> Technically `Devres` can contain references to the registration, which is known
>>> to outlive the bound device.
>>
>> That's technically true, but how would you express "outlives the device" as a
>> lifetime bound on Devres?
>
> Just having the lifetime would mean it outlives the device (lifetimes that
> cannot be guaranteed to outlive the device cannot be used in devres).
>
> In a world with registration data, I would imagine all lifetime parameters on
> registration data are also valid for `Devres`.

[...]

> Resources that depend on both registration data and device resource perhaps?

You'd still need a separate constructor for this, and I'm still not convinced
that this makes a lot of sense.

That said if you feel strongly about it, I don't mind moving the 'static bound
to new() too much.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] rust: devres: add 'static bound to Devres<T>
  2026-05-27 19:49           ` Danilo Krummrich
@ 2026-05-28 23:10             ` Danilo Krummrich
  0 siblings, 0 replies; 12+ messages in thread
From: Danilo Krummrich @ 2026-05-28 23:10 UTC (permalink / raw)
  To: Gary Guo
  Cc: gregkh, rafael, ojeda, boqun, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross, driver-core, linux-kernel, rust-for-linux

On Wed May 27, 2026 at 9:49 PM CEST, Danilo Krummrich wrote:
> That said if you feel strongly about it, I don't mind moving the 'static bound
> to new() too much.

Keeping the bound on the struct for now, given that we agree to move it if it
creeps to more places off-list.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] rust: devres: add 'static bound to Devres<T>
  2026-05-26  0:04 [PATCH] rust: devres: add 'static bound to Devres<T> Danilo Krummrich
                   ` (3 preceding siblings ...)
  2026-05-27 14:44 ` Gary Guo
@ 2026-05-28 23:10 ` Danilo Krummrich
  4 siblings, 0 replies; 12+ messages in thread
From: Danilo Krummrich @ 2026-05-28 23:10 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg,
	aliceryhl, tmgross, driver-core, linux-kernel, rust-for-linux

On Tue, 26 May 2026 02:04:41 +0200, Danilo Krummrich wrote:
> [PATCH] rust: devres: add 'static bound to Devres<T>

Applied, thanks!

  Branch: driver-core-testing
  Tree:   git://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git

[1/1] rust: devres: add 'static bound to Devres<T>
      commit: 016267b521b1

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-05-28 23:10 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-26  0:04 [PATCH] rust: devres: add 'static bound to Devres<T> Danilo Krummrich
2026-05-26  0:09 ` Danilo Krummrich
2026-05-26  1:08 ` Eliot Courtney
2026-05-26  1:49 ` Alexandre Courbot
2026-05-27 14:44 ` Gary Guo
2026-05-27 18:04   ` Danilo Krummrich
2026-05-27 18:07     ` Gary Guo
2026-05-27 18:13       ` Danilo Krummrich
2026-05-27 19:25         ` Gary Guo
2026-05-27 19:49           ` Danilo Krummrich
2026-05-28 23:10             ` Danilo Krummrich
2026-05-28 23:10 ` Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox