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

feat: I/O safety for 'sys/uid' & 'sched' #1931

Merged
merged 1 commit into from
Dec 9, 2022
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
6 changes: 3 additions & 3 deletions src/sched.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ mod sched_linux_like {
use libc::{self, c_int, c_void};
use std::mem;
use std::option::Option;
use std::os::unix::io::RawFd;
use std::os::unix::io::{AsFd, AsRawFd};

// For some functions taking with a parameter of type CloneFlags,
// only a subset of these flags have an effect.
Expand Down Expand Up @@ -136,8 +136,8 @@ mod sched_linux_like {
/// reassociate thread with a namespace
///
/// See also [setns(2)](https://man7.org/linux/man-pages/man2/setns.2.html)
pub fn setns(fd: RawFd, nstype: CloneFlags) -> Result<()> {
let res = unsafe { libc::setns(fd, nstype.bits()) };
pub fn setns<Fd: AsFd>(fd: Fd, nstype: CloneFlags) -> Result<()> {
let res = unsafe { libc::setns(fd.as_fd().as_raw_fd(), nstype.bits()) };

Errno::result(res).map(drop)
}
Expand Down
28 changes: 14 additions & 14 deletions src/sys/uio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ use crate::errno::Errno;
use crate::Result;
use libc::{self, c_int, c_void, off_t, size_t};
use std::io::{IoSlice, IoSliceMut};
use std::os::unix::io::RawFd;
use std::os::unix::io::{AsFd, AsRawFd};

/// Low-level vectored write to a raw file descriptor
///
/// See also [writev(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/writev.html)
pub fn writev(fd: RawFd, iov: &[IoSlice<'_>]) -> Result<usize> {
pub fn writev<Fd: AsFd>(fd: Fd, iov: &[IoSlice<'_>]) -> Result<usize> {
// SAFETY: to quote the documentation for `IoSlice`:
//
// [IoSlice] is semantically a wrapper around a &[u8], but is
Expand All @@ -18,7 +18,7 @@ pub fn writev(fd: RawFd, iov: &[IoSlice<'_>]) -> Result<usize> {
//
// Because it is ABI compatible, a pointer cast here is valid
let res = unsafe {
libc::writev(fd, iov.as_ptr() as *const libc::iovec, iov.len() as c_int)
libc::writev(fd.as_fd().as_raw_fd(), iov.as_ptr() as *const libc::iovec, iov.len() as c_int)
};

Errno::result(res).map(|r| r as usize)
Expand All @@ -27,10 +27,10 @@ pub fn writev(fd: RawFd, iov: &[IoSlice<'_>]) -> Result<usize> {
/// Low-level vectored read from a raw file descriptor
///
/// See also [readv(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/readv.html)
pub fn readv(fd: RawFd, iov: &mut [IoSliceMut<'_>]) -> Result<usize> {
pub fn readv<Fd: AsFd>(fd: Fd, iov: &mut [IoSliceMut<'_>]) -> Result<usize> {
// SAFETY: same as in writev(), IoSliceMut is ABI-compatible with iovec
let res = unsafe {
libc::readv(fd, iov.as_ptr() as *const libc::iovec, iov.len() as c_int)
libc::readv(fd.as_fd().as_raw_fd(), iov.as_ptr() as *const libc::iovec, iov.len() as c_int)
};

Errno::result(res).map(|r| r as usize)
Expand All @@ -44,14 +44,14 @@ pub fn readv(fd: RawFd, iov: &mut [IoSliceMut<'_>]) -> Result<usize> {
/// See also: [`writev`](fn.writev.html) and [`pwrite`](fn.pwrite.html)
#[cfg(not(any(target_os = "redox", target_os = "haiku")))]
#[cfg_attr(docsrs, doc(cfg(all())))]
pub fn pwritev(fd: RawFd, iov: &[IoSlice<'_>], offset: off_t) -> Result<usize> {
pub fn pwritev<Fd: AsFd>(fd: Fd, iov: &[IoSlice<'_>], offset: off_t) -> Result<usize> {
#[cfg(target_env = "uclibc")]
let offset = offset as libc::off64_t; // uclibc doesn't use off_t

// SAFETY: same as in writev()
let res = unsafe {
libc::pwritev(
fd,
fd.as_fd().as_raw_fd(),
iov.as_ptr() as *const libc::iovec,
iov.len() as c_int,
offset,
Expand All @@ -70,8 +70,8 @@ pub fn pwritev(fd: RawFd, iov: &[IoSlice<'_>], offset: off_t) -> Result<usize> {
/// See also: [`readv`](fn.readv.html) and [`pread`](fn.pread.html)
#[cfg(not(any(target_os = "redox", target_os = "haiku")))]
#[cfg_attr(docsrs, doc(cfg(all())))]
pub fn preadv(
fd: RawFd,
pub fn preadv<Fd: AsFd>(
fd: Fd,
iov: &mut [IoSliceMut<'_>],
offset: off_t,
) -> Result<usize> {
Expand All @@ -81,7 +81,7 @@ pub fn preadv(
// SAFETY: same as in readv()
let res = unsafe {
libc::preadv(
fd,
fd.as_fd().as_raw_fd(),
iov.as_ptr() as *const libc::iovec,
iov.len() as c_int,
offset,
Expand All @@ -95,10 +95,10 @@ pub fn preadv(
///
/// See also [pwrite(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/pwrite.html)
// TODO: move to unistd
pub fn pwrite(fd: RawFd, buf: &[u8], offset: off_t) -> Result<usize> {
pub fn pwrite<Fd: AsFd>(fd: Fd, buf: &[u8], offset: off_t) -> Result<usize> {
let res = unsafe {
libc::pwrite(
fd,
fd.as_fd().as_raw_fd(),
buf.as_ptr() as *const c_void,
buf.len() as size_t,
offset,
Expand All @@ -112,10 +112,10 @@ pub fn pwrite(fd: RawFd, buf: &[u8], offset: off_t) -> Result<usize> {
///
/// See also [pread(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/pread.html)
// TODO: move to unistd
pub fn pread(fd: RawFd, buf: &mut [u8], offset: off_t) -> Result<usize> {
pub fn pread<Fd: AsFd>(fd: Fd, buf: &mut [u8], offset: off_t) -> Result<usize> {
let res = unsafe {
libc::pread(
fd,
fd.as_fd().as_raw_fd(),
buf.as_mut_ptr() as *mut c_void,
buf.len() as size_t,
offset,
Expand Down
29 changes: 18 additions & 11 deletions test/sys/test_uio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rand::distributions::Alphanumeric;
use rand::{thread_rng, Rng};
use std::fs::OpenOptions;
use std::io::IoSlice;
use std::os::unix::io::AsRawFd;
use std::os::unix::io::{FromRawFd, OwnedFd};
use std::{cmp, iter};

#[cfg(not(target_os = "redox"))]
Expand Down Expand Up @@ -40,12 +40,16 @@ fn test_writev() {
iovecs.push(IoSlice::new(b));
consumed += slice_len;
}
let pipe_res = pipe();
let (reader, writer) = pipe_res.expect("Couldn't create pipe");
let (reader, writer) = pipe().expect("Couldn't create pipe");
// FileDesc will close its filedesc (reader).
let mut read_buf: Vec<u8> = iter::repeat(0u8).take(128 * 16).collect();

// Temporary workaround to cope with the existing RawFd pipe(2), should be
// removed when pipe(2) becomes I/O-safe.
let writer = unsafe { OwnedFd::from_raw_fd(writer) };

// Blocking io, should write all data.
let write_res = writev(writer, &iovecs);
let write_res = writev(&writer, &iovecs);
let written = write_res.expect("couldn't write");
// Check whether we written all data
assert_eq!(to_write.len(), written);
Expand All @@ -55,7 +59,6 @@ fn test_writev() {
assert_eq!(read, written);
// Check equality of written and read data
assert_eq!(&to_write, &read_buf);
close(writer).expect("closed writer");
close(reader).expect("closed reader");
}

Expand Down Expand Up @@ -88,7 +91,12 @@ fn test_readv() {
let (reader, writer) = pipe().expect("couldn't create pipe");
// Blocking io, should write all data.
write(writer, &to_write).expect("write failed");
let read = readv(reader, &mut iovecs[..]).expect("read failed");

// Temporary workaround to cope with the existing RawFd pipe(2), should be
// removed when pipe(2) becomes I/O-safe.
let reader = unsafe { OwnedFd::from_raw_fd(reader) };

let read = readv(&reader, &mut iovecs[..]).expect("read failed");
// Check whether we've read all data
assert_eq!(to_write.len(), read);
// Cccumulate data from iovecs
Expand All @@ -100,7 +108,6 @@ fn test_readv() {
assert_eq!(read_buf.len(), to_write.len());
// Check equality of written and read data
assert_eq!(&read_buf, &to_write);
close(reader).expect("couldn't close reader");
close(writer).expect("couldn't close writer");
}

Expand All @@ -111,7 +118,7 @@ fn test_pwrite() {

let mut file = tempfile().unwrap();
let buf = [1u8; 8];
assert_eq!(Ok(8), pwrite(file.as_raw_fd(), &buf, 8));
assert_eq!(Ok(8), pwrite(&file, &buf, 8));
let mut file_content = Vec::new();
file.read_to_end(&mut file_content).unwrap();
let mut expected = vec![0u8; 8];
Expand All @@ -137,7 +144,7 @@ fn test_pread() {
file.write_all(&file_content).unwrap();

let mut buf = [0u8; 16];
assert_eq!(Ok(16), pread(file.as_raw_fd(), &mut buf, 16));
assert_eq!(Ok(16), pread(&file, &mut buf, 16));
let expected: Vec<_> = (16..32).collect();
assert_eq!(&buf[..], &expected[..]);
}
Expand Down Expand Up @@ -168,7 +175,7 @@ fn test_pwritev() {
.open(path)
.unwrap();

let written = pwritev(file.as_raw_fd(), &iovecs, 100).ok().unwrap();
let written = pwritev(&file, &iovecs, 100).ok().unwrap();
assert_eq!(written, to_write.len());

// Read the data back and make sure it matches
Expand Down Expand Up @@ -206,7 +213,7 @@ fn test_preadv() {
.iter_mut()
.map(|buf| IoSliceMut::new(&mut buf[..]))
.collect();
assert_eq!(Ok(100), preadv(file.as_raw_fd(), &mut iovecs, 100));
assert_eq!(Ok(100), preadv(&file, &mut iovecs, 100));
}

let all = buffers.concat();
Expand Down