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.
This commit is contained in:
Dan Gohman
2019-09-09 09:14:23 -07:00
committed by Jakub Konka
parent c98b3d10ec
commit febecc418c
7 changed files with 26 additions and 22 deletions

View File

@@ -85,7 +85,7 @@ impl Drop for FdObject {
impl FdEntry { impl FdEntry {
pub(crate) fn from(file: fs::File) -> Result<Self> { pub(crate) fn from(file: fs::File) -> Result<Self> {
determine_type_and_access_rights(&file).map( unsafe { determine_type_and_access_rights(&file) }.map(
|(file_type, rights_base, rights_inheriting)| Self { |(file_type, rights_base, rights_inheriting)| Self {
fd_object: FdObject { fd_object: FdObject {
file_type, file_type,
@@ -104,7 +104,7 @@ impl FdEntry {
} }
pub(crate) fn duplicate_stdin() -> Result<Self> { pub(crate) fn duplicate_stdin() -> Result<Self> {
determine_type_and_access_rights(&io::stdin()).map( unsafe { determine_type_and_access_rights(&io::stdin()) }.map(
|(file_type, rights_base, rights_inheriting)| Self { |(file_type, rights_base, rights_inheriting)| Self {
fd_object: FdObject { fd_object: FdObject {
file_type, file_type,
@@ -119,7 +119,7 @@ impl FdEntry {
} }
pub(crate) fn duplicate_stdout() -> Result<Self> { pub(crate) fn duplicate_stdout() -> Result<Self> {
determine_type_and_access_rights(&io::stdout()).map( unsafe { determine_type_and_access_rights(&io::stdout()) }.map(
|(file_type, rights_base, rights_inheriting)| Self { |(file_type, rights_base, rights_inheriting)| Self {
fd_object: FdObject { fd_object: FdObject {
file_type, file_type,
@@ -134,7 +134,7 @@ impl FdEntry {
} }
pub(crate) fn duplicate_stderr() -> Result<Self> { pub(crate) fn duplicate_stderr() -> Result<Self> {
determine_type_and_access_rights(&io::stderr()).map( unsafe { determine_type_and_access_rights(&io::stderr()) }.map(
|(file_type, rights_base, rights_inheriting)| Self { |(file_type, rights_base, rights_inheriting)| Self {
fd_object: FdObject { fd_object: FdObject {
file_type, file_type,

View File

@@ -583,7 +583,7 @@ pub(crate) fn path_open(
let fd = hostcalls_impl::path_open(resolved, read, write, oflags, fs_flags)?; 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 // 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)?; let mut fe = FdEntry::from(fd)?;
fe.rights_base &= max_base; fe.rights_base &= max_base;
fe.rights_inheriting &= max_inheriting; fe.rights_inheriting &= max_inheriting;

View File

@@ -29,7 +29,8 @@ impl AsRawFd for Descriptor {
} }
} }
pub(crate) fn determine_type_and_access_rights<Fd: AsRawFd>( /// This function is unsafe because it operates on a raw file descriptor.
pub(crate) unsafe fn determine_type_and_access_rights<Fd: AsRawFd>(
fd: &Fd, fd: &Fd,
) -> Result<( ) -> Result<(
host::__wasi_filetype_t, host::__wasi_filetype_t,
@@ -51,7 +52,8 @@ pub(crate) fn determine_type_and_access_rights<Fd: AsRawFd>(
Ok((file_type, rights_base, rights_inheriting)) Ok((file_type, rights_base, rights_inheriting))
} }
pub(crate) fn determine_type_rights<Fd: AsRawFd>( /// This function is unsafe because it operates on a raw file descriptor.
pub(crate) unsafe fn determine_type_rights<Fd: AsRawFd>(
fd: &Fd, fd: &Fd,
) -> Result<( ) -> Result<(
host::__wasi_filetype_t, host::__wasi_filetype_t,
@@ -60,8 +62,7 @@ pub(crate) fn determine_type_rights<Fd: AsRawFd>(
)> { )> {
let (file_type, rights_base, rights_inheriting) = { 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 // we just make a `File` here for convenience; we don't want it to close when it drops
let file = let file = std::mem::ManuallyDrop::new(std::fs::File::from_raw_fd(fd.as_raw_fd()));
std::mem::ManuallyDrop::new(unsafe { std::fs::File::from_raw_fd(fd.as_raw_fd()) });
let ft = file.metadata()?.file_type(); let ft = file.metadata()?.file_type();
if ft.is_block_device() { if ft.is_block_device() {
( (

View File

@@ -45,7 +45,8 @@ impl AsRawHandle for Descriptor {
} }
} }
pub(crate) fn determine_type_and_access_rights<Handle: AsRawHandle>( /// This function is unsafe because it operates on a raw file handle.
pub(crate) unsafe fn determine_type_and_access_rights<Handle: AsRawHandle>(
handle: &Handle, handle: &Handle,
) -> Result<( ) -> Result<(
host::__wasi_filetype_t, host::__wasi_filetype_t,
@@ -75,7 +76,8 @@ pub(crate) fn determine_type_and_access_rights<Handle: AsRawHandle>(
Ok((file_type, rights_base, rights_inheriting)) Ok((file_type, rights_base, rights_inheriting))
} }
pub(crate) fn determine_type_rights<Handle: AsRawHandle>( /// This function is unsafe because it operates on a raw file handle.
pub(crate) unsafe fn determine_type_rights<Handle: AsRawHandle>(
handle: &Handle, handle: &Handle,
) -> Result<( ) -> Result<(
host::__wasi_filetype_t, host::__wasi_filetype_t,

View File

@@ -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<host::__wasi_fdflags_t> { pub(crate) fn fd_fdstat_get(fd: &File) -> Result<host::__wasi_fdflags_t> {
use winx::file::AccessMode; 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(host_impl::fdflags_from_win)
.map_err(Into::into) .map_err(Into::into)
} }
@@ -175,7 +175,7 @@ pub(crate) fn fd_readdir(
} }
pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result<usize> { pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result<usize> {
use winx::file::get_path_by_handle; use winx::file::get_file_path;
let path = resolved.concatenate()?; let path = resolved.concatenate()?;
let target_path = path.read_link()?; let target_path = path.read_link()?;
@@ -184,7 +184,7 @@ pub(crate) fn path_readlink(resolved: PathGet, buf: &mut [u8]) -> Result<usize>
// we need to strip the prefix from the absolute path // we need to strip the prefix from the absolute path
// as otherwise we will error out since WASI is not capable // as otherwise we will error out since WASI is not capable
// of dealing with absolute paths // 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 dir_path = PathBuf::from(strip_extended_prefix(dir_path));
let target_path = target_path let target_path = target_path
.strip_prefix(dir_path) .strip_prefix(dir_path)

View File

@@ -73,7 +73,7 @@ pub(crate) fn openat(dirfd: &File, path: &str) -> Result<File> {
} }
pub(crate) fn readlinkat(dirfd: &File, s_path: &str) -> Result<String> { pub(crate) fn readlinkat(dirfd: &File, s_path: &str) -> Result<String> {
use winx::file::get_path_by_handle; use winx::file::get_file_path;
use winx::winerror::WinError; use winx::winerror::WinError;
let path = concatenate(dirfd, Path::new(s_path))?; let path = concatenate(dirfd, Path::new(s_path))?;
@@ -83,7 +83,7 @@ pub(crate) fn readlinkat(dirfd: &File, s_path: &str) -> Result<String> {
// we need to strip the prefix from the absolute path // we need to strip the prefix from the absolute path
// as otherwise we will error out since WASI is not capable // as otherwise we will error out since WASI is not capable
// of dealing with absolute paths // 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)); let dir_path = PathBuf::from(strip_extended_prefix(dir_path));
target_path target_path
.strip_prefix(dir_path) .strip_prefix(dir_path)
@@ -128,7 +128,7 @@ pub(crate) fn strip_extended_prefix<P: AsRef<OsStr>>(path: P) -> OsString {
} }
pub(crate) fn concatenate<P: AsRef<Path>>(dirfd: &File, path: P) -> Result<PathBuf> { pub(crate) fn concatenate<P: AsRef<Path>>(dirfd: &File, path: P) -> Result<PathBuf> {
use winx::file::get_path_by_handle; use winx::file::get_file_path;
// WASI is not able to deal with absolute paths // WASI is not able to deal with absolute paths
// so error out if absolute // so error out if absolute
@@ -136,7 +136,7 @@ pub(crate) fn concatenate<P: AsRef<Path>>(dirfd: &File, path: P) -> Result<PathB
return Err(Error::ENOTCAPABLE); return Err(Error::ENOTCAPABLE);
} }
let dir_path = get_path_by_handle(dirfd.as_raw_handle())?; let dir_path = get_file_path(dirfd)?;
// concatenate paths // concatenate paths
let mut out_path = PathBuf::from(dir_path); let mut out_path = PathBuf::from(dir_path);
out_path.push(path.as_ref()); out_path.push(path.as_ref());

View File

@@ -45,8 +45,8 @@ impl FileType {
} }
} }
pub fn get_file_type(handle: RawHandle) -> Result<FileType> { pub unsafe fn get_file_type(handle: RawHandle) -> Result<FileType> {
let file_type = unsafe { FileType(GetFileType(handle)) }; let file_type = FileType(GetFileType(handle));
let err = winerror::WinError::last(); let err = winerror::WinError::last();
if file_type.is_unknown() && err != winerror::WinError::ERROR_SUCCESS { if file_type.is_unknown() && err != winerror::WinError::ERROR_SUCCESS {
Err(err) Err(err)
@@ -314,7 +314,7 @@ bitflags! {
} }
} }
pub fn get_file_access_mode(handle: RawHandle) -> Result<AccessMode> { pub unsafe fn get_file_access_mode(handle: RawHandle) -> Result<AccessMode> {
use winapi::shared::minwindef::FALSE; use winapi::shared::minwindef::FALSE;
use winapi::um::accctrl; use winapi::um::accctrl;
use winapi::um::aclapi::GetSecurityInfo; use winapi::um::aclapi::GetSecurityInfo;
@@ -357,11 +357,12 @@ pub fn get_file_access_mode(handle: RawHandle) -> Result<AccessMode> {
} }
} }
pub fn get_path_by_handle(handle: RawHandle) -> Result<OsString> { pub fn get_file_path(file: &File) -> Result<OsString> {
use winapi::um::fileapi::GetFinalPathNameByHandleW; use winapi::um::fileapi::GetFinalPathNameByHandleW;
let mut raw_path: Vec<u16> = vec![0; WIDE_MAX_PATH as usize]; let mut raw_path: Vec<u16> = vec![0; WIDE_MAX_PATH as usize];
let handle = file.as_raw_handle();
let read_len = let read_len =
unsafe { GetFinalPathNameByHandleW(handle, raw_path.as_mut_ptr(), WIDE_MAX_PATH, 0) }; unsafe { GetFinalPathNameByHandleW(handle, raw_path.as_mut_ptr(), WIDE_MAX_PATH, 0) };