Skip to content

Commit a8ee30f

Browse files
Darksonnojeda
authored andcommitted
rust: sync: require T: Sync for LockedBy::access
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: 7b1f55e ("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>
1 parent ece207a commit a8ee30f

File tree

1 file changed

+14
-4
lines changed

1 file changed

+14
-4
lines changed

rust/kernel/sync/locked_by.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,12 @@ pub struct LockedBy<T: ?Sized, U: ?Sized> {
8383
// SAFETY: `LockedBy` can be transferred across thread boundaries iff the data it protects can.
8484
unsafe impl<T: ?Sized + Send, U: ?Sized> Send for LockedBy<T, U> {}
8585

86-
// SAFETY: `LockedBy` serialises the interior mutability it provides, so it is `Sync` as long as the
87-
// data it protects is `Send`.
86+
// SAFETY: If `T` is not `Sync`, then parallel shared access to this `LockedBy` allows you to use
87+
// `access_mut` to hand out `&mut T` on one thread at the time. The requirement that `T: Send` is
88+
// sufficient to allow that.
89+
//
90+
// If `T` is `Sync`, then the `access` method also becomes available, which allows you to obtain
91+
// several `&T` from several threads at once. However, this is okay as `T` is `Sync`.
8892
unsafe impl<T: ?Sized + Send, U: ?Sized> Sync for LockedBy<T, U> {}
8993

9094
impl<T, U> LockedBy<T, U> {
@@ -118,7 +122,10 @@ impl<T: ?Sized, U> LockedBy<T, U> {
118122
///
119123
/// Panics if `owner` is different from the data protected by the lock used in
120124
/// [`new`](LockedBy::new).
121-
pub fn access<'a>(&'a self, owner: &'a U) -> &'a T {
125+
pub fn access<'a>(&'a self, owner: &'a U) -> &'a T
126+
where
127+
T: Sync,
128+
{
122129
build_assert!(
123130
size_of::<U>() > 0,
124131
"`U` cannot be a ZST because `owner` wouldn't be unique"
@@ -127,7 +134,10 @@ impl<T: ?Sized, U> LockedBy<T, U> {
127134
panic!("mismatched owners");
128135
}
129136

130-
// SAFETY: `owner` is evidence that the owner is locked.
137+
// SAFETY: `owner` is evidence that there are only shared references to the owner for the
138+
// duration of 'a, so it's not possible to use `Self::access_mut` to obtain a mutable
139+
// reference to the inner value that aliases with this shared reference. The type is `Sync`
140+
// so there are no other requirements.
131141
unsafe { &*self.data.get() }
132142
}
133143

0 commit comments

Comments
 (0)