This series is aimed at fixing a soundness issue with how dynamically allocated LockClassKeys are handled. Currently, LockClassKeys can be used without being Pin'd, which can break lockdep since it relies on address stability. Similarly, these keys are not automatically (de)registered with lockdep.
At the suggestion of Alice Ryhl, this series includes a patch for -stable kernels that disables dynamically allocated keys. This prevents backported patches from using the unsound implementation.
Currently, this series requires that all dynamically allocated LockClassKeys have a lifetime of 'static (i.e., they must be leaked after allocation). This is because Lock does not currently keep a reference to the LockClassKey, instead passing it to C via FFI. This causes a problem because the rust compiler would allow creating a 'static Lock with a 'a LockClassKey (with 'a < 'static) while C would expect the LockClassKey to live as long as the lock. This problem represents an avenue for future work.
--- Changes in v4: - Expand the cover letter in the 2nd patch to explain why Pin is used despite being redundant at the moment (Thanks Benno Lossin). - Include a Fixes tag in the 1st patch (Thanks Miguel Ojeda). - Link to v3: https://lore.kernel.org/r/20250205-rust-lockdep-v3-0-5313e83a0bef@gmail.com
Changes in v3: - Rebased on rust-next - Fixed clippy/compiler warninings (Thanks Boqun Feng) - Link to v2: https://lore.kernel.org/r/20241219-rust-lockdep-v2-0-f65308fbc5ca@gmail.com
Changes in v2: - Dropped formatting change that's already fixed upstream (Thanks Dirk Behme). - Moved safety comment to the right point in the patch series (Thanks Dirk Behme and Boqun Feng). - Added an example of dynamic LockClassKey usage (Thanks Boqun Feng). - Link to v1: https://lore.kernel.org/r/20241004-rust-lockdep-v1-0-e9a5c45721fc@gmail.com
Changes from RFC: - Split into two commits so that dynamically allocated LockClassKeys are removed from stable kernels. (Thanks Alice Ryhl) - Extract calls to C lockdep functions into helpers so things build properly when LOCKDEP=n. (Thanks Benno Lossin) - Remove extraneous `get_ref()` calls. (Thanks Benno Lossin) - Provide better documentation for `new_dynamic()`. (Thanks Benno Lossin) - Ran rustfmt to fix formatting and some extraneous changes. (Thanks Alice Ryhl and Benno Lossin) - Link to RFC: https://lore.kernel.org/r/20240905-rust-lockdep-v1-1-d2c9c21aa8b2@gmail.com
--- Mitchell Levy (2): rust: lockdep: Remove support for dynamically allocated LockClassKeys rust: lockdep: Use Pin for all LockClassKey usages
rust/helpers/helpers.c | 1 + rust/helpers/sync.c | 13 +++++++++ rust/kernel/sync.rs | 63 ++++++++++++++++++++++++++++++++++------- rust/kernel/sync/condvar.rs | 5 ++-- rust/kernel/sync/lock.rs | 9 ++---- rust/kernel/sync/lock/global.rs | 5 ++-- rust/kernel/sync/poll.rs | 2 +- rust/kernel/workqueue.rs | 2 +- 8 files changed, 77 insertions(+), 23 deletions(-) --- base-commit: beeb78d46249cab8b2b8359a2ce8fa5376b5ad2d change-id: 20240905-rust-lockdep-d3e30521c8ba
Best regards,
Currently, dynamically allocated LockCLassKeys can be used from the Rust side without having them registered. This is a soundness issue, so remove them.
Suggested-by: Alice Ryhl aliceryhl@google.com Link: https://lore.kernel.org/rust-for-linux/20240815074519.2684107-3-nmi@metaspac... Cc: stable@vger.kernel.org Fixes: 6ea5aa08857a ("rust: sync: introduce `LockClassKey`") Reviewed-by: Benno Lossin benno.lossin@proton.me Signed-off-by: Mitchell Levy levymitchell0@gmail.com --- rust/kernel/sync.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs index 3498fb344dc9..16eab9138b2b 100644 --- a/rust/kernel/sync.rs +++ b/rust/kernel/sync.rs @@ -30,28 +30,20 @@ unsafe impl Sync for LockClassKey {}
impl LockClassKey { - /// Creates a new lock class key. - pub const fn new() -> Self { - Self(Opaque::uninit()) - } - pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key { self.0.get() } }
-impl Default for LockClassKey { - fn default() -> Self { - Self::new() - } -} - /// Defines a new static lock class and returns a pointer to it. #[doc(hidden)] #[macro_export] macro_rules! static_lock_class { () => {{ - static CLASS: $crate::sync::LockClassKey = $crate::sync::LockClassKey::new(); + static CLASS: $crate::sync::LockClassKey = + // SAFETY: lockdep expects uninitialized memory when it's handed a statically allocated + // lock_class_key + unsafe { ::core::mem::MaybeUninit::uninit().assume_init() }; &CLASS }}; }
The following commit has been merged into the locking/core branch of tip:
Commit-ID: 5fc1506d33db23894e74caf048ba5591f4986767 Gitweb: https://git.kernel.org/tip/5fc1506d33db23894e74caf048ba5591f4986767 Author: Mitchell Levy levymitchell0@gmail.com AuthorDate: Fri, 07 Feb 2025 16:39:08 -08:00 Committer: Boqun Feng boqun.feng@gmail.com CommitterDate: Tue, 25 Feb 2025 08:53:08 -08:00
rust: lockdep: Remove support for dynamically allocated LockClassKeys
Currently, dynamically allocated LockCLassKeys can be used from the Rust side without having them registered. This is a soundness issue, so remove them.
Suggested-by: Alice Ryhl aliceryhl@google.com Link: https://lore.kernel.org/rust-for-linux/20240815074519.2684107-3-nmi@metaspac... Cc: stable@vger.kernel.org Fixes: 6ea5aa08857a ("rust: sync: introduce `LockClassKey`") Reviewed-by: Benno Lossin benno.lossin@proton.me Signed-off-by: Mitchell Levy levymitchell0@gmail.com Signed-off-by: Boqun Feng boqun.feng@gmail.com Link: https://lore.kernel.org/r/20250207-rust-lockdep-v4-1-7a50a7e88656@gmail.com --- rust/kernel/sync.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs index 3498fb3..16eab91 100644 --- a/rust/kernel/sync.rs +++ b/rust/kernel/sync.rs @@ -30,28 +30,20 @@ pub struct LockClassKey(Opaquebindings::lock_class_key); unsafe impl Sync for LockClassKey {}
impl LockClassKey { - /// Creates a new lock class key. - pub const fn new() -> Self { - Self(Opaque::uninit()) - } - pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key { self.0.get() } }
-impl Default for LockClassKey { - fn default() -> Self { - Self::new() - } -} - /// Defines a new static lock class and returns a pointer to it. #[doc(hidden)] #[macro_export] macro_rules! static_lock_class { () => {{ - static CLASS: $crate::sync::LockClassKey = $crate::sync::LockClassKey::new(); + static CLASS: $crate::sync::LockClassKey = + // SAFETY: lockdep expects uninitialized memory when it's handed a statically allocated + // lock_class_key + unsafe { ::core::mem::MaybeUninit::uninit().assume_init() }; &CLASS }}; }
linux-stable-mirror@lists.linaro.org
-
Mitchell Levy -
tip-bot2 for Mitchell Levy