Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
1913: feat: I/O safety for 'sys/inotify' r=asomers a=SteveLauC

#### What this PR does:

1. Changes the `fd` field of `struct Inotify` from `RawFd` to `OwnedFd`
2. Changes the interfaces of functions in the `impl Inotify {}`
   
    > The type of `self` changes from `Self` to `&mut Self`. 
    
    From:

   ```rust
   pub fn add_watch<P: ?Sized + NixPath>(
         self,
         path: &P,
         mask: AddWatchFlags,
   ) -> Result<WatchDescriptor> 

   pub fn rm_watch(self, wd: WatchDescriptor) -> Result<()>

   pub fn read_events(self) -> Result<Vec<InotifyEvent>>
   ```

   To:

   ```rust
   pub fn add_watch<P: ?Sized + NixPath>(
        &mut self,
        path: &P,
        mask: AddWatchFlags,
    ) -> Result<WatchDescriptor>

   pub fn rm_watch(&mut self, wd: WatchDescriptor) -> Result<()>

   pub fn read_events(&mut self) -> Result<Vec<InotifyEvent>>
   ```
  

   In the previous implementation, these functions can take `self` by value as `struct Inotify` [was `Copy`](https://docs.rs/nix/latest/nix/sys/inotify/struct.Inotify.html#impl-Copy-for-Inotify). With the changes in `1` applied, `struct Inotify` is no longer `Copy`, so we have to take `self` by reference.

-------

Blocks until the merge of #1863 as this PR needs `read(2)` to be I/O-safe.


1926: feat: I/O safety for 'sys/sendfile' r=asomers a=SteveLauC

#### What this PR does:
1. Adds I/O safety for module `sys/sendfile`.

1927: feat: I/O safety for 'sys/statvfs' r=asomers a=SteveLauC

#### What this PR does:
1. Adds I/O safety for module `sys/statvfs`.

1931: feat: I/O safety for 'sys/uid' & 'sched' r=asomers a=SteveLauC

#### What this PR does:
Adds I/O safety for modules:

1. `sys/uio`
2. `sched`

1933: feat: I/O safety for 'sys/timerfd' r=asomers a=SteveLauC

#### What this PR does:
1. Adds I/O safety  for module `sys/timerfd`.

Co-authored-by: Steve Lau <[email protected]>
  • Loading branch information
bors[bot] and SteveLauC authored Dec 9, 2022
6 parents e11646e + 9d74330 + ab5c032 + a0e39af + 8772cde + f592678 commit 3d3e6b9
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 106 deletions.
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: 11 additions & 17 deletions src/sys/inotify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use libc::{c_char, c_int};
use std::ffi::{CStr, OsStr, OsString};
use std::mem::{size_of, MaybeUninit};
use std::os::unix::ffi::OsStrExt;
use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
use std::os::unix::io::{AsRawFd, FromRawFd, OwnedFd, RawFd};
use std::ptr;

libc_bitflags! {
Expand Down Expand Up @@ -101,9 +101,9 @@ libc_bitflags! {

/// An inotify instance. This is also a file descriptor, you can feed it to
/// other interfaces consuming file descriptors, epoll for example.
#[derive(Debug, Clone, Copy)]
#[derive(Debug)]
pub struct Inotify {
fd: RawFd,
fd: OwnedFd,
}

/// This object is returned when you create a new watch on an inotify instance.
Expand Down Expand Up @@ -143,7 +143,7 @@ impl Inotify {
pub fn init(flags: InitFlags) -> Result<Inotify> {
let res = Errno::result(unsafe { libc::inotify_init1(flags.bits()) });

res.map(|fd| Inotify { fd })
res.map(|fd| Inotify { fd: unsafe { OwnedFd::from_raw_fd(fd) } })
}

/// Adds a new watch on the target file or directory.
Expand All @@ -152,12 +152,12 @@ impl Inotify {
///
/// For more information see, [inotify_add_watch(2)](https://man7.org/linux/man-pages/man2/inotify_add_watch.2.html).
pub fn add_watch<P: ?Sized + NixPath>(
self,
&self,
path: &P,
mask: AddWatchFlags,
) -> Result<WatchDescriptor> {
let res = path.with_nix_path(|cstr| unsafe {
libc::inotify_add_watch(self.fd, cstr.as_ptr(), mask.bits())
libc::inotify_add_watch(self.fd.as_raw_fd(), cstr.as_ptr(), mask.bits())
})?;

Errno::result(res).map(|wd| WatchDescriptor { wd })
Expand All @@ -169,15 +169,15 @@ impl Inotify {
/// Returns an EINVAL error if the watch descriptor is invalid.
///
/// For more information see, [inotify_rm_watch(2)](https://man7.org/linux/man-pages/man2/inotify_rm_watch.2.html).
pub fn rm_watch(self, wd: WatchDescriptor) -> Result<()> {
pub fn rm_watch(&self, wd: WatchDescriptor) -> Result<()> {
cfg_if! {
if #[cfg(target_os = "linux")] {
let arg = wd.wd;
} else if #[cfg(target_os = "android")] {
let arg = wd.wd as u32;
}
}
let res = unsafe { libc::inotify_rm_watch(self.fd, arg) };
let res = unsafe { libc::inotify_rm_watch(self.fd.as_raw_fd(), arg) };

Errno::result(res).map(drop)
}
Expand All @@ -188,14 +188,14 @@ impl Inotify {
///
/// Returns as many events as available. If the call was non blocking and no
/// events could be read then the EAGAIN error is returned.
pub fn read_events(self) -> Result<Vec<InotifyEvent>> {
pub fn read_events(&self) -> Result<Vec<InotifyEvent>> {
let header_size = size_of::<libc::inotify_event>();
const BUFSIZ: usize = 4096;
let mut buffer = [0u8; BUFSIZ];
let mut events = Vec::new();
let mut offset = 0;

let nread = read(self.fd, &mut buffer)?;
let nread = read(self.fd.as_raw_fd(), &mut buffer)?;

while (nread - offset) >= header_size {
let event = unsafe {
Expand Down Expand Up @@ -235,14 +235,8 @@ impl Inotify {
}
}

impl AsRawFd for Inotify {
fn as_raw_fd(&self) -> RawFd {
self.fd
}
}

impl FromRawFd for Inotify {
unsafe fn from_raw_fd(fd: RawFd) -> Self {
Inotify { fd }
Inotify { fd: OwnedFd::from_raw_fd(fd) }
}
}
62 changes: 38 additions & 24 deletions src/sys/sendfile.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Send data from a file to a socket, bypassing userland.
use cfg_if::cfg_if;
use std::os::unix::io::RawFd;
use std::os::unix::io::{AsFd, AsRawFd};
use std::ptr;

use libc::{self, off_t};
Expand All @@ -23,16 +23,23 @@ use crate::Result;
/// For more information, see [the sendfile(2) man page.](https://man7.org/linux/man-pages/man2/sendfile.2.html)
#[cfg(any(target_os = "android", target_os = "linux"))]
#[cfg_attr(docsrs, doc(cfg(all())))]
pub fn sendfile(
out_fd: RawFd,
in_fd: RawFd,
pub fn sendfile<F1: AsFd, F2: AsFd>(
out_fd: F1,
in_fd: F2,
offset: Option<&mut off_t>,
count: usize,
) -> Result<usize> {
let offset = offset
.map(|offset| offset as *mut _)
.unwrap_or(ptr::null_mut());
let ret = unsafe { libc::sendfile(out_fd, in_fd, offset, count) };
let ret = unsafe {
libc::sendfile(
out_fd.as_fd().as_raw_fd(),
in_fd.as_fd().as_raw_fd(),
offset,
count,
)
};
Errno::result(ret).map(|r| r as usize)
}

Expand All @@ -50,16 +57,23 @@ pub fn sendfile(
/// For more information, see [the sendfile(2) man page.](https://man7.org/linux/man-pages/man2/sendfile.2.html)
#[cfg(target_os = "linux")]
#[cfg_attr(docsrs, doc(cfg(all())))]
pub fn sendfile64(
out_fd: RawFd,
in_fd: RawFd,
pub fn sendfile64<F1: AsFd, F2: AsFd>(
out_fd: F1,
in_fd: F2,
offset: Option<&mut libc::off64_t>,
count: usize,
) -> Result<usize> {
let offset = offset
.map(|offset| offset as *mut _)
.unwrap_or(ptr::null_mut());
let ret = unsafe { libc::sendfile64(out_fd, in_fd, offset, count) };
let ret = unsafe {
libc::sendfile64(
out_fd.as_fd().as_raw_fd(),
in_fd.as_fd().as_raw_fd(),
offset,
count,
)
};
Errno::result(ret).map(|r| r as usize)
}

Expand Down Expand Up @@ -156,9 +170,9 @@ cfg_if! {
/// For more information, see
/// [the sendfile(2) man page.](https://www.freebsd.org/cgi/man.cgi?query=sendfile&sektion=2)
#[allow(clippy::too_many_arguments)]
pub fn sendfile(
in_fd: RawFd,
out_sock: RawFd,
pub fn sendfile<F1: AsFd, F2: AsFd>(
in_fd: F1,
out_sock: F2,
offset: off_t,
count: Option<usize>,
headers: Option<&[&[u8]]>,
Expand All @@ -175,8 +189,8 @@ cfg_if! {
let hdtr = headers.or(trailers).map(|_| SendfileHeaderTrailer::new(headers, trailers));
let hdtr_ptr = hdtr.as_ref().map_or(ptr::null(), |s| &s.0 as *const libc::sf_hdtr);
let return_code = unsafe {
libc::sendfile(in_fd,
out_sock,
libc::sendfile(in_fd.as_fd().as_raw_fd(),
out_sock.as_fd().as_raw_fd(),
offset,
count.unwrap_or(0),
hdtr_ptr as *mut libc::sf_hdtr,
Expand Down Expand Up @@ -206,9 +220,9 @@ cfg_if! {
///
/// For more information, see
/// [the sendfile(2) man page.](https://leaf.dragonflybsd.org/cgi/web-man?command=sendfile&section=2)
pub fn sendfile(
in_fd: RawFd,
out_sock: RawFd,
pub fn sendfile<F1: AsFd, F2: AsFd>(
in_fd: F1,
out_sock: F2,
offset: off_t,
count: Option<usize>,
headers: Option<&[&[u8]]>,
Expand All @@ -218,8 +232,8 @@ cfg_if! {
let hdtr = headers.or(trailers).map(|_| SendfileHeaderTrailer::new(headers, trailers));
let hdtr_ptr = hdtr.as_ref().map_or(ptr::null(), |s| &s.0 as *const libc::sf_hdtr);
let return_code = unsafe {
libc::sendfile(in_fd,
out_sock,
libc::sendfile(in_fd.as_fd().as_raw_fd(),
out_sock.as_fd().as_raw_fd(),
offset,
count.unwrap_or(0),
hdtr_ptr as *mut libc::sf_hdtr,
Expand Down Expand Up @@ -252,9 +266,9 @@ cfg_if! {
///
/// For more information, see
/// [the sendfile(2) man page.](https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man2/sendfile.2.html)
pub fn sendfile(
in_fd: RawFd,
out_sock: RawFd,
pub fn sendfile<F1: AsFd, F2: AsFd>(
in_fd: F1,
out_sock: F2,
offset: off_t,
count: Option<off_t>,
headers: Option<&[&[u8]]>,
Expand All @@ -264,8 +278,8 @@ cfg_if! {
let hdtr = headers.or(trailers).map(|_| SendfileHeaderTrailer::new(headers, trailers));
let hdtr_ptr = hdtr.as_ref().map_or(ptr::null(), |s| &s.0 as *const libc::sf_hdtr);
let return_code = unsafe {
libc::sendfile(in_fd,
out_sock,
libc::sendfile(in_fd.as_fd().as_raw_fd(),
out_sock.as_fd().as_raw_fd(),
offset,
&mut len as *mut off_t,
hdtr_ptr as *mut libc::sf_hdtr,
Expand Down
6 changes: 3 additions & 3 deletions src/sys/statvfs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! See [the man pages](https://pubs.opengroup.org/onlinepubs/9699919799/functions/fstatvfs.html)
//! for more details.
use std::mem;
use std::os::unix::io::AsRawFd;
use std::os::unix::io::{AsFd, AsRawFd};

use libc::{self, c_ulong};

Expand Down Expand Up @@ -146,11 +146,11 @@ pub fn statvfs<P: ?Sized + NixPath>(path: &P) -> Result<Statvfs> {
}

/// Return a `Statvfs` object with information about `fd`
pub fn fstatvfs<T: AsRawFd>(fd: &T) -> Result<Statvfs> {
pub fn fstatvfs<Fd: AsFd>(fd: Fd) -> Result<Statvfs> {
unsafe {
Errno::clear();
let mut stat = mem::MaybeUninit::<libc::statvfs>::uninit();
Errno::result(libc::fstatvfs(fd.as_raw_fd(), stat.as_mut_ptr()))
Errno::result(libc::fstatvfs(fd.as_fd().as_raw_fd(), stat.as_mut_ptr()))
.map(|_| Statvfs(stat.assume_init()))
}
}
Expand Down
44 changes: 21 additions & 23 deletions src/sys/timerfd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,24 +33,28 @@ pub use crate::sys::time::timer::{Expiration, TimerSetTimeFlags};
use crate::unistd::read;
use crate::{errno::Errno, Result};
use libc::c_int;
use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd, RawFd};

/// A timerfd instance. This is also a file descriptor, you can feed it to
/// other interfaces consuming file descriptors, epoll for example.
/// other interfaces taking file descriptors as arguments, [`epoll`] for example.
///
/// [`epoll`]: crate::sys::epoll
#[derive(Debug)]
pub struct TimerFd {
fd: RawFd,
fd: OwnedFd,
}

impl AsRawFd for TimerFd {
fn as_raw_fd(&self) -> RawFd {
self.fd
impl AsFd for TimerFd {
fn as_fd(&self) -> BorrowedFd<'_> {
self.fd.as_fd()
}
}

impl FromRawFd for TimerFd {
unsafe fn from_raw_fd(fd: RawFd) -> Self {
TimerFd { fd }
TimerFd {
fd: OwnedFd::from_raw_fd(fd),
}
}
}

Expand Down Expand Up @@ -97,7 +101,9 @@ impl TimerFd {
Errno::result(unsafe {
libc::timerfd_create(clockid as i32, flags.bits())
})
.map(|fd| Self { fd })
.map(|fd| Self {
fd: unsafe { OwnedFd::from_raw_fd(fd) },
})
}

/// Sets a new alarm on the timer.
Expand Down Expand Up @@ -145,7 +151,7 @@ impl TimerFd {
let timerspec: TimerSpec = expiration.into();
Errno::result(unsafe {
libc::timerfd_settime(
self.fd,
self.fd.as_fd().as_raw_fd(),
flags.bits(),
timerspec.as_ref(),
std::ptr::null_mut(),
Expand All @@ -159,7 +165,10 @@ impl TimerFd {
pub fn get(&self) -> Result<Option<Expiration>> {
let mut timerspec = TimerSpec::none();
Errno::result(unsafe {
libc::timerfd_gettime(self.fd, timerspec.as_mut())
libc::timerfd_gettime(
self.fd.as_fd().as_raw_fd(),
timerspec.as_mut(),
)
})
.map(|_| {
if timerspec.as_ref().it_interval.tv_sec == 0
Expand All @@ -179,7 +188,7 @@ impl TimerFd {
pub fn unset(&self) -> Result<()> {
Errno::result(unsafe {
libc::timerfd_settime(
self.fd,
self.fd.as_fd().as_raw_fd(),
TimerSetTimeFlags::empty().bits(),
TimerSpec::none().as_ref(),
std::ptr::null_mut(),
Expand All @@ -192,7 +201,7 @@ impl TimerFd {
///
/// Note: If the alarm is unset, then you will wait forever.
pub fn wait(&self) -> Result<()> {
while let Err(e) = read(self.fd, &mut [0u8; 8]) {
while let Err(e) = read(self.fd.as_fd().as_raw_fd(), &mut [0u8; 8]) {
if e != Errno::EINTR {
return Err(e);
}
Expand All @@ -201,14 +210,3 @@ impl TimerFd {
Ok(())
}
}

impl Drop for TimerFd {
fn drop(&mut self) {
if !std::thread::panicking() {
let result = Errno::result(unsafe { libc::close(self.fd) });
if let Err(Errno::EBADF) = result {
panic!("close of TimerFd encountered EBADF");
}
}
}
}
Loading

0 comments on commit 3d3e6b9

Please sign in to comment.