Fix rights checks across the codebase.
* Fix path_open granting more rights than requested * Add missing rights checks in: fd_fdstat_set_flags, fd_filestat_get, poll_oneoff * Fix `open_scratch_directory` not requesting any rights. * Properly request needed rights in various tests * Add some extra trace-level logging * Remove a no-op restriction of rights to the ones returned by `determine_type_rights`. It was redundant, because `FdEntry:from` internally also called `determine_type_rights` and only dropped some of them.
This commit is contained in:
committed by
Dan Gohman
parent
5efa640e23
commit
f7f10c12b3
@@ -155,8 +155,21 @@ impl FdEntry {
|
||||
rights_base: wasi::__wasi_rights_t,
|
||||
rights_inheriting: wasi::__wasi_rights_t,
|
||||
) -> Result<()> {
|
||||
if !self.rights_base & rights_base != 0 || !self.rights_inheriting & rights_inheriting != 0
|
||||
{
|
||||
let missing_base = !self.rights_base & rights_base;
|
||||
let missing_inheriting = !self.rights_inheriting & rights_inheriting;
|
||||
if missing_base != 0 || missing_inheriting != 0 {
|
||||
log::trace!(
|
||||
" | validate_rights failed: required: \
|
||||
rights_base = {:#x}, rights_inheriting = {:#x}; \
|
||||
actual: rights_base = {:#x}, rights_inheriting = {:#x}; \
|
||||
missing_base = {:#x}, missing_inheriting = {:#x}",
|
||||
rights_base,
|
||||
rights_inheriting,
|
||||
self.rights_base,
|
||||
self.rights_inheriting,
|
||||
missing_base,
|
||||
missing_inheriting
|
||||
);
|
||||
Err(Error::ENOTCAPABLE)
|
||||
} else {
|
||||
Ok(())
|
||||
|
||||
@@ -5,7 +5,6 @@ use crate::fdentry::{Descriptor, FdEntry};
|
||||
use crate::helpers::*;
|
||||
use crate::memory::*;
|
||||
use crate::sandboxed_tty_writer::SandboxedTTYWriter;
|
||||
use crate::sys::fdentry_impl::determine_type_rights;
|
||||
use crate::sys::hostcalls_impl::fs_helpers::path_open_rights;
|
||||
use crate::sys::{host_impl, hostcalls_impl};
|
||||
use crate::{helpers, host, wasi, wasi32, Error, Result};
|
||||
@@ -308,7 +307,7 @@ pub(crate) unsafe fn fd_fdstat_set_flags(
|
||||
|
||||
let fd = wasi_ctx
|
||||
.get_fd_entry(fd)?
|
||||
.as_descriptor(0, 0)?
|
||||
.as_descriptor(wasi::__WASI_RIGHTS_FD_FDSTAT_SET_FLAGS, 0)?
|
||||
.as_os_handle();
|
||||
|
||||
hostcalls_impl::fd_fdstat_set_flags(&fd, fdflags)
|
||||
@@ -574,6 +573,11 @@ pub(crate) unsafe fn path_open(
|
||||
|
||||
let (needed_base, needed_inheriting) =
|
||||
path_open_rights(fs_rights_base, fs_rights_inheriting, oflags, fs_flags);
|
||||
trace!(
|
||||
" | needed_base = {}, needed_inheriting = {}",
|
||||
needed_base,
|
||||
needed_inheriting
|
||||
);
|
||||
let fe = wasi_ctx.get_fd_entry(dirfd)?;
|
||||
let resolved = path_get(
|
||||
fe,
|
||||
@@ -593,13 +597,20 @@ pub(crate) unsafe fn path_open(
|
||||
| wasi::__WASI_RIGHTS_FD_FILESTAT_SET_SIZE)
|
||||
!= 0;
|
||||
|
||||
trace!(
|
||||
" | calling path_open impl: read={}, write={}",
|
||||
read,
|
||||
write
|
||||
);
|
||||
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 mut fe = FdEntry::from(fd)?;
|
||||
fe.rights_base &= max_base;
|
||||
fe.rights_inheriting &= max_inheriting;
|
||||
// We need to manually deny the rights which are not explicitly requested.
|
||||
// This should not be needed, but currently determine_type_and_access_rights,
|
||||
// which is used by FdEntry::from, may grant extra rights while inferring it
|
||||
// from the open mode.
|
||||
fe.rights_base &= fs_rights_base;
|
||||
fe.rights_inheriting &= fs_rights_inheriting;
|
||||
let guest_fd = wasi_ctx.insert_fd_entry(fe)?;
|
||||
|
||||
trace!(" | *fd={:?}", guest_fd);
|
||||
@@ -709,7 +720,10 @@ pub(crate) unsafe fn fd_filestat_get(
|
||||
filestat_ptr
|
||||
);
|
||||
|
||||
let fd = wasi_ctx.get_fd_entry(fd)?.as_descriptor(0, 0)?.as_file()?;
|
||||
let fd = wasi_ctx
|
||||
.get_fd_entry(fd)?
|
||||
.as_descriptor(wasi::__WASI_RIGHTS_FD_FILESTAT_GET, 0)?
|
||||
.as_file()?;
|
||||
let host_filestat = hostcalls_impl::fd_filestat_get(fd)?;
|
||||
|
||||
trace!(" | *filestat_ptr={:?}", host_filestat);
|
||||
|
||||
@@ -242,9 +242,9 @@ pub(crate) fn poll_oneoff(
|
||||
{
|
||||
let wasi_fd = unsafe { subscription.u.fd_readwrite.file_descriptor };
|
||||
let rights = if r#type == wasi::__WASI_EVENTTYPE_FD_READ {
|
||||
wasi::__WASI_RIGHTS_FD_READ
|
||||
wasi::__WASI_RIGHTS_FD_READ | wasi::__WASI_RIGHTS_POLL_FD_READWRITE
|
||||
} else {
|
||||
wasi::__WASI_RIGHTS_FD_WRITE
|
||||
wasi::__WASI_RIGHTS_FD_WRITE | wasi::__WASI_RIGHTS_POLL_FD_READWRITE
|
||||
};
|
||||
|
||||
match unsafe {
|
||||
|
||||
Reference in New Issue
Block a user