freebsd sync: fix _umtx_time flags check to use bitwise operation.#4911
freebsd sync: fix _umtx_time flags check to use bitwise operation.#4911RalfJung merged 2 commits intorust-lang:masterfrom
Conversation
|
Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two. |
|
Could you please add a PR description explaining the change? What's wrong with the old code, why is the new code more correct? |
The `_umtx_time` flags check in `read_umtx_time` used equality (`flags == abs_time`) instead of bitwise AND (`flags & abs_time != 0`) to detect `UMTX_ABSTIME`. While functionally equivalent for current valid inputs (0 or `UMTX_ABSTIME` alone), the equality check would silently treat an absolute timeout as relative if `flags` had `UMTX_ABSTIME` set alongside other bits. Additionally, unknown flags were silently accepted, whereas the FreeBSD kernel (`umtx_copyin_umtx_time()` in `kern_umtx.c`) rejects them with `EINVAL`. The fix adds validation that rejects unsupported flags and switches to the standard bitwise AND pattern used elsewhere in the codebase (e.g. `O_APPEND`/`O_TRUNC` checks in `fs.rs`).
devnexen
force-pushed
the
freebsd_abstime_flag_fix
branch
from
612b613 to
1b5a282
Compare
There was a problem hiding this comment.
That makes sense, thanks! We should really error on unsupported flags, good catch.
I have one minor comment.
@rustbot author
src/shims/unix/freebsd/sync.rs
Outdated
| if flags & !abs_time != 0 { | ||
| throw_unsup_format!("unsupported `_umtx_time` flags: {:#x}", flags); | ||
| } | ||
|
|
||
| let abs_time_flag = flags & abs_time != 0; |
There was a problem hiding this comment.
The usual way we do this is by "subtracting" what we know from the flag, and then checking for 0 at the end. This avoids the hazard of having to list all supported flags twice, like this code currently does. (open uses a mirror variable instead, but that seems unnecessary complicated. Elsewhere we "subtract".)
|
Reminder, once the PR becomes ready for a review, use |
rustbot
added
the
S-waiting-on-author
Instead of listing supported flags twice (once for the unsupported check and once for extraction), subtract known flags and check for != 0 at the end, matching the convention used elsewhere in Miri.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
rustbot
removed
the
S-waiting-on-author
3 participants
The
_umtx_timeflags check inread_umtx_timeused equality (flags == abs_time) instead of bitwise AND (flags & abs_time != 0) to detectUMTX_ABSTIME. While functionally equivalent for current valid inputs (0 orUMTX_ABSTIMEalone), the equality check would silently treat an absolute timeout as relative ifflagshadUMTX_ABSTIMEset alongside other bits. Additionally, unknown flags were silently accepted, whereas the FreeBSD kernel (umtx_copyin_umtx_time()inkern_umtx.c) rejects them withEINVAL.The fix adds validation that rejects unsupported flags and switches to the standard bitwise AND pattern used elsewhere in the codebase (e.g.
O_APPEND/O_TRUNCchecks infs.rs).