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 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/lib.rs | 2 +- rust/kernel/sync.rs | 34 ++++++++++++++++++++++++---------- rust/kernel/sync/condvar.rs | 11 +++++++---- rust/kernel/sync/lock.rs | 4 ++-- rust/kernel/workqueue.rs | 2 +- 7 files changed, 49 insertions(+), 18 deletions(-) --- base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc change-id: 20240905-rust-lockdep-d3e30521c8ba
Best regards,
From: Mitchell Levy levymitchell0@gmail.com
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 Signed-off-by: Mitchell Levy levymitchell0@gmail.com --- rust/kernel/lib.rs | 2 +- rust/kernel/sync.rs | 14 ++------------ 2 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 22a3bfa5a9e9..b5f4b3ce6b48 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -44,8 +44,8 @@ pub mod page; pub mod prelude; pub mod print; -pub mod sizes; pub mod rbtree; +pub mod sizes; mod static_assert; #[doc(hidden)] pub mod std_vendor; diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs index 0ab20975a3b5..d270db9b9894 100644 --- a/rust/kernel/sync.rs +++ b/rust/kernel/sync.rs @@ -27,28 +27,18 @@ 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 = + unsafe { ::core::mem::MaybeUninit::uninit().assume_init() }; &CLASS }}; }
Am 05.10.24 um 00:01 schrieb Mitchell Levy via B4 Relay:
From: Mitchell Levy levymitchell0@gmail.com
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 Signed-off-by: Mitchell Levy levymitchell0@gmail.com
rust/kernel/lib.rs | 2 +- rust/kernel/sync.rs | 14 ++------------ 2 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 22a3bfa5a9e9..b5f4b3ce6b48 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -44,8 +44,8 @@ pub mod page; pub mod prelude; pub mod print; -pub mod sizes; pub mod rbtree; +pub mod sizes; mod static_assert; #[doc(hidden)] pub mod std_vendor;
This is fixed already
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/ru...
and can be dropped here.
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs index 0ab20975a3b5..d270db9b9894 100644 --- a/rust/kernel/sync.rs +++ b/rust/kernel/sync.rs @@ -27,28 +27,18 @@ 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();
Should the SAFETY comment added in the 2nd patch go to here?
+ // SAFETY: lockdep expects uninitialized memory when it's handed a statically allocated + // lock_class_key
static CLASS: $crate::sync::LockClassKey =unsafe { ::core::mem::MaybeUninit::uninit().assume_init() }; &CLASS }};
linux-stable-mirror@lists.linaro.org
-
Dirk Behme -
Mitchell Levy via B4 Relay