From 1c6013acc8c1494409d030ba8ef07c018ee26b00 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 25 Oct 2023 10:23:07 -0700 Subject: [PATCH] Fix p{read,write}v{,v2}'s encoding of the offset argument on Linux. (#896) (#898) Unlike with `p{read,write}`, Linux's `p{read,write}v` syscall's offset argument is not passed in an endian-specific order. And, the expectation is for syscall wrappers to always pass both the high and low halves of the offset as separate arguments, even though on 64-bit architectures the low half is passed throgh as a 64-bit value containing the full offset and the kernel doesn't mask it. And `p{read,write}v2` follow the behavior of `p{read,write}`. --- src/backend/libc/offset.rs | 134 ++++++---------------- src/backend/linux_raw/io/syscalls.rs | 70 +++--------- src/lib.rs | 2 + tests/fs/seek.rs | 80 ++++++++++++++ tests/io/read_write.rs | 160 +++++++++++++++++++++++++++ 5 files changed, 292 insertions(+), 154 deletions(-) create mode 100644 tests/fs/seek.rs diff --git a/src/backend/libc/offset.rs b/src/backend/libc/offset.rs index 6e8ce49a2..e5cad5d13 100644 --- a/src/backend/libc/offset.rs +++ b/src/backend/libc/offset.rs @@ -232,25 +232,6 @@ pub(super) use c::{preadv64 as libc_preadv, pwritev64 as libc_pwritev}; mod readwrite_pv64 { use super::c; - // 64-bit offsets on 32-bit platforms are passed in endianness-specific - // lo/hi pairs. See src/backend/linux_raw/conv.rs for details. - #[cfg(all(target_endian = "little", target_pointer_width = "32"))] - fn lo(x: u64) -> usize { - (x >> 32) as usize - } - #[cfg(all(target_endian = "little", target_pointer_width = "32"))] - fn hi(x: u64) -> usize { - (x & 0xffff_ffff) as usize - } - #[cfg(all(target_endian = "big", target_pointer_width = "32"))] - fn lo(x: u64) -> usize { - (x & 0xffff_ffff) as usize - } - #[cfg(all(target_endian = "big", target_pointer_width = "32"))] - fn hi(x: u64) -> usize { - (x >> 32) as usize - } - pub(in super::super) unsafe fn preadv64( fd: c::c_int, iov: *const c::iovec, @@ -267,21 +248,14 @@ mod readwrite_pv64 { if let Some(fun) = preadv64.get() { fun(fd, iov, iovcnt, offset) } else { - #[cfg(target_pointer_width = "32")] - { - c::syscall( - c::SYS_preadv, - fd, - iov, - iovcnt, - hi(offset as u64), - lo(offset as u64), - ) as c::ssize_t - } - #[cfg(target_pointer_width = "64")] - { - c::syscall(c::SYS_preadv, fd, iov, iovcnt, offset) as c::ssize_t - } + c::syscall( + c::SYS_preadv, + fd, + iov, + iovcnt, + offset as usize, + (offset >> 32) as usize, + ) as c::ssize_t } } pub(in super::super) unsafe fn pwritev64( @@ -297,21 +271,14 @@ mod readwrite_pv64 { if let Some(fun) = pwritev64.get() { fun(fd, iov, iovcnt, offset) } else { - #[cfg(target_pointer_width = "32")] - { - c::syscall( - c::SYS_pwritev, - fd, - iov, - iovcnt, - hi(offset as u64), - lo(offset as u64), - ) as c::ssize_t - } - #[cfg(target_pointer_width = "64")] - { - c::syscall(c::SYS_pwritev, fd, iov, iovcnt, offset) as c::ssize_t - } + c::syscall( + c::SYS_pwritev, + fd, + iov, + iovcnt, + offset as usize, + (offset >> 32) as usize, + ) as c::ssize_t } } } @@ -347,25 +314,6 @@ pub(super) use readwrite_pv::{preadv as libc_preadv, pwritev as libc_pwritev}; mod readwrite_pv64v2 { use super::c; - // 64-bit offsets on 32-bit platforms are passed in endianness-specific - // lo/hi pairs. See src/backend/linux_raw/conv.rs for details. - #[cfg(all(target_endian = "little", target_pointer_width = "32"))] - fn lo(x: u64) -> usize { - (x >> 32) as usize - } - #[cfg(all(target_endian = "little", target_pointer_width = "32"))] - fn hi(x: u64) -> usize { - (x & 0xffff_ffff) as usize - } - #[cfg(all(target_endian = "big", target_pointer_width = "32"))] - fn lo(x: u64) -> usize { - (x & 0xffff_ffff) as usize - } - #[cfg(all(target_endian = "big", target_pointer_width = "32"))] - fn hi(x: u64) -> usize { - (x >> 32) as usize - } - pub(in super::super) unsafe fn preadv64v2( fd: c::c_int, iov: *const c::iovec, @@ -383,22 +331,15 @@ mod readwrite_pv64v2 { if let Some(fun) = preadv64v2.get() { fun(fd, iov, iovcnt, offset, flags) } else { - #[cfg(target_pointer_width = "32")] - { - c::syscall( - c::SYS_preadv, - fd, - iov, - iovcnt, - hi(offset as u64), - lo(offset as u64), - flags, - ) as c::ssize_t - } - #[cfg(target_pointer_width = "64")] - { - c::syscall(c::SYS_preadv2, fd, iov, iovcnt, offset, flags) as c::ssize_t - } + c::syscall( + c::SYS_preadv, + fd, + iov, + iovcnt, + offset as usize, + (offset >> 32) as usize, + flags, + ) as c::ssize_t } } pub(in super::super) unsafe fn pwritev64v2( @@ -415,22 +356,15 @@ mod readwrite_pv64v2 { if let Some(fun) = pwritev64v2.get() { fun(fd, iov, iovcnt, offset, flags) } else { - #[cfg(target_pointer_width = "32")] - { - c::syscall( - c::SYS_pwritev, - fd, - iov, - iovcnt, - hi(offset as u64), - lo(offset as u64), - flags, - ) as c::ssize_t - } - #[cfg(target_pointer_width = "64")] - { - c::syscall(c::SYS_pwritev2, fd, iov, iovcnt, offset, flags) as c::ssize_t - } + c::syscall( + c::SYS_pwritev, + fd, + iov, + iovcnt, + offset as usize, + (offset >> 32) as usize, + flags, + ) as c::ssize_t } } } diff --git a/src/backend/linux_raw/io/syscalls.rs b/src/backend/linux_raw/io/syscalls.rs index 2cc7898af..7fb860d25 100644 --- a/src/backend/linux_raw/io/syscalls.rs +++ b/src/backend/linux_raw/io/syscalls.rs @@ -107,25 +107,16 @@ pub(crate) fn preadv( ) -> io::Result { let (bufs_addr, bufs_len) = slice(&bufs[..cmp::min(bufs.len(), max_iov())]); - #[cfg(target_pointer_width = "32")] + // Unlike the plain "p" functions, the "pv" functions pass their offset in + // an endian-independent way, and always in two registers. unsafe { ret_usize(syscall!( __NR_preadv, fd, bufs_addr, bufs_len, - hi(pos), - lo(pos) - )) - } - #[cfg(target_pointer_width = "64")] - unsafe { - ret_usize(syscall!( - __NR_preadv, - fd, - bufs_addr, - bufs_len, - loff_t_from_u64(pos) + pass_usize(pos as usize), + pass_usize((pos >> 32) as usize) )) } } @@ -139,26 +130,16 @@ pub(crate) fn preadv2( ) -> io::Result { let (bufs_addr, bufs_len) = slice(&bufs[..cmp::min(bufs.len(), max_iov())]); - #[cfg(target_pointer_width = "32")] - unsafe { - ret_usize(syscall!( - __NR_preadv2, - fd, - bufs_addr, - bufs_len, - hi(pos), - lo(pos), - flags - )) - } - #[cfg(target_pointer_width = "64")] + // Unlike the plain "p" functions, the "pv" functions pass their offset in + // an endian-independent way, and always in two registers. unsafe { ret_usize(syscall!( __NR_preadv2, fd, bufs_addr, bufs_len, - loff_t_from_u64(pos), + pass_usize(pos as usize), + pass_usize((pos >> 32) as usize), flags )) } @@ -228,25 +209,16 @@ pub(crate) fn writev(fd: BorrowedFd<'_>, bufs: &[IoSlice<'_>]) -> io::Result, bufs: &[IoSlice<'_>], pos: u64) -> io::Result { let (bufs_addr, bufs_len) = slice(&bufs[..cmp::min(bufs.len(), max_iov())]); - #[cfg(target_pointer_width = "32")] + // Unlike the plain "p" functions, the "pv" functions pass their offset in + // an endian-independent way, and always in two registers. unsafe { ret_usize(syscall_readonly!( __NR_pwritev, fd, bufs_addr, bufs_len, - hi(pos), - lo(pos) - )) - } - #[cfg(target_pointer_width = "64")] - unsafe { - ret_usize(syscall_readonly!( - __NR_pwritev, - fd, - bufs_addr, - bufs_len, - loff_t_from_u64(pos) + pass_usize(pos as usize), + pass_usize((pos >> 32) as usize) )) } } @@ -260,26 +232,16 @@ pub(crate) fn pwritev2( ) -> io::Result { let (bufs_addr, bufs_len) = slice(&bufs[..cmp::min(bufs.len(), max_iov())]); - #[cfg(target_pointer_width = "32")] - unsafe { - ret_usize(syscall_readonly!( - __NR_pwritev2, - fd, - bufs_addr, - bufs_len, - hi(pos), - lo(pos), - flags - )) - } - #[cfg(target_pointer_width = "64")] + // Unlike the plain "p" functions, the "pv" functions pass their offset in + // an endian-independent way, and always in two registers. unsafe { ret_usize(syscall_readonly!( __NR_pwritev2, fd, bufs_addr, bufs_len, - loff_t_from_u64(pos), + pass_usize(pos as usize), + pass_usize((pos >> 32) as usize), flags )) } diff --git a/src/lib.rs b/src/lib.rs index 38cc77ef8..23104459e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -126,6 +126,8 @@ // Redox and WASI have enough differences that it isn't worth // precisely conditionallizing all the `use`s for them. #![cfg_attr(any(target_os = "redox", target_os = "wasi"), allow(unused_imports))] +// On the release branch, don't worry about unused-import warnings. +#![allow(unused_imports)] #[cfg(not(feature = "rustc-dep-of-std"))] extern crate alloc; diff --git a/tests/fs/seek.rs b/tests/fs/seek.rs new file mode 100644 index 000000000..701d22a4e --- /dev/null +++ b/tests/fs/seek.rs @@ -0,0 +1,80 @@ +/// Test seek positions related to file "holes". +#[cfg(any(apple, freebsdlike, linux_kernel, solarish))] +#[test] +fn test_seek_holes() { + use rustix::fs::{fstat, openat, seek, Mode, OFlags, SeekFrom, CWD}; + use std::io::Write; + + let tmp = tempfile::tempdir().unwrap(); + let dir = openat(CWD, tmp.path(), OFlags::RDONLY, Mode::empty()).unwrap(); + let foo = openat( + &dir, + "foo", + OFlags::RDWR | OFlags::CREATE | OFlags::TRUNC, + Mode::RUSR | Mode::WUSR, + ) + .unwrap(); + let mut foo = std::fs::File::from(foo); + + let stat = fstat(&foo).unwrap(); + let hole_size = stat.st_blksize as u64; + + #[cfg(any(solarish, freebsdlike, netbsdlike))] + let hole_size = unsafe { + use std::os::unix::io::AsRawFd; + + let r = libc::fpathconf(foo.as_raw_fd(), libc::_PC_MIN_HOLE_SIZE); + + if r < 0 { + // Holes not supported. + return; + } + + // Holes are supported. + core::cmp::max(hole_size, r as u64) + }; + + foo.write_all(b"prefix").unwrap(); + assert_eq!( + seek(&foo, SeekFrom::Start(hole_size * 2)), + Ok(hole_size * 2) + ); + foo.write_all(b"suffix").unwrap(); + assert_eq!(seek(&foo, SeekFrom::Start(0)), Ok(0)); + assert_eq!(seek(&foo, SeekFrom::Current(0)), Ok(0)); + assert_eq!(seek(&foo, SeekFrom::Hole(0)), Ok(hole_size)); + assert_eq!(seek(&foo, SeekFrom::Hole(hole_size as i64)), Ok(hole_size)); + assert_eq!( + seek(&foo, SeekFrom::Hole(hole_size as i64 * 2)), + Ok(hole_size * 2 + 6) + ); + assert_eq!(seek(&foo, SeekFrom::Data(0)), Ok(0)); + assert_eq!( + seek(&foo, SeekFrom::Data(hole_size as i64)), + Ok(hole_size * 2) + ); + assert_eq!( + seek(&foo, SeekFrom::Data(hole_size as i64 * 2)), + Ok(hole_size * 2) + ); +} + +#[test] +fn test_seek_offsets() { + use rustix::fs::{openat, seek, Mode, OFlags, SeekFrom, CWD}; + + let f = openat(CWD, "Cargo.toml", OFlags::RDONLY, Mode::empty()).unwrap(); + + match seek(&f, SeekFrom::Start(0)) { + Ok(_) => {} + Err(e) => panic!("seek failed with an unexpected error: {:?}", e), + } + for invalid_offset in &[i32::MIN as u64, !1 as u64, i64::MIN as u64] { + let invalid_offset = *invalid_offset; + match seek(&f, SeekFrom::Start(invalid_offset)) { + Err(rustix::io::Errno::INVAL) => {} + Ok(_) => panic!("seek unexpectedly succeeded"), + Err(e) => panic!("seek failed with an unexpected error: {:?}", e), + } + } +} diff --git a/tests/io/read_write.rs b/tests/io/read_write.rs index 45f6dcbc7..fe8c64ebe 100644 --- a/tests/io/read_write.rs +++ b/tests/io/read_write.rs @@ -198,3 +198,163 @@ fn test_pwritev2() { .unwrap(); assert_eq!(&buf, b"world"); } + +#[cfg(linux_kernel)] +#[cfg(all(feature = "net", feature = "pipe"))] +#[test] +fn test_preadv2_nowait() { + use rustix::io::{preadv2, ReadWriteFlags}; + use rustix::net::{socketpair, AddressFamily, SocketFlags, SocketType}; + use rustix::pipe::pipe; + + let mut buf = [0_u8; 5]; + + let (reader, _writer) = socketpair( + AddressFamily::UNIX, + SocketType::STREAM, + SocketFlags::CLOEXEC, + None, + ) + .unwrap(); + match preadv2( + &reader, + &mut [IoSliceMut::new(&mut buf)], + u64::MAX, + ReadWriteFlags::NOWAIT, + ) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Err(rustix::io::Errno::AGAIN) => {} + Ok(_) => panic!("preadv2 unexpectedly succeeded"), + Err(e) => panic!("preadv2 failed with an unexpected error: {:?}", e), + } + + let (reader, _writer) = pipe().unwrap(); + match preadv2( + &reader, + &mut [IoSliceMut::new(&mut buf)], + u64::MAX, + ReadWriteFlags::NOWAIT, + ) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Err(rustix::io::Errno::AGAIN) => {} + Ok(_) => panic!("preadv2 unexpectedly succeeded"), + Err(e) => panic!("preadv2 failed with an unexpected error: {:?}", e), + } +} + +#[cfg(feature = "net")] +#[cfg(not(target_os = "espidf"))] // no preadv/pwritev +#[cfg(not(target_os = "solaris"))] // no preadv/pwritev +#[cfg(not(target_os = "haiku"))] // no preadv/pwritev +#[test] +fn test_p_offsets() { + use rustix::fs::{cwd, openat, Mode, OFlags}; + use rustix::io::{pread, preadv, pwrite, pwritev}; + #[cfg(linux_kernel)] + use rustix::io::{preadv2, pwritev2, ReadWriteFlags}; + + let mut buf = [0_u8; 5]; + + let tmp = tempfile::tempdir().unwrap(); + let f = openat( + cwd(), + tmp.path().join("file"), + OFlags::RDWR | OFlags::CREATE | OFlags::TRUNC, + Mode::RUSR | Mode::WUSR, + ) + .unwrap(); + + // Test that offset 0 works. + match pread(&f, &mut buf, 0_u64) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Ok(_) => {} + Err(e) => panic!("pread failed with an unexpected error: {:?}", e), + } + match pwrite(&f, &buf, 0_u64) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Ok(_) => {} + Err(e) => panic!("pwrite failed with an unexpected error: {:?}", e), + } + match preadv(&f, &mut [IoSliceMut::new(&mut buf)], 0_u64) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Ok(_) => {} + Err(e) => panic!("preadv failed with an unexpected error: {:?}", e), + } + match pwritev(&f, &[IoSlice::new(&buf)], 0_u64) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Ok(_) => {} + Err(e) => panic!("pwritev failed with an unexpected error: {:?}", e), + } + #[cfg(linux_kernel)] + { + match preadv2( + &f, + &mut [IoSliceMut::new(&mut buf)], + 0_u64, + ReadWriteFlags::empty(), + ) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Ok(_) => {} + Err(e) => panic!("preadv2 failed with an unexpected error: {:?}", e), + } + match pwritev2(&f, &[IoSlice::new(&buf)], 0_u64, ReadWriteFlags::empty()) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Ok(_) => {} + Err(e) => panic!("pwritev2 failed with an unexpected error: {:?}", e), + } + } + + // Test that negative offsets fail with `INVAL`. + for invalid_offset in &[i32::MIN as u64, !1 as u64, i64::MIN as u64] { + let invalid_offset = *invalid_offset; + match pread(&f, &mut buf, invalid_offset) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Err(rustix::io::Errno::INVAL) => {} + Ok(_) => panic!("pread unexpectedly succeeded"), + Err(e) => panic!("pread failed with an unexpected error: {:?}", e), + } + match pwrite(&f, &buf, invalid_offset) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Err(rustix::io::Errno::INVAL) => {} + Ok(_) => panic!("pwrite unexpectedly succeeded"), + Err(e) => panic!("pwrite failed with an unexpected error: {:?}", e), + } + match preadv(&f, &mut [IoSliceMut::new(&mut buf)], invalid_offset) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Err(rustix::io::Errno::INVAL) => {} + Ok(_) => panic!("preadv unexpectedly succeeded"), + Err(e) => panic!("preadv failed with an unexpected error: {:?}", e), + } + match pwritev(&f, &[IoSlice::new(&buf)], invalid_offset) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Err(rustix::io::Errno::INVAL) => {} + Ok(_) => panic!("pwritev unexpectedly succeeded"), + Err(e) => panic!("pwritev failed with an unexpected error: {:?}", e), + } + #[cfg(linux_kernel)] + { + match preadv2( + &f, + &mut [IoSliceMut::new(&mut buf)], + invalid_offset, + ReadWriteFlags::empty(), + ) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Err(rustix::io::Errno::INVAL) => {} + Ok(_) => panic!("preadv2 unexpectedly succeeded"), + Err(e) => panic!("preadv2 failed with an unexpected error: {:?}", e), + } + match pwritev2( + &f, + &[IoSlice::new(&buf)], + invalid_offset, + ReadWriteFlags::empty(), + ) { + Err(rustix::io::Errno::OPNOTSUPP) | Err(rustix::io::Errno::NOSYS) => {} + Err(rustix::io::Errno::INVAL) => {} + Ok(_) => panic!("pwritev2 unexpectedly succeeded"), + Err(e) => panic!("pwritev2 failed with an unexpected error: {:?}", e), + } + } + } +}