From febecc418c01344a4276f001c080afb0ed8b430a Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 9 Sep 2019 09:14:23 -0700 Subject: [PATCH] Make functions that operate on raw I/O handles unsafe. Functions which trust that their arguments are valid raw file descriptors or raw handles should be marked unsafe, because these arguments are passed unchecked to I/O routines. --- src/fdentry.rs | 8 ++++---- src/hostcalls_impl/fs.rs | 2 +- src/sys/unix/fdentry_impl.rs | 9 +++++---- src/sys/windows/fdentry_impl.rs | 6 ++++-- src/sys/windows/hostcalls_impl/fs.rs | 6 +++--- src/sys/windows/hostcalls_impl/fs_helpers.rs | 8 ++++---- winx/src/file.rs | 9 +++++---- 7 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/fdentry.rs b/src/fdentry.rs index 99c4bf6297..694942321d 100644 --- a/src/fdentry.rs +++ b/src/fdentry.rs @@ -85,7 +85,7 @@ impl Drop for FdObject { impl FdEntry { pub(crate) fn from(file: fs::File) -> Result { - determine_type_and_access_rights(&file).map( + unsafe { determine_type_and_access_rights(&file) }.map( |(file_type, rights_base, rights_inheriting)| Self { fd_object: FdObject { file_type, @@ -104,7 +104,7 @@ impl FdEntry { } pub(crate) fn duplicate_stdin() -> Result { - determine_type_and_access_rights(&io::stdin()).map( + unsafe { determine_type_and_access_rights(&io::stdin()) }.map( |(file_type, rights_base, rights_inheriting)| Self { fd_object: FdObject { file_type, @@ -119,7 +119,7 @@ impl FdEntry { } pub(crate) fn duplicate_stdout() -> Result { - determine_type_and_access_rights(&io::stdout()).map( + unsafe { determine_type_and_access_rights(&io::stdout()) }.map( |(file_type, rights_base, rights_inheriting)| Self { fd_object: FdObject { file_type, @@ -134,7 +134,7 @@ impl FdEntry { } pub(crate) fn duplicate_stderr() -> Result { - determine_type_and_access_rights(&io::stderr()).map( + unsafe { determine_type_and_access_rights(&io::stderr()) }.map( |(file_type, rights_base, rights_inheriting)| Self { fd_object: FdObject { file_type, diff --git a/src/hostcalls_impl/fs.rs b/src/hostcalls_impl/fs.rs index 064efa3199..53aaef865c 100644 --- a/src/hostcalls_impl/fs.rs +++ b/src/hostcalls_impl/fs.rs @@ -583,7 +583,7 @@ pub(crate) fn path_open( let fd = hostcalls_impl::path_open(resolved, read, write, oflags, fs_flags)?; // Determine the type of the new file descriptor and which rights contradict with this type - let (_ty, max_base, max_inheriting) = determine_type_rights(&fd)?; + let (_ty, max_base, max_inheriting) = unsafe { determine_type_rights(&fd) }?; let mut fe = FdEntry::from(fd)?; fe.rights_base &= max_base; fe.rights_inheriting &= max_inheriting; diff --git a/src/sys/unix/fdentry_impl.rs b/src/sys/unix/fdentry_impl.rs index 5b37c508fc..ca1dff5d9d 100644 --- a/src/sys/unix/fdentry_impl.rs +++ b/src/sys/unix/fdentry_impl.rs @@ -29,7 +29,8 @@ impl AsRawFd for Descriptor { } } -pub(crate) fn determine_type_and_access_rights( +/// This function is unsafe because it operates on a raw file descriptor. +pub(crate) unsafe fn determine_type_and_access_rights( fd: &Fd, ) -> Result<( host::__wasi_filetype_t, @@ -51,7 +52,8 @@ pub(crate) fn determine_type_and_access_rights( Ok((file_type, rights_base, rights_inheriting)) } -pub(crate) fn determine_type_rights( +/// This function is unsafe because it operates on a raw file descriptor. +pub(crate) unsafe fn determine_type_rights( fd: &Fd, ) -> Result<( host::__wasi_filetype_t, @@ -60,8 +62,7 @@ pub(crate) fn determine_type_rights( )> { let (file_type, rights_base, rights_inheriting) = { // we just make a `File` here for convenience; we don't want it to close when it drops - let file = - std::mem::ManuallyDrop::new(unsafe { std::fs::File::from_raw_fd(fd.as_raw_fd()) }); + let file = std::mem::ManuallyDrop::new(std::fs::File::from_raw_fd(fd.as_raw_fd())); let ft = file.metadata()?.file_type(); if ft.is_block_device() { ( diff --git a/src/sys/windows/fdentry_impl.rs b/src/sys/windows/fdentry_impl.rs index 5daaeeee7a..932d7fa122 100644 --- a/src/sys/windows/fdentry_impl.rs +++ b/src/sys/windows/fdentry_impl.rs @@ -45,7 +45,8 @@ impl AsRawHandle for Descriptor { } } -pub(crate) fn determine_type_and_access_rights( +/// This function is unsafe because it operates on a raw file handle. +pub(crate) unsafe fn determine_type_and_access_rights( handle: &Handle, ) -> Result<( host::__wasi_filetype_t, @@ -75,7 +76,8 @@ pub(crate) fn determine_type_and_access_rights( Ok((file_type, rights_base, rights_inheriting)) } -pub(crate) fn determine_type_rights( +/// This function is unsafe because it operates on a raw file handle. +pub(crate) unsafe fn determine_type_rights( handle: &Handle, ) -> Result<( host::__wasi_filetype_t, diff --git a/src/sys/windows/hostcalls_impl/fs.rs b/src/sys/windows/hostcalls_impl/fs.rs index 32286e887c..70ae34f7f8 100644 --- a/src/sys/windows/hostcalls_impl/fs.rs +++ b/src/sys/windows/hostcalls_impl/fs.rs @@ -52,7 +52,7 @@ pub(crate) fn fd_pwrite(file: &File, buf: &[u8], offset: host::__wasi_filesize_t pub(crate) fn fd_fdstat_get(fd: &File) -> Result { use winx::file::AccessMode; - winx::file::get_file_access_mode(fd.as_raw_handle()) + unsafe { winx::file::get_file_access_mode(fd.as_raw_handle()) } .map(host_impl::fdflags_from_win) .map_err(Into::into) } @@ -175,7 +175,7 @@ pub(crate) fn fd_readdir( } pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result { - use winx::file::get_path_by_handle; + use winx::file::get_file_path; let path = resolved.concatenate()?; let target_path = path.read_link()?; @@ -184,7 +184,7 @@ pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result // we need to strip the prefix from the absolute path // as otherwise we will error out since WASI is not capable // of dealing with absolute paths - let dir_path = get_path_by_handle(resolved.dirfd().as_raw_handle())?; + let dir_path = get_file_path(resolved.dirfd())?; let dir_path = PathBuf::from(strip_extended_prefix(dir_path)); let target_path = target_path .strip_prefix(dir_path) diff --git a/src/sys/windows/hostcalls_impl/fs_helpers.rs b/src/sys/windows/hostcalls_impl/fs_helpers.rs index 4b59e4bbf9..746fb8a91d 100644 --- a/src/sys/windows/hostcalls_impl/fs_helpers.rs +++ b/src/sys/windows/hostcalls_impl/fs_helpers.rs @@ -73,7 +73,7 @@ pub(crate) fn openat(dirfd: &File, path: &str) -> Result { } pub(crate) fn readlinkat(dirfd: &File, s_path: &str) -> Result { - use winx::file::get_path_by_handle; + use winx::file::get_file_path; use winx::winerror::WinError; let path = concatenate(dirfd, Path::new(s_path))?; @@ -83,7 +83,7 @@ pub(crate) fn readlinkat(dirfd: &File, s_path: &str) -> Result { // we need to strip the prefix from the absolute path // as otherwise we will error out since WASI is not capable // of dealing with absolute paths - let dir_path = get_path_by_handle(dirfd.as_raw_handle())?; + let dir_path = get_file_path(dirfd)?; let dir_path = PathBuf::from(strip_extended_prefix(dir_path)); target_path .strip_prefix(dir_path) @@ -128,7 +128,7 @@ pub(crate) fn strip_extended_prefix>(path: P) -> OsString { } pub(crate) fn concatenate>(dirfd: &File, path: P) -> Result { - use winx::file::get_path_by_handle; + use winx::file::get_file_path; // WASI is not able to deal with absolute paths // so error out if absolute @@ -136,7 +136,7 @@ pub(crate) fn concatenate>(dirfd: &File, path: P) -> Result Result { - let file_type = unsafe { FileType(GetFileType(handle)) }; +pub unsafe fn get_file_type(handle: RawHandle) -> Result { + let file_type = FileType(GetFileType(handle)); let err = winerror::WinError::last(); if file_type.is_unknown() && err != winerror::WinError::ERROR_SUCCESS { Err(err) @@ -314,7 +314,7 @@ bitflags! { } } -pub fn get_file_access_mode(handle: RawHandle) -> Result { +pub unsafe fn get_file_access_mode(handle: RawHandle) -> Result { use winapi::shared::minwindef::FALSE; use winapi::um::accctrl; use winapi::um::aclapi::GetSecurityInfo; @@ -357,11 +357,12 @@ pub fn get_file_access_mode(handle: RawHandle) -> Result { } } -pub fn get_path_by_handle(handle: RawHandle) -> Result { +pub fn get_file_path(file: &File) -> Result { use winapi::um::fileapi::GetFinalPathNameByHandleW; let mut raw_path: Vec = vec![0; WIDE_MAX_PATH as usize]; + let handle = file.as_raw_handle(); let read_len = unsafe { GetFinalPathNameByHandleW(handle, raw_path.as_mut_ptr(), WIDE_MAX_PATH, 0) };