Skip to content

check that all syscall arguments are scalars #1570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ fn main() {
),
FromHexError::OddLength =>
panic!("-Zmiri-seed should have an even number of digits"),
err => panic!("Unknown error decoding -Zmiri-seed as hex: {:?}", err),
err => panic!("unknown error decoding -Zmiri-seed as hex: {:?}", err),
});
if seed_raw.len() > 8 {
panic!(format!(
Expand Down
32 changes: 17 additions & 15 deletions src/shims/posix/linux/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,16 +113,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx

// Dynamically invoked syscalls
"syscall" => {
// FIXME: The libc syscall() function is a variadic function.
// It's valid to call it with more arguments than a syscall
// needs, so none of these syscalls should use check_arg_count.
// It's even valid to call it with the wrong type of arguments,
// as long as they'd end up in the same place with the calling
// convention used. (E.g. using a `usize` instead of a pointer.)
// It's not directly clear which number, size, and type of arguments
// are acceptable in which cases and which aren't. (E.g. some
// types might take up the space of two registers.)
// So this needs to be researched first.
// The syscall variadic function is legal to call with more arguments than needed,
// extra arguments are simply ignored. However, all arguments need to be scalars;
// other types might be treated differently by the calling convention.
for arg in args {
if !matches!(arg.layout.abi, rustc_target::abi::Abi::Scalar(_)) {
throw_ub_format!("`syscall` arguments must all have scalar layout, but {} does not", arg.layout.ty);
}
}

let sys_getrandom = this
.eval_libc("SYS_getrandom")?
Expand All @@ -144,22 +142,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// is called if a `HashMap` is created the regular way (e.g. HashMap<K, V>).
id if id == sys_getrandom => {
// The first argument is the syscall id, so skip over it.
let &[_, ptr, len, flags] = check_arg_count(args)?;
getrandom(this, ptr, len, flags, dest)?;
if args.len() < 4 {
throw_ub_format!("incorrect number of arguments for `getrandom` syscall: got {}, expected at least 4", args.len());
}
getrandom(this, args[1], args[2], args[3], dest)?;
}
// `statx` is used by `libstd` to retrieve metadata information on `linux`
// instead of using `stat`,`lstat` or `fstat` as on `macos`.
id if id == sys_statx => {
// The first argument is the syscall id, so skip over it.
let &[_, dirfd, pathname, flags, mask, statxbuf] = check_arg_count(args)?;
let result = this.linux_statx(dirfd, pathname, flags, mask, statxbuf)?;
if args.len() < 6 {
throw_ub_format!("incorrect number of arguments for `statx` syscall: got {}, expected at least 6", args.len());
}
let result = this.linux_statx(args[1], args[2], args[3], args[4], args[5])?;
this.write_scalar(Scalar::from_machine_isize(result.into(), this), dest)?;
}
// `futex` is used by some synchonization primitives.
id if id == sys_futex => {
futex(this, args, dest)?;
}
id => throw_unsup_format!("miri does not support syscall ID {}", id),
id => throw_unsup_format!("Miri does not support syscall ID {}", id),
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/shims/posix/linux/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ pub fn futex<'tcx>(
// may or may not be left out from the `syscall()` call.
// Therefore we don't use `check_arg_count` here, but only check for the
// number of arguments to fall within a range.
if !(4..=7).contains(&args.len()) {
throw_ub_format!("incorrect number of arguments for futex syscall: got {}, expected between 4 and 7 (inclusive)", args.len());
if args.len() < 4 {
throw_ub_format!("incorrect number of arguments for `futex` syscall: got {}, expected at least 4", args.len());
}

// The first three arguments (after the syscall number itself) are the same to all futex operations:
Expand Down Expand Up @@ -49,13 +49,13 @@ pub fn futex<'tcx>(
// or *timeout expires. `timeout == null` for an infinite timeout.
op if op & !futex_realtime == futex_wait => {
if args.len() < 5 {
throw_ub_format!("incorrect number of arguments for FUTEX_WAIT syscall: got {}, expected at least 5", args.len());
throw_ub_format!("incorrect number of arguments for `futex` syscall with `op=FUTEX_WAIT`: got {}, expected at least 5", args.len());
}
let timeout = args[4];
let timeout_time = if this.is_null(this.read_scalar(timeout)?.check_init()?)? {
None
} else {
this.check_no_isolation("`syscall(SYS_FUTEX, op=FUTEX_WAIT)` with timeout")?;
this.check_no_isolation("`futex` syscall with `op=FUTEX_WAIT` and non-null timeout")?;
let duration = match this.read_timespec(timeout)? {
Some(duration) => duration,
None => {
Expand Down Expand Up @@ -126,7 +126,7 @@ pub fn futex<'tcx>(
}
this.write_scalar(Scalar::from_machine_isize(n, this), dest)?;
}
op => throw_unsup_format!("miri does not support SYS_futex operation {}", op),
op => throw_unsup_format!("Miri does not support `futex` syscall with op={}", op),
}

Ok(())
Expand Down