Skip to content
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

Trying to SeekFrom::Start past i64::MAX ICEs Miri #3680

Closed
saethlin opened this issue Jun 16, 2024 · 0 comments · Fixed by #3689
Closed

Trying to SeekFrom::Start past i64::MAX ICEs Miri #3680

saethlin opened this issue Jun 16, 2024 · 0 comments · Fixed by #3689
Labels
A-shims Area: This affects the external function shims C-bug Category: This is a bug. E-good-first-issue A good way to start contributing, mentoring is available I-ICE Impact: makes Miri crash with some ICE

Comments

@saethlin
Copy link
Member

saethlin commented Jun 16, 2024

This program (reduced from an ICE encountered when running the tests for https://crates.io/crates/tempfile):

use std::io::Seek;

fn main() {
    let mut f = std::fs::File::create("test").unwrap();
    f.seek(std::io::SeekFrom::Start(i64::MAX as u64 + 1)).unwrap();
}

will ICE with:

thread 'rustc' panicked at src/tools/miri/src/shims/unix/fs.rs:398:51:
called `Result::unwrap()` on an `Err` value: TryFromIntError(())

The problem is the standard library casts our u64 SeekFrom to an i64 (off64_t is i64):
https://github.com/rust-lang/rust/blob/55cac26a9ef17da1c9c77c0816e88e178b7cc5dd/library/std/src/sys/pal/unix/fs.rs#L1296-L1302

            // Casting to `i64` is fine, too large values will end up as
            // negative which will cause an error in `lseek64`.
            SeekFrom::Start(off) => (libc::SEEK_SET, off as i64),
            SeekFrom::End(off) => (libc::SEEK_END, off),
            SeekFrom::Current(off) => (libc::SEEK_CUR, off),
        };
        let n = cvt(unsafe { lseek64(self.as_raw_fd(), pos as off64_t, whence) })?;

But we just convert the i64 to an i128 (with into):

let offset = this.read_scalar(offset)?.to_i64()?;
let whence = this.read_scalar(whence)?.to_i32()?;
let result = this.lseek64(fd, offset.into(), whence)?;

Then we try to convert the i128 numerically into a u64:

miri/src/shims/unix/fs.rs

Lines 392 to 398 in 60a7200

fn lseek64(&mut self, fd: i32, offset: i128, whence: i32) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut();
// Isolation check is done via `FileDescriptor` trait.
let seek_from = if whence == this.eval_libc_i32("SEEK_SET") {
SeekFrom::Start(u64::try_from(offset).unwrap())

... even though the standard library expects us to just return an error for a negative seek offset. We should do that.

@saethlin saethlin added C-bug Category: This is a bug. I-ICE Impact: makes Miri crash with some ICE labels Jun 16, 2024
@RalfJung RalfJung added A-shims Area: This affects the external function shims E-good-first-issue A good way to start contributing, mentoring is available labels Jun 16, 2024
@bors bors closed this as completed in a784b63 Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-shims Area: This affects the external function shims C-bug Category: This is a bug. E-good-first-issue A good way to start contributing, mentoring is available I-ICE Impact: makes Miri crash with some ICE
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants