From: Danilo Krummrich Date: Tue, 12 Aug 2025 13:09:06 +0000 (+0200) Subject: rust: devres: fix leaking call to devm_add_action() X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=75a7b151e808355a1fdf972e85da137612b8f2ae;p=users%2Fjedix%2Flinux-maple.git rust: devres: fix leaking call to devm_add_action() When the data argument of Devres::new() is Err(), we leak the preceding call to devm_add_action(). In order to fix this, call devm_add_action() in a unit type initializer in try_pin_init!() after the initializers of all other fields. Fixes: f5d3ef25d238 ("rust: devres: get rid of Devres' inner Arc") Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250812130928.11075-1-dakr@kernel.org Signed-off-by: Danilo Krummrich --- diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index da18091143a67..d04e3fcebafbb 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -115,10 +115,11 @@ pub struct Devres { /// Contains all the fields shared with [`Self::callback`]. // TODO: Replace with `UnsafePinned`, once available. // - // Subsequently, the `drop_in_place()` in `Devres::drop` and the explicit `Send` and `Sync' - // impls can be removed. + // Subsequently, the `drop_in_place()` in `Devres::drop` and `Devres::new` as well as the + // explicit `Send` and `Sync' impls can be removed. #[pin] inner: Opaque>, + _add_action: (), } impl Devres { @@ -140,7 +141,15 @@ impl Devres { dev: dev.into(), callback, // INVARIANT: `inner` is properly initialized. - inner <- { + inner <- Opaque::pin_init(try_pin_init!(Inner { + devm <- Completion::new(), + revoke <- Completion::new(), + data <- Revocable::new(data), + })), + // TODO: Replace with "initializer code blocks" [1] once available. + // + // [1] https://github.com/Rust-for-Linux/pin-init/pull/69 + _add_action: { // SAFETY: `this` is a valid pointer to uninitialized memory. let inner = unsafe { &raw mut (*this.as_ptr()).inner }; @@ -152,13 +161,13 @@ impl Devres { // live at least as long as the returned `impl PinInit`. to_result(unsafe { bindings::devm_add_action(dev.as_raw(), Some(callback), inner.cast()) - })?; + }).inspect_err(|_| { + let inner = Opaque::cast_into(inner); - Opaque::pin_init(try_pin_init!(Inner { - devm <- Completion::new(), - revoke <- Completion::new(), - data <- Revocable::new(data), - })) + // SAFETY: `inner` is a valid pointer to an `Inner` and valid for both reads + // and writes. + unsafe { core::ptr::drop_in_place(inner) }; + })?; }, }) }