diff options
| author | Danilo Krummrich <dakr@kernel.org> | 2026-06-28 19:44:38 +0200 |
|---|---|---|
| committer | Danilo Krummrich <dakr@kernel.org> | 2026-07-14 00:57:19 +0200 |
| commit | acc516dfa1972d31836b50abc0115216cd0fccc5 (patch) | |
| tree | d547120b882d2e75bdd0717cb4f3ca73028e8e5f | |
| parent | b07fc8d60bd30caaba4d293929459780166da194 (diff) | |
| download | linux-next-acc516dfa1972d31836b50abc0115216cd0fccc5.tar.gz linux-next-acc516dfa1972d31836b50abc0115216cd0fccc5.zip | |
rust: devres: fix race between concurrent revokers
There is a potential race condition when two paths try to revoke a
Devres concurrently.
The driver core's devres_release_all() calls Revocable::revoke() via the
release callback, while Devres::drop() calls revoke_nosync() on another
CPU.
The revoker that does not claim the is_available swap returns
immediately, but the revoker that did may still be executing
drop_in_place() on the inner data. This can cause a use-after-free when
the other revoker's caller proceeds to drop adjacent resources that
drop_in_place() still references (e.g., Devres<DmaMappedSgt> racing with
SGTable freeing the backing sg_table and pages).
Fix this by adding a Completion. The release callback signals the
Completion after revoke() finishes, and Devres::drop() waits for it when
it loses the is_available swap. This ensures the wrapped object is fully
torn down before Devres::drop() returns.
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/dri-devel/20260612202841.2577C1F000E9@smtp.kernel.org/
Fixes: 05aa6fb1c21d ("rust: scatterlist: Add abstraction for sg_table")
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://patch.msgid.link/20260628174451.2275679-1-dakr@kernel.org
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
| -rw-r--r-- | rust/kernel/devres.rs | 18 |
1 files changed, 16 insertions, 2 deletions
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 20f94030f977..4a1e5eec78ab 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -21,7 +21,8 @@ use crate::{ sync::{ aref::ARef, rcu, - Arc, // + Arc, + Completion, // }, types::{ CovariantForLt, @@ -39,6 +40,8 @@ struct Inner<T> { node: Opaque<bindings::devres_node>, #[pin] data: Revocable<T>, + #[pin] + revocation: Completion, } /// This abstraction is meant to be used by subsystems to containerize [`Device`] bound resources to @@ -55,6 +58,10 @@ struct Inner<T> { /// After the [`Devres`] has been unbound it is not possible to access the encapsulated resource /// anymore. /// +/// When a [`Devres`] is dropped, it is guaranteed that `T` has been fully dropped by the time +/// [`Devres::drop`] returns, even if a concurrent revocation through the release callback is in +/// progress. +/// /// [`Devres`] users should make sure to simply free the corresponding backing resource in `T`'s /// [`Drop`] implementation. /// @@ -222,6 +229,7 @@ impl<T: Send + 'static> Devres<T> { }; }), data <- Revocable::new(data), + revocation <- Completion::new(), }), GFP_KERNEL, )?; @@ -259,7 +267,9 @@ impl<T: Send + 'static> Devres<T> { // SAFETY: `inner` is a valid `Inner<T>` pointer. let inner = unsafe { &*inner }; - inner.data.revoke(); + if inner.data.revoke() { + inner.revocation.complete_all(); + } } #[allow(clippy::missing_safety_doc)] @@ -363,6 +373,10 @@ impl<T: Send + 'static> Drop for Devres<T> { // this additional reference count. drop(unsafe { Arc::from_raw(Arc::as_ptr(&self.inner)) }); } + } else { + // The release callback is concurrently revoking; wait for it to finish + // `drop_in_place()` of the wrapped object before returning. + self.inner.revocation.wait_for_completion(); } } } |
