linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rust: devres: initialize Devres::inner::data last
@ 2025-07-14 11:32 Danilo Krummrich
  2025-07-15  7:24 ` Alice Ryhl
  2025-07-15 20:29 ` Danilo Krummrich
  0 siblings, 2 replies; 5+ messages in thread
From: Danilo Krummrich @ 2025-07-14 11:32 UTC (permalink / raw)
  To: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, daniel.almeida,
	m.wilczynski
  Cc: rust-for-linux, linux-kernel, Danilo Krummrich

Users may want to access the Devres object from callbacks registered
through the initialization of Devres::inner::data.

For those accesses to be valid, Devres::inner::data must be initialized
last [1].

Credit to Boqun for spotting this [2].

Link: https://lore.kernel.org/lkml/DBBPHO26CPBS.2OVI1OERCB2J5@kernel.org/ [1]
Link: https://lore.kernel.org/lkml/aHSmxWeIy3L-AKIV@Mac.home/ [2]
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/devres.rs | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index a8a2d24cac24..152a89b78943 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -137,14 +137,10 @@ pub fn new<'a, E>(
         let callback = Self::devres_callback;
 
         try_pin_init!(&this in Self {
-            // INVARIANT: `inner` is properly initialized.
-            inner <- Opaque::pin_init(try_pin_init!(Inner {
-                data <- Revocable::new(data),
-                devm <- Completion::new(),
-                revoke <- Completion::new(),
-            })),
+            dev: dev.into(),
             callback,
-            dev: {
+            // INVARIANT: `inner` is properly initialized.
+            inner <- {
                 // SAFETY: `this` is a valid pointer to uninitialized memory.
                 let inner = unsafe { &raw mut (*this.as_ptr()).inner };
 
@@ -158,7 +154,11 @@ pub fn new<'a, E>(
                     bindings::devm_add_action(dev.as_raw(), Some(callback), inner.cast())
                 })?;
 
-                dev.into()
+                Opaque::pin_init(try_pin_init!(Inner {
+                    devm <- Completion::new(),
+                    revoke <- Completion::new(),
+                    data <- Revocable::new(data),
+                }))
             },
         })
     }

base-commit: 3964d07dd821efe9680e90c51c86661a98e60a0f
prerequisite-patch-id: 12a88f7c46bec668e4f71ec0d94b1782d9446d0b
-- 
2.50.0


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

* Re: [PATCH] rust: devres: initialize Devres::inner::data last
  2025-07-14 11:32 [PATCH] rust: devres: initialize Devres::inner::data last Danilo Krummrich
@ 2025-07-15  7:24 ` Alice Ryhl
  2025-07-15  8:22   ` Danilo Krummrich
  2025-07-15 20:29 ` Danilo Krummrich
  1 sibling, 1 reply; 5+ messages in thread
From: Alice Ryhl @ 2025-07-15  7:24 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, daniel.almeida, m.wilczynski,
	rust-for-linux, linux-kernel

On Mon, Jul 14, 2025 at 01:32:35PM +0200, Danilo Krummrich wrote:
> Users may want to access the Devres object from callbacks registered
> through the initialization of Devres::inner::data.
> 
> For those accesses to be valid, Devres::inner::data must be initialized
> last [1].
> 
> Credit to Boqun for spotting this [2].
> 
> Link: https://lore.kernel.org/lkml/DBBPHO26CPBS.2OVI1OERCB2J5@kernel.org/ [1]
> Link: https://lore.kernel.org/lkml/aHSmxWeIy3L-AKIV@Mac.home/ [2]
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> base-commit: 3964d07dd821efe9680e90c51c86661a98e60a0f

I couldn't find this commit. Where does this apply?

Alice

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

* Re: [PATCH] rust: devres: initialize Devres::inner::data last
  2025-07-15  7:24 ` Alice Ryhl
@ 2025-07-15  8:22   ` Danilo Krummrich
  2025-07-15  8:44     ` Alice Ryhl
  0 siblings, 1 reply; 5+ messages in thread
From: Danilo Krummrich @ 2025-07-15  8:22 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, daniel.almeida, m.wilczynski,
	rust-for-linux, linux-kernel

On Tue Jul 15, 2025 at 9:24 AM CEST, Alice Ryhl wrote:
> On Mon, Jul 14, 2025 at 01:32:35PM +0200, Danilo Krummrich wrote:
>> Users may want to access the Devres object from callbacks registered
>> through the initialization of Devres::inner::data.
>> 
>> For those accesses to be valid, Devres::inner::data must be initialized
>> last [1].
>> 
>> Credit to Boqun for spotting this [2].
>> 
>> Link: https://lore.kernel.org/lkml/DBBPHO26CPBS.2OVI1OERCB2J5@kernel.org/ [1]
>> Link: https://lore.kernel.org/lkml/aHSmxWeIy3L-AKIV@Mac.home/ [2]
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> ---
>> base-commit: 3964d07dd821efe9680e90c51c86661a98e60a0f
>
> I couldn't find this commit. Where does this apply?

The commit is in driver-core-next.

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

* Re: [PATCH] rust: devres: initialize Devres::inner::data last
  2025-07-15  8:22   ` Danilo Krummrich
@ 2025-07-15  8:44     ` Alice Ryhl
  0 siblings, 0 replies; 5+ messages in thread
From: Alice Ryhl @ 2025-07-15  8:44 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, daniel.almeida, m.wilczynski,
	rust-for-linux, linux-kernel

On Tue, Jul 15, 2025 at 10:22 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Tue Jul 15, 2025 at 9:24 AM CEST, Alice Ryhl wrote:
> > On Mon, Jul 14, 2025 at 01:32:35PM +0200, Danilo Krummrich wrote:
> >> Users may want to access the Devres object from callbacks registered
> >> through the initialization of Devres::inner::data.
> >>
> >> For those accesses to be valid, Devres::inner::data must be initialized
> >> last [1].
> >>
> >> Credit to Boqun for spotting this [2].
> >>
> >> Link: https://lore.kernel.org/lkml/DBBPHO26CPBS.2OVI1OERCB2J5@kernel.org/ [1]
> >> Link: https://lore.kernel.org/lkml/aHSmxWeIy3L-AKIV@Mac.home/ [2]
> >> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> >> ---
> >> base-commit: 3964d07dd821efe9680e90c51c86661a98e60a0f
> >
> > I couldn't find this commit. Where does this apply?
>
> The commit is in driver-core-next.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

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

* Re: [PATCH] rust: devres: initialize Devres::inner::data last
  2025-07-14 11:32 [PATCH] rust: devres: initialize Devres::inner::data last Danilo Krummrich
  2025-07-15  7:24 ` Alice Ryhl
@ 2025-07-15 20:29 ` Danilo Krummrich
  1 sibling, 0 replies; 5+ messages in thread
From: Danilo Krummrich @ 2025-07-15 20:29 UTC (permalink / raw)
  To: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, daniel.almeida,
	m.wilczynski
  Cc: rust-for-linux, linux-kernel

On Mon Jul 14, 2025 at 1:32 PM CEST, Danilo Krummrich wrote:
> Users may want to access the Devres object from callbacks registered
> through the initialization of Devres::inner::data.
>
> For those accesses to be valid, Devres::inner::data must be initialized
> last [1].
>
> Credit to Boqun for spotting this [2].
>
> Link: https://lore.kernel.org/lkml/DBBPHO26CPBS.2OVI1OERCB2J5@kernel.org/ [1]
> Link: https://lore.kernel.org/lkml/aHSmxWeIy3L-AKIV@Mac.home/ [2]
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Applied to driver-core-testing, thanks!

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

end of thread, other threads:[~2025-07-15 20:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-14 11:32 [PATCH] rust: devres: initialize Devres::inner::data last Danilo Krummrich
2025-07-15  7:24 ` Alice Ryhl
2025-07-15  8:22   ` Danilo Krummrich
2025-07-15  8:44     ` Alice Ryhl
2025-07-15 20:29 ` Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).