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?
