From 2b2a3a0cc11c40a8fd6d8015d0b7fb11b30d8156 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 3 Oct 2020 16:01:53 +0200 Subject: [PATCH] check that all syscall arguments are scalars --- src/bin/miri.rs | 2 +- src/shims/posix/linux/foreign_items.rs | 32 ++++++++++++++------------ src/shims/posix/linux/sync.rs | 10 ++++---- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/bin/miri.rs b/src/bin/miri.rs index e3317d0d4d6..7b417990af8 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -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!( diff --git a/src/shims/posix/linux/foreign_items.rs b/src/shims/posix/linux/foreign_items.rs index 364cfde6c07..04efa79b9d9 100644 --- a/src/shims/posix/linux/foreign_items.rs +++ b/src/shims/posix/linux/foreign_items.rs @@ -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")? @@ -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). 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), } } diff --git a/src/shims/posix/linux/sync.rs b/src/shims/posix/linux/sync.rs index ef172fa2a67..9d124872f5c 100644 --- a/src/shims/posix/linux/sync.rs +++ b/src/shims/posix/linux/sync.rs @@ -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: @@ -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 => { @@ -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(())