rust: sync: require T: Sync
for LockedBy::access
commit a8ee30f45d5d57467ddb7877ed6914d0eba0af7f upstream.
The `LockedBy::access` method only requires a shared reference to the
owner, so if we have shared access to the `LockedBy` from several
threads at once, then two threads could call `access` in parallel and
both obtain a shared reference to the inner value. Thus, require that
`T: Sync` when calling the `access` method.
An alternative is to require `T: Sync` in the `impl Sync for LockedBy`.
This patch does not choose that approach as it gives up the ability to
use `LockedBy` with `!Sync` types, which is okay as long as you only use
`access_mut`.
Cc: stable@vger.kernel.org
Fixes: 7b1f55e3a9
("rust: sync: introduce `LockedBy`")
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Suggested-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/r/20240915-locked-by-sync-fix-v2-1-1a8d89710392@google.com
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
406c275efb
commit
a2fd4042ae
1 changed files with 14 additions and 4 deletions
|
@ -83,8 +83,12 @@ pub struct LockedBy<T: ?Sized, U: ?Sized> {
|
|||
// SAFETY: `LockedBy` can be transferred across thread boundaries iff the data it protects can.
|
||||
unsafe impl<T: ?Sized + Send, U: ?Sized> Send for LockedBy<T, U> {}
|
||||
|
||||
// SAFETY: `LockedBy` serialises the interior mutability it provides, so it is `Sync` as long as the
|
||||
// data it protects is `Send`.
|
||||
// SAFETY: If `T` is not `Sync`, then parallel shared access to this `LockedBy` allows you to use
|
||||
// `access_mut` to hand out `&mut T` on one thread at the time. The requirement that `T: Send` is
|
||||
// sufficient to allow that.
|
||||
//
|
||||
// If `T` is `Sync`, then the `access` method also becomes available, which allows you to obtain
|
||||
// several `&T` from several threads at once. However, this is okay as `T` is `Sync`.
|
||||
unsafe impl<T: ?Sized + Send, U: ?Sized> Sync for LockedBy<T, U> {}
|
||||
|
||||
impl<T, U> LockedBy<T, U> {
|
||||
|
@ -118,7 +122,10 @@ impl<T: ?Sized, U> LockedBy<T, U> {
|
|||
///
|
||||
/// Panics if `owner` is different from the data protected by the lock used in
|
||||
/// [`new`](LockedBy::new).
|
||||
pub fn access<'a>(&'a self, owner: &'a U) -> &'a T {
|
||||
pub fn access<'a>(&'a self, owner: &'a U) -> &'a T
|
||||
where
|
||||
T: Sync,
|
||||
{
|
||||
build_assert!(
|
||||
size_of::<U>() > 0,
|
||||
"`U` cannot be a ZST because `owner` wouldn't be unique"
|
||||
|
@ -127,7 +134,10 @@ pub fn access<'a>(&'a self, owner: &'a U) -> &'a T {
|
|||
panic!("mismatched owners");
|
||||
}
|
||||
|
||||
// SAFETY: `owner` is evidence that the owner is locked.
|
||||
// SAFETY: `owner` is evidence that there are only shared references to the owner for the
|
||||
// duration of 'a, so it's not possible to use `Self::access_mut` to obtain a mutable
|
||||
// reference to the inner value that aliases with this shared reference. The type is `Sync`
|
||||
// so there are no other requirements.
|
||||
unsafe { &*self.data.get() }
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue