Skip to content

Commit 6364ccd

Browse files
committed
Adds detection of mixed usage of FUTEX_WAIT and FUTEX_WAKE
1 parent 6d4e664 commit 6364ccd

File tree

2 files changed

+64
-19
lines changed
  • library/std/src/sys/pal/unix
  • src/tools/miri/src/shims/unix/linux

2 files changed

+64
-19
lines changed

library/std/src/sys/pal/unix/futex.rs

+19-11
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option<Duration>) -
2626
use super::time::Timespec;
2727
use crate::ptr::null;
2828
use crate::sync::atomic::Ordering::Relaxed;
29+
use crate::sys::cvt;
2930

3031
// Calculate the timeout as an absolute timespec.
3132
//
@@ -40,7 +41,7 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option<Duration>) -
4041
return true;
4142
}
4243

43-
let r = unsafe {
44+
let r = cvt(unsafe {
4445
cfg_if::cfg_if! {
4546
if #[cfg(target_os = "freebsd")] {
4647
// FreeBSD doesn't have futex(), but it has
@@ -77,12 +78,16 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option<Duration>) -
7778
compile_error!("unknown target_os");
7879
}
7980
}
80-
};
81-
82-
match (r < 0).then(super::os::errno) {
83-
Some(libc::ETIMEDOUT) => return false,
84-
Some(libc::EINTR) => continue,
85-
_ => return true,
81+
});
82+
83+
match r {
84+
Ok(_) => return true,
85+
Err(e) => match e.raw_os_error() {
86+
Some(libc::ETIMEDOUT) => return false,
87+
Some(libc::EINTR) => continue,
88+
Some(libc::EAGAIN) => return true,
89+
_ => panic!("failed to wait on futex: {e:?}"),
90+
},
8691
}
8792
}
8893
}
@@ -95,19 +100,22 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option<Duration>) -
95100
/// On some platforms, this always returns false.
96101
#[cfg(any(target_os = "linux", target_os = "android"))]
97102
pub fn futex_wake(futex: &AtomicU32) -> bool {
103+
use crate::sys::cvt;
98104
let ptr = futex as *const AtomicU32;
99105
let op = libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG;
100-
unsafe { libc::syscall(libc::SYS_futex, ptr, op, 1) > 0 }
106+
cvt(unsafe { libc::syscall(libc::SYS_futex, ptr, op, 1) })
107+
.expect("failed to wake futex waiters")
108+
> 0
101109
}
102110

103111
/// Wakes up all threads that are waiting on `futex_wait` on this futex.
104112
#[cfg(any(target_os = "linux", target_os = "android"))]
105113
pub fn futex_wake_all(futex: &AtomicU32) {
114+
use crate::sys::cvt;
106115
let ptr = futex as *const AtomicU32;
107116
let op = libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG;
108-
unsafe {
109-
libc::syscall(libc::SYS_futex, ptr, op, i32::MAX);
110-
}
117+
cvt(unsafe { libc::syscall(libc::SYS_futex, ptr, op, i32::MAX) })
118+
.expect("failed to wake futex waiters");
111119
}
112120

113121
// FreeBSD doesn't tell us how many threads are woken up, so this always returns false.

src/tools/miri/src/shims/unix/linux/sync.rs

+45-8
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,13 @@ pub fn futex<'tcx>(
187187
// It's not uncommon for `addr` to be passed as another type than `*mut i32`, such as `*const AtomicI32`.
188188
let futex_val = this.read_scalar_atomic(&addr, AtomicReadOrd::Relaxed)?.to_i32()?;
189189
if val == futex_val {
190+
// Check that the top waiter (if exists) is waiting using FUTEX_WAIT_*.
191+
if this.futex_waiter_count(addr_usize) != 0 && this.futex_top_waiter_extra(addr_usize).is_some() {
192+
this.set_last_error(LibcError("EINVAL"))?;
193+
this.write_scalar(Scalar::from_target_isize(-1, this), dest)?;
194+
return interp_ok(());
195+
}
196+
190197
// The value still matches, so we block the thread and make it wait for FUTEX_WAKE.
191198
this.futex_wait(
192199
addr_usize,
@@ -245,6 +252,16 @@ pub fn futex<'tcx>(
245252
// before doing the syscall.
246253
this.atomic_fence(AtomicFenceOrd::SeqCst)?;
247254
let mut n = 0;
255+
256+
// Check that the waiter is waiting using FUTEX_WAIT_*.
257+
if this.futex_waiter_count(addr_usize) != 0 {
258+
if this.futex_top_waiter_extra(addr_usize).is_some() {
259+
this.set_last_error(LibcError("EINVAL"))?;
260+
this.write_scalar(Scalar::from_target_isize(-1, this), dest)?;
261+
return interp_ok(());
262+
}
263+
}
264+
248265
#[allow(clippy::arithmetic_side_effects)]
249266
for _ in 0..val {
250267
if this.futex_wake(addr_usize, bitset)? {
@@ -280,6 +297,15 @@ pub fn futex<'tcx>(
280297

281298
if futex_val == 0 {
282299
// 0 means unlocked - then lock it.
300+
301+
// Check that there is no waiters.
302+
if this.futex_waiter_count(addr_usize) != 0 {
303+
this.set_last_error(LibcError("EINVAL"))?;
304+
this.write_scalar(Scalar::from_target_isize(-1, this), dest)?;
305+
return interp_ok(());
306+
}
307+
308+
// Declare ownership.
283309
this.write_scalar_atomic(Scalar::from_u32(tid), &addr, AtomicWriteOrd::Relaxed)?;
284310

285311
// This ensures all loads afterwards get updated value of *addr.
@@ -293,15 +319,26 @@ pub fn futex<'tcx>(
293319
this.write_scalar(Scalar::from_target_isize(-1, this), dest)?;
294320
} else {
295321
// Other values mean locked.
296-
// Mark the futex as contended.
297-
this.write_scalar_atomic(
298-
Scalar::from_u32(futex_val | futex_waiters),
299-
&addr,
300-
AtomicWriteOrd::Relaxed,
301-
)?;
302322

303-
// This ensures all loads afterwards get updated value of *addr.
304-
this.atomic_fence(AtomicFenceOrd::SeqCst)?;
323+
if this.futex_waiter_count(addr_usize) == 0 {
324+
// Mark the futex as contended.
325+
this.write_scalar_atomic(
326+
Scalar::from_u32(futex_val | futex_waiters),
327+
&addr,
328+
AtomicWriteOrd::Relaxed,
329+
)?;
330+
331+
// This ensures all loads afterwards get updated value of *addr.
332+
this.atomic_fence(AtomicFenceOrd::SeqCst)?;
333+
} else {
334+
// Check that the futex has been marked as contended.
335+
// Check that the top waiter is waiting using FUTEX_LOCK_PI.
336+
if (futex_val & futex_waiters) == 0 || this.futex_top_waiter_extra(addr_usize).is_none() {
337+
this.set_last_error(LibcError("EINVAL"))?;
338+
this.write_scalar(Scalar::from_target_isize(-1, this), dest)?;
339+
return interp_ok(());
340+
}
341+
}
305342

306343
// Put ourselves into the wait queue.
307344
this.futex_wait(

0 commit comments

Comments
 (0)