Re: [PATCH v2 0/2] rust: lockdep: Fix soundness issue affecting LockClassKeys

[Boqun Feng <boqun.feng@gmail.com>]

On Thu, Dec 19, 2024 at 12:58:54PM -0800, Mitchell Levy wrote:
> 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.
> 

Thanks for doing this! I found some clippy warnings with the current
version, but overall it looks good to me. That said, appreciate it if
patch #2 gets more reviews on the interface changes, thanks!

Regards,
Boqun

> ---
> 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
> 
> ---
> 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
> 
> ---
> 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        |  3 +-
>  8 files changed, 78 insertions(+), 23 deletions(-)
> ---
> base-commit: 0c5928deada15a8d075516e6e0d9ee19011bb000
> change-id: 20240905-rust-lockdep-d3e30521c8ba
> 
> Best regards,
> -- 
> Mitchell Levy <levymitchell0@gmail.com>
>