On 06.09.24 01:13, Mitchell Levy via B4 Relay 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 don't think that we can make this sound without also adding a lifetime to `Lock`. Because with only the changes you have outlined above, the key is at least valid for lifetime of the initializer, but might not be afterwards (while the lock still exists). So I think we should leave it as is now. --- Cheers, Benno
