From 8d38e2eede492051241588d4af76b9a7a41a403b Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Tue, 12 Nov 2019 16:41:54 -0800 Subject: [PATCH] Implement `__wasi_fd_fdstat_get` for Windows. This commit fully implements `__wasi_fd_fdstat_get` on Windows so that the descriptor flags can be determined. It does this by calling into `NtQueryInformationFile` (safe to call from usermode) to get the open mode and access of the underlying OS handle. `NtQueryInformationFile` isn't included in the `winapi` crate, so it is manually being linked against. This commit also fixes several bugs on Windows: * `__WASI_O_TRUNC` should open files for write. * Use `FILE_FLAG_WRITE_THROUGH` for the `__WASI_FDFLAG_?SYNC` flags. * `__WASI_FDFLAG_APPEND` should disallow `FILE_WRITE_DATA` access to force append-only on write operations. * Use `GENERIC_READ` and `GENERIC_WRITE` access flags. The latter is required when opening a file for truncation. Additionally, the `truncation_rights` test is now enabled. --- crates/wasi-common/build.rs | 3 +- crates/wasi-common/src/hostcalls_impl/fs.rs | 3 +- .../src/sys/windows/fdentry_impl.rs | 4 +- .../wasi-common/src/sys/windows/host_impl.rs | 71 ++++--- .../src/sys/windows/hostcalls_impl/fs.rs | 29 +-- crates/wasi-common/winx/src/file.rs | 189 ++++++++++++------ 6 files changed, 193 insertions(+), 106 deletions(-) diff --git a/crates/wasi-common/build.rs b/crates/wasi-common/build.rs index 8d464ff92f51..5158dfe447a2 100644 --- a/crates/wasi-common/build.rs +++ b/crates/wasi-common/build.rs @@ -133,7 +133,7 @@ mod wasm_tests { &stemstr.replace("-", "_") )?; writeln!(out, " setup_log();")?; - write!( + writeln!( out, " let path = std::path::Path::new(r#\"{}\"#);", path.display() @@ -176,7 +176,6 @@ mod wasm_tests { "readlink_no_buffer" => true, "dangling_symlink" => true, "symlink_loop" => true, - "truncation_rights" => true, "path_rename_trailing_slashes" => true, "fd_readdir" => true, "poll_oneoff" => true, diff --git a/crates/wasi-common/src/hostcalls_impl/fs.rs b/crates/wasi-common/src/hostcalls_impl/fs.rs index ae2485be211d..f627c400a5c7 100644 --- a/crates/wasi-common/src/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/hostcalls_impl/fs.rs @@ -559,7 +559,8 @@ pub(crate) unsafe fn path_open( | wasi::__WASI_RIGHT_FD_WRITE | wasi::__WASI_RIGHT_FD_ALLOCATE | wasi::__WASI_RIGHT_FD_FILESTAT_SET_SIZE) - != 0; + != 0 + || oflags & wasi::__WASI_O_TRUNC != 0; let fd = hostcalls_impl::path_open(resolved, read, write, oflags, fs_flags)?; diff --git a/crates/wasi-common/src/sys/windows/fdentry_impl.rs b/crates/wasi-common/src/sys/windows/fdentry_impl.rs index 7e8bda05670b..20f6d9cc52f4 100644 --- a/crates/wasi-common/src/sys/windows/fdentry_impl.rs +++ b/crates/wasi-common/src/sys/windows/fdentry_impl.rs @@ -53,13 +53,13 @@ pub(crate) unsafe fn determine_type_and_access_rights( wasi::__wasi_rights_t, wasi::__wasi_rights_t, )> { - use winx::file::{get_file_access_mode, AccessMode}; + use winx::file::{query_access_information, AccessMode}; let (file_type, mut rights_base, rights_inheriting) = determine_type_rights(handle)?; match file_type { wasi::__WASI_FILETYPE_DIRECTORY | wasi::__WASI_FILETYPE_REGULAR_FILE => { - let mode = get_file_access_mode(handle.as_raw_handle())?; + let mode = query_access_information(handle.as_raw_handle())?; if mode.contains(AccessMode::FILE_GENERIC_READ) { rights_base |= wasi::__WASI_RIGHT_FD_READ; } diff --git a/crates/wasi-common/src/sys/windows/host_impl.rs b/crates/wasi-common/src/sys/windows/host_impl.rs index 18cf4a4adb17..e12c6c63520b 100644 --- a/crates/wasi-common/src/sys/windows/host_impl.rs +++ b/crates/wasi-common/src/sys/windows/host_impl.rs @@ -7,7 +7,7 @@ use std::ffi::OsStr; use std::fs::OpenOptions; use std::os::windows::ffi::OsStrExt; use std::os::windows::fs::OpenOptionsExt; -use winx::file::{AccessMode, Attributes, CreationDisposition, Flags}; +use winx::file::{AccessMode, Attributes, CreationDisposition, FileModeInformation, Flags}; pub(crate) fn errno_from_win(error: winx::winerror::WinError) -> wasi::__wasi_errno_t { // TODO: implement error mapping between Windows and WASI @@ -40,45 +40,70 @@ pub(crate) fn errno_from_win(error: winx::winerror::WinError) -> wasi::__wasi_er } } -pub(crate) fn fdflags_from_win(mode: AccessMode) -> wasi::__wasi_fdflags_t { +pub(crate) fn fdflags_from_win( + access: AccessMode, + mode: FileModeInformation, +) -> wasi::__wasi_fdflags_t { let mut fdflags = 0; - // TODO verify this! - if mode.contains(AccessMode::FILE_APPEND_DATA) { + + if access.contains(AccessMode::FILE_APPEND_DATA) + && !access.contains(AccessMode::FILE_WRITE_DATA) + { fdflags |= wasi::__WASI_FDFLAG_APPEND; } - if mode.contains(AccessMode::SYNCHRONIZE) { - fdflags |= wasi::__WASI_FDFLAG_DSYNC; - fdflags |= wasi::__WASI_FDFLAG_RSYNC; + + if mode.contains(FileModeInformation::FILE_WRITE_THROUGH) { + // Only report __WASI_FDFLAG_SYNC, which is technically the only one of the O_?SYNC flags Windows supports. fdflags |= wasi::__WASI_FDFLAG_SYNC; } - // The NONBLOCK equivalent is FILE_FLAG_OVERLAPPED - // but it seems winapi doesn't provide a mechanism - // for checking whether the handle supports async IO. - // On the contrary, I've found some dicsussion online - // which suggests that on Windows all handles should - // generally be assumed to be opened with async support - // and then the program should fallback should that **not** - // be the case at the time of the operation. - // TODO: this requires further investigation + + if !mode.contains(FileModeInformation::FILE_SYNCHRONOUS_IO_ALERT) + && !mode.contains(FileModeInformation::FILE_SYNCHRONOUS_IO_NONALERT) + { + fdflags |= wasi::__WASI_FDFLAG_NONBLOCK; + } + fdflags } -pub(crate) fn win_from_fdflags(fdflags: wasi::__wasi_fdflags_t) -> (AccessMode, Flags) { - let mut access_mode = AccessMode::empty(); - let mut flags = Flags::empty(); +pub(crate) fn win_from_fdflags( + fdflags: wasi::__wasi_fdflags_t, + read: bool, + write: bool, +) -> (AccessMode, Flags) { + let mut access_mode = AccessMode::READ_CONTROL; - // TODO verify this! - if fdflags & wasi::__WASI_FDFLAG_NONBLOCK != 0 { - flags.insert(Flags::FILE_FLAG_OVERLAPPED); + if read { + access_mode.insert(AccessMode::GENERIC_READ); } + + if write { + access_mode.insert(AccessMode::GENERIC_WRITE); + } + + // For append, grant the handle FILE_APPEND_DATA access but *not* FILE_WRITE_DATA. + // This makes the handle "append only". if fdflags & wasi::__WASI_FDFLAG_APPEND != 0 { access_mode.insert(AccessMode::FILE_APPEND_DATA); + access_mode.remove(AccessMode::FILE_WRITE_DATA); } + + // Enable backup semantics so directories can be opened as files + let mut flags = Flags::FILE_FLAG_BACKUP_SEMANTICS; + + // TODO: setting this flag is the correct thing to do on Windows. + // However, the read *must* be overlapped (i.e. asynchronous) or the underlying system calls + // may report operations as completed when they have not. + if fdflags & wasi::__WASI_FDFLAG_NONBLOCK != 0 { + flags.insert(Flags::FILE_FLAG_OVERLAPPED); + } + + // Technically, Windows only supports __WASI_FDFLAG_SYNC, but treat all the flags as the same. if fdflags & wasi::__WASI_FDFLAG_DSYNC != 0 || fdflags & wasi::__WASI_FDFLAG_RSYNC != 0 || fdflags & wasi::__WASI_FDFLAG_SYNC != 0 { - access_mode.insert(AccessMode::SYNCHRONIZE); + flags.insert(Flags::FILE_FLAG_WRITE_THROUGH); } (access_mode, flags) diff --git a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs index cba0edc623fe..20640703f1b8 100644 --- a/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/windows/hostcalls_impl/fs.rs @@ -53,10 +53,12 @@ pub(crate) fn fd_pwrite(file: &File, buf: &[u8], offset: wasi::__wasi_filesize_t } pub(crate) fn fd_fdstat_get(fd: &File) -> Result { - use winx::file::AccessMode; - unsafe { winx::file::get_file_access_mode(fd.as_raw_handle()) } - .map(host_impl::fdflags_from_win) - .map_err(Into::into) + unsafe { + Ok(host_impl::fdflags_from_win( + winx::file::query_access_information(fd.as_raw_handle())?, + winx::file::query_mode_information(fd.as_raw_handle())?, + )) + } } pub(crate) fn fd_fdstat_set_flags(fd: &File, fdflags: wasi::__wasi_fdflags_t) -> Result<()> { @@ -100,16 +102,6 @@ pub(crate) fn path_open( ) -> Result { use winx::file::{AccessMode, CreationDisposition, Flags}; - let mut access_mode = AccessMode::READ_CONTROL; - if read { - access_mode.insert(AccessMode::FILE_GENERIC_READ); - } - if write { - access_mode.insert(AccessMode::FILE_GENERIC_WRITE); - } - - let mut flags = Flags::FILE_FLAG_BACKUP_SEMANTICS; - // convert open flags let mut opts = OpenOptions::new(); match host_impl::win_from_oflags(oflags) { @@ -120,16 +112,11 @@ pub(crate) fn path_open( opts.create_new(true).write(true); } CreationDisposition::TRUNCATE_EXISTING => { - opts.truncate(true); + opts.truncate(true).write(true); } _ => {} } - // convert file descriptor flags - let (add_access_mode, add_flags) = host_impl::win_from_fdflags(fdflags); - access_mode.insert(add_access_mode); - flags.insert(add_flags); - let path = resolved.concatenate()?; match path.symlink_metadata().map(|metadata| metadata.file_type()) { @@ -162,6 +149,8 @@ pub(crate) fn path_open( }, } + let (access_mode, flags) = host_impl::win_from_fdflags(fdflags, read, write); + opts.access_mode(access_mode.bits()) .custom_flags(flags.bits()) .open(&path) diff --git a/crates/wasi-common/winx/src/file.rs b/crates/wasi-common/winx/src/file.rs index b72b813813f4..333714a570ac 100644 --- a/crates/wasi-common/winx/src/file.rs +++ b/crates/wasi-common/winx/src/file.rs @@ -5,8 +5,12 @@ use cvt::cvt; use std::ffi::{c_void, OsString}; use std::fs::File; use std::io; +use std::os::raw::c_ulong; use std::os::windows::prelude::{AsRawHandle, OsStringExt, RawHandle}; -use winapi::shared::minwindef::{self, DWORD}; +use winapi::shared::{ + minwindef::{self, DWORD}, + {ntdef::NTSTATUS, ntstatus}, +}; use winapi::um::{fileapi, fileapi::GetFileType, minwinbase, winbase, winnt}; /// Maximum total path length for Unicode in Windows. @@ -291,70 +295,39 @@ bitflags! { /// This is convenience flag which is translated by the OS into actual [`FILE_GENERIC_READ`] union. const GENERIC_READ = winnt::GENERIC_READ; /// Provides read access. - /// This flag is a union of: FILE_READ_ATTRIBUTES, FILE_READ_DATA, FILE_READ_EA, READ_CONTROL, SYNCHRONIZE - const FILE_GENERIC_READ = AccessMode::FILE_READ_ATTRIBUTES.bits - | AccessMode::FILE_READ_DATA.bits - | AccessMode::FILE_READ_EA.bits - | AccessMode::READ_CONTROL.bits - | AccessMode::SYNCHRONIZE.bits; + const FILE_GENERIC_READ = winnt::FILE_GENERIC_READ; /// Provides write access. - /// This flag is a union of: FILE_WRITE_ATTRIBUTES, FILE_WRITE_DATA, FILE_WRITE_EA, READ_CONTROL, SYNCHRONIZE - const FILE_GENERIC_WRITE = AccessMode::FILE_WRITE_ATTRIBUTES.bits - | AccessMode::FILE_WRITE_DATA.bits - | AccessMode::FILE_WRITE_EA.bits - | AccessMode::READ_CONTROL.bits - | AccessMode::SYNCHRONIZE.bits; + const FILE_GENERIC_WRITE = winnt::FILE_GENERIC_WRITE; /// Provides execute access. - /// This flag is a union of: FILE_WRITE_ATTRIBUTES, FILE_WRITE_DATA, FILE_WRITE_EA, READ_CONTROL, SYNCHRONIZE - const FILE_GENERIC_EXECUTE = AccessMode::FILE_EXECUTE.bits - | AccessMode::FILE_READ_ATTRIBUTES.bits - | AccessMode::READ_CONTROL.bits - | AccessMode::SYNCHRONIZE.bits; + const FILE_GENERIC_EXECUTE = winnt::FILE_GENERIC_EXECUTE; /// Provides all accesses. - /// This flag is a union of: FILE_GENERIC_READ, FILE_GENERIC_WRITE, FILE_GENERIC_EXECUTE - const FILE_GENERIC_ALL = AccessMode::FILE_GENERIC_READ.bits | AccessMode::FILE_GENERIC_WRITE.bits | AccessMode::FILE_GENERIC_EXECUTE.bits; + const FILE_ALL_ACCESS = winnt::FILE_ALL_ACCESS; } } -pub unsafe fn get_file_access_mode(handle: RawHandle) -> Result { - use winapi::shared::minwindef::FALSE; - use winapi::um::accctrl; - use winapi::um::aclapi::GetSecurityInfo; - use winapi::um::securitybaseapi::{GetAce, IsValidAcl}; - let mut dacl = 0 as winnt::PACL; - let mut sec_desc = 0 as winnt::PSECURITY_DESCRIPTOR; - - let err = winerror::WinError::from_u32(GetSecurityInfo( - handle, - accctrl::SE_FILE_OBJECT, - winnt::DACL_SECURITY_INFORMATION, - std::ptr::null_mut(), - std::ptr::null_mut(), - &mut dacl, - std::ptr::null_mut(), - &mut sec_desc, - )); - - if err != winerror::WinError::ERROR_SUCCESS { - return Err(err); - } - - if IsValidAcl(dacl) == FALSE { - return Err(winerror::WinError::last()); - } - - // let count = (*dacl).AceCount; - let mut ace = 0 as winnt::PVOID; - - if GetAce(dacl, 0, &mut ace) == FALSE { - return Err(winerror::WinError::last()); +bitflags! { + // https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/52df7798-8330-474b-ac31-9afe8075640c + pub struct FileModeInformation: minwindef::DWORD { + /// When set, any system services, file system drivers (FSDs), and drivers that write data to + /// the file are required to actually transfer the data into the file before any requested write + /// operation is considered complete. + const FILE_WRITE_THROUGH = 0x2; + /// This is a hint that informs the cache that it SHOULD optimize for sequential access. + /// Non-sequential access of the file can result in performance degradation. + const FILE_SEQUENTIAL_ONLY = 0x4; + /// When set, the file cannot be cached or buffered in a driver's internal buffers. + const FILE_NO_INTERMEDIATE_BUFFERING = 0x8; + /// When set, all operations on the file are performed synchronously. + /// Any wait on behalf of the caller is subject to premature termination from alerts. + /// This flag also causes the I/O system to maintain the file position context. + const FILE_SYNCHRONOUS_IO_ALERT = 0x10; + /// When set, all operations on the file are performed synchronously. + /// Wait requests in the system to synchronize I/O queuing and completion are not subject to alerts. + /// This flag also causes the I/O system to maintain the file position context. + const FILE_SYNCHRONOUS_IO_NONALERT = 0x20; + /// This flag is not implemented and is always returned as not set. + const FILE_DELETE_ON_CLOSE = 0x1000; } - - // TODO: check for PACCESS_ALLOWED_ACE in Ace before accessing - // let header = (*(ace as winnt::PACCESS_ALLOWED_ACE)).Header.AceType; - Ok(AccessMode::from_bits_truncate( - (*(ace as winnt::PACCESS_ALLOWED_ACE)).Mask, - )) } pub fn get_file_path(file: &File) -> Result { @@ -415,3 +388,103 @@ pub fn change_time(file: &File) -> io::Result { Ok(tm) } + +pub fn query_access_information(handle: RawHandle) -> Result { + let mut io_status_block = IO_STATUS_BLOCK::default(); + let mut info = FILE_ACCESS_INFORMATION::default(); + + unsafe { + let status = NtQueryInformationFile( + handle, + &mut io_status_block, + &mut info as *mut _ as *mut c_void, + std::mem::size_of::() as u32, + FILE_INFORMATION_CLASS::FileAccessInformation, + ); + + if status != ntstatus::STATUS_SUCCESS { + return Err(winerror::WinError::from_u32(RtlNtStatusToDosError(status))); + } + } + + Ok(AccessMode::from_bits_truncate(info.AccessFlags)) +} + +pub fn query_mode_information(handle: RawHandle) -> Result { + let mut io_status_block = IO_STATUS_BLOCK::default(); + let mut info = FILE_MODE_INFORMATION::default(); + + unsafe { + let status = NtQueryInformationFile( + handle, + &mut io_status_block, + &mut info as *mut _ as *mut c_void, + std::mem::size_of::() as u32, + FILE_INFORMATION_CLASS::FileModeInformation, + ); + + if status != ntstatus::STATUS_SUCCESS { + return Err(winerror::WinError::from_u32(RtlNtStatusToDosError(status))); + } + } + + Ok(FileModeInformation::from_bits_truncate(info.Mode)) +} + +#[repr(C)] +#[derive(Copy, Clone)] +#[allow(non_snake_case)] +enum FILE_INFORMATION_CLASS { + FileAccessInformation = 8, + FileModeInformation = 16, +} + +#[repr(C)] +#[derive(Copy, Clone)] +#[allow(non_snake_case)] +pub union IO_STATUS_BLOCK_u { + Status: NTSTATUS, + Pointer: *mut c_void, +} + +#[repr(C)] +#[derive(Copy, Clone)] +#[allow(non_snake_case)] +struct IO_STATUS_BLOCK { + u: IO_STATUS_BLOCK_u, + Information: *mut c_void, +} + +#[repr(C)] +#[derive(Copy, Clone, Default)] +#[allow(non_snake_case)] +struct FILE_ACCESS_INFORMATION { + AccessFlags: winnt::ACCESS_MASK, +} + +#[repr(C)] +#[derive(Copy, Clone, Default)] +#[allow(non_snake_case)] +struct FILE_MODE_INFORMATION { + Mode: c_ulong, +} + +impl Default for IO_STATUS_BLOCK { + #[inline] + fn default() -> Self { + unsafe { std::mem::zeroed() } + } +} + +#[link(name = "ntdll")] +extern "C" { + fn NtQueryInformationFile( + FileHandle: RawHandle, + IoStatusBlock: *mut IO_STATUS_BLOCK, + FileInformation: *mut c_void, + Length: c_ulong, + FileInformationClass: FILE_INFORMATION_CLASS, + ) -> NTSTATUS; + + fn RtlNtStatusToDosError(status: NTSTATUS) -> c_ulong; +}