Re: [PATCH RFC] rust: lockdep: Use Pin for all LockClassKey usages

[Alice Ryhl]

On Fri, Sep 6, 2024 at 1:13 AM Mitchell Levy via B4 Relay
<devnull+levymitchell0.gmail.com@xxxxxxxxxx> wrote:
>
> From: Mitchell Levy <levymitchell0@xxxxxxxxx>
>
> The current LockClassKey API has soundness issues related to the use of
> dynamically allocated LockClassKeys. In particular, these keys can be
> used without being registered and don't have address stability.
>
> This fixes the issue by using Pin<&LockClassKey> and properly
> registering/deregistering the keys on init/drop.
>
> Link: https://lore.kernel.org/rust-for-linux/20240815074519.2684107-1-nmi@xxxxxxxxxxxx/
> Suggested-by: Benno Lossin <benno.lossin@xxxxxxxxx>
> Suggested-by: Boqun Feng <boqun.feng@xxxxxxxxx>
> Signed-off-by: Mitchell Levy <levymitchell0@xxxxxxxxx>
> ---
> This change is based on applying the linked patch to the top of
> rust-next.
>
> I'm sending this as an RFC because I'm not sure that using
> Pin<&'static LockClassKey> is appropriate as the parameter for, e.g.,
> Work::new. This should preclude using dynamically allocated
> LockClassKeys here, which might not be desirable. Unfortunately, using
> Pin<&'a LockClassKey> creates other headaches as the compiler then
> requires that T and PinImpl<Self> be bounded by 'a, which also seems
> undesirable. I would be especially interested in feedback/ideas along
> these lines.

I think what should happen here is split this into two commits:

1. Get rid of LockClassKey::new so that the only constructor is the
`static_lock_class!` macro. Backport this change to stable kernels.
2. Everything else you have as-is.

Everything that takes a lock class key right now takes it by &'static
so they technically don't need to be wrapped in Pin (see
Pin::static_ref), but I don't mind making this change to pave the way
for LockClassKeys that don't live forever in the future.

The patch *does* introduce the ability to create LockClassKeys, but
they're only usable if they are leaked.

Alice

> -        T: WorkItem<ID>,
> +        T: WorkItem<ID>

Spurious change?