From a514caa706bbe240d7f01d84b520d22b3aa7949a Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Tue, 30 Apr 2019 21:20:50 +0200 Subject: [PATCH 1/3] Add Rust impl of wasmtime_ssp_fd_prestat_get In more detail, this commit: * makes fd_prestat_get safe * rewrites fd_prestats_get_entry in (safe) Rust * creates helper macros for rwlock read lock and unlock --- wasmtime-wasi/build.rs | 2 + .../include/wasmtime_ssp.h | 8 --- .../sandboxed-system-primitives/src/locking.h | 8 +-- .../sandboxed-system-primitives/src/posix.c | 30 ---------- .../sandboxed-system-primitives/src/posix.h | 5 +- wasmtime-wasi/src/host_impls.rs | 56 +++++++++++++++++++ wasmtime-wasi/src/syscalls.rs | 2 +- 7 files changed, 67 insertions(+), 44 deletions(-) diff --git a/wasmtime-wasi/build.rs b/wasmtime-wasi/build.rs index 46d7058caf6b..fa9d79f17fd0 100644 --- a/wasmtime-wasi/build.rs +++ b/wasmtime-wasi/build.rs @@ -20,8 +20,10 @@ fn main() { .whitelist_function("fd_prestats_init") .whitelist_function("fd_prestats_insert") .whitelist_function("argv_environ_init") + .whitelist_function("rwlock_.*") .whitelist_type("__wasi_.*") .whitelist_type("fd_table") + .whitelist_type("fd_prestat") .whitelist_type("fd_prestats") .whitelist_type("argv_environ_values") .whitelist_var("__WASI_.*") diff --git a/wasmtime-wasi/sandboxed-system-primitives/include/wasmtime_ssp.h b/wasmtime-wasi/sandboxed-system-primitives/include/wasmtime_ssp.h index 7410161ee1bc..660c4b2538ae 100644 --- a/wasmtime-wasi/sandboxed-system-primitives/include/wasmtime_ssp.h +++ b/wasmtime-wasi/sandboxed-system-primitives/include/wasmtime_ssp.h @@ -497,14 +497,6 @@ __wasi_errno_t wasmtime_ssp_environ_sizes_get( size_t *environ_buf_size ) WASMTIME_SSP_SYSCALL_NAME(environ_sizes_get) __attribute__((__warn_unused_result__)); -__wasi_errno_t wasmtime_ssp_fd_prestat_get( -#if !defined(WASMTIME_SSP_STATIC_CURFDS) - struct fd_prestats *prestats, -#endif - __wasi_fd_t fd, - __wasi_prestat_t *buf -) WASMTIME_SSP_SYSCALL_NAME(fd_prestat_get) __attribute__((__warn_unused_result__)); - __wasi_errno_t wasmtime_ssp_fd_prestat_dir_name( #if !defined(WASMTIME_SSP_STATIC_CURFDS) struct fd_prestats *prestats, diff --git a/wasmtime-wasi/sandboxed-system-primitives/src/locking.h b/wasmtime-wasi/sandboxed-system-primitives/src/locking.h index 0efb788ed5e4..3c606235ebb3 100644 --- a/wasmtime-wasi/sandboxed-system-primitives/src/locking.h +++ b/wasmtime-wasi/sandboxed-system-primitives/src/locking.h @@ -83,21 +83,21 @@ struct LOCKABLE rwlock { pthread_rwlock_t object; }; -static inline void rwlock_init(struct rwlock *lock) REQUIRES_UNLOCKED(*lock) { +void rwlock_init(struct rwlock *lock) REQUIRES_UNLOCKED(*lock) { pthread_rwlock_init(&lock->object, NULL); } -static inline void rwlock_rdlock(struct rwlock *lock) +void rwlock_rdlock(struct rwlock *lock) LOCKS_SHARED(*lock) NO_LOCK_ANALYSIS { pthread_rwlock_rdlock(&lock->object); } -static inline void rwlock_wrlock(struct rwlock *lock) +void rwlock_wrlock(struct rwlock *lock) LOCKS_EXCLUSIVE(*lock) NO_LOCK_ANALYSIS { pthread_rwlock_wrlock(&lock->object); } -static inline void rwlock_unlock(struct rwlock *lock) +void rwlock_unlock(struct rwlock *lock) UNLOCKS(*lock) NO_LOCK_ANALYSIS { pthread_rwlock_unlock(&lock->object); } diff --git a/wasmtime-wasi/sandboxed-system-primitives/src/posix.c b/wasmtime-wasi/sandboxed-system-primitives/src/posix.c index 6f7ba8ab3d15..31d379a1b394 100644 --- a/wasmtime-wasi/sandboxed-system-primitives/src/posix.c +++ b/wasmtime-wasi/sandboxed-system-primitives/src/posix.c @@ -243,10 +243,6 @@ __wasi_errno_t wasmtime_ssp_clock_time_get( return 0; } -struct fd_prestat { - const char *dir; -}; - void fd_prestats_init( struct fd_prestats *pt ) { @@ -653,32 +649,6 @@ static __wasi_errno_t fd_table_insert_fd( return fd_table_insert(ft, fo, rights_base, rights_inheriting, out); } -__wasi_errno_t wasmtime_ssp_fd_prestat_get( -#if !defined(WASMTIME_SSP_STATIC_CURFDS) - struct fd_prestats *prestats, -#endif - __wasi_fd_t fd, - __wasi_prestat_t *buf -) { - rwlock_rdlock(&prestats->lock); - struct fd_prestat *prestat; - __wasi_errno_t error = fd_prestats_get_entry(prestats, fd, &prestat); - if (error != 0) { - rwlock_unlock(&prestats->lock); - return error; - } - - *buf = (__wasi_prestat_t) { - .pr_type = __WASI_PREOPENTYPE_DIR, - }; - - buf->u.dir.pr_name_len = strlen(prestat->dir); - - rwlock_unlock(&prestats->lock); - - return 0; -} - __wasi_errno_t wasmtime_ssp_fd_prestat_dir_name( #if !defined(WASMTIME_SSP_STATIC_CURFDS) struct fd_prestats *prestats, diff --git a/wasmtime-wasi/sandboxed-system-primitives/src/posix.h b/wasmtime-wasi/sandboxed-system-primitives/src/posix.h index 6711c9b2e37f..b3519da78a63 100644 --- a/wasmtime-wasi/sandboxed-system-primitives/src/posix.h +++ b/wasmtime-wasi/sandboxed-system-primitives/src/posix.h @@ -18,9 +18,12 @@ #include "locking.h" struct fd_entry; -struct fd_prestat; struct syscalls; +struct fd_prestat { + const char *dir; +}; + struct fd_table { struct rwlock lock; struct fd_entry *entries; diff --git a/wasmtime-wasi/src/host_impls.rs b/wasmtime-wasi/src/host_impls.rs index a509ba25828d..d614d6d0b159 100644 --- a/wasmtime-wasi/src/host_impls.rs +++ b/wasmtime-wasi/src/host_impls.rs @@ -88,10 +88,66 @@ fn convert_errno(error: Errno) -> host::__wasi_errno_t { } } +fn fd_prestats_get_entry( + pt: &host::fd_prestats, + fd: host::__wasi_fd_t, +) -> Option<&host::fd_prestat> { + // Test for file descriptor existence + if fd as usize >= pt.size { + return None; + } + + let prestat = unsafe { &*pt.prestats.add(fd as usize) }; + if prestat.dir == ::std::ptr::null() { + return None; + } + + Some(prestat) +} + +macro_rules! rwlock_rdlock { + ($prestats:expr) => { + unsafe { + host::rwlock_rdlock(&mut (*$prestats).lock as *mut host::rwlock); + } + }; +} + +macro_rules! rwlock_unlock { + ($prestats:expr) => { + unsafe { + host::rwlock_unlock(&mut (*$prestats).lock as *mut host::rwlock); + } + }; +} + pub fn wasmtime_ssp_proc_exit(rval: wasm32::__wasi_exitcode_t) { ::std::process::exit(rval as i32) } +pub fn wasmtime_ssp_fd_prestat_get( + prestats: &mut host::fd_prestats, + fd: host::__wasi_fd_t, + buf: &mut host::__wasi_prestat_t, +) -> host::__wasi_errno_t { + rwlock_rdlock!(prestats); + + let ret_code = if let Some(prestat) = fd_prestats_get_entry(prestats, fd) { + (*buf).pr_type = host::__WASI_PREOPENTYPE_DIR as host::__wasi_preopentype_t; + unsafe { + let dir_name = ::std::ffi::CStr::from_ptr((*prestat).dir).to_str().unwrap(); + (*buf).u.dir.pr_name_len = dir_name.len(); + } + host::__WASI_ESUCCESS + } else { + host::__WASI_EBADF + }; + + rwlock_unlock!(prestats); + + ret_code +} + pub fn wasmtime_ssp_sched_yield() -> host::__wasi_errno_t { unsafe { if libc::sched_yield() < 0 { diff --git a/wasmtime-wasi/src/syscalls.rs b/wasmtime-wasi/src/syscalls.rs index 04241500ade1..0151095976e4 100644 --- a/wasmtime-wasi/src/syscalls.rs +++ b/wasmtime-wasi/src/syscalls.rs @@ -380,7 +380,7 @@ syscalls! { return return_encoded_errno(e); } - let e = host::wasmtime_ssp_fd_prestat_get(prestats, fd, &mut host_buf); + let e = host_impls::wasmtime_ssp_fd_prestat_get(&mut *prestats, fd, &mut host_buf); encode_prestat_byref(vmctx, buf, host_buf); From 7bd6db6400c19c3a70fdcf512775536afc751844 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 1 May 2019 15:23:28 +0200 Subject: [PATCH 2/3] Port host::__wasi_preopentype_t to Rust --- wasmtime-wasi/build.rs | 3 ++- wasmtime-wasi/src/host.rs | 3 +++ wasmtime-wasi/src/host_impls.rs | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/wasmtime-wasi/build.rs b/wasmtime-wasi/build.rs index fa9d79f17fd0..1ed59201b4ab 100644 --- a/wasmtime-wasi/build.rs +++ b/wasmtime-wasi/build.rs @@ -103,7 +103,8 @@ fn main() { .blacklist_item("__WASI_ETIMEDOUT") .blacklist_item("__WASI_ETXTBSY") .blacklist_item("__WASI_EXDEV") - .blacklist_item("__WASI_ENOTCAPABLE"); + .blacklist_item("__WASI_ENOTCAPABLE") + .blacklist_item("__WASI_PREOPENTYPE_DIR"); let out_path = PathBuf::from(env::var("OUT_DIR").unwrap()); diff --git a/wasmtime-wasi/src/host.rs b/wasmtime-wasi/src/host.rs index 1b83c5491eea..afd01bc1637d 100644 --- a/wasmtime-wasi/src/host.rs +++ b/wasmtime-wasi/src/host.rs @@ -83,3 +83,6 @@ pub const __WASI_ETIMEDOUT: __wasi_errno_t = 73; pub const __WASI_ETXTBSY: __wasi_errno_t = 74; pub const __WASI_EXDEV: __wasi_errno_t = 75; pub const __WASI_ENOTCAPABLE: __wasi_errno_t = 76; + +// pub type __wasi_preopentype_t = u8; +pub const __WASI_PREOPENTYPE_DIR: __wasi_preopentype_t = 0; diff --git a/wasmtime-wasi/src/host_impls.rs b/wasmtime-wasi/src/host_impls.rs index d614d6d0b159..eb0bae29e083 100644 --- a/wasmtime-wasi/src/host_impls.rs +++ b/wasmtime-wasi/src/host_impls.rs @@ -133,7 +133,7 @@ pub fn wasmtime_ssp_fd_prestat_get( rwlock_rdlock!(prestats); let ret_code = if let Some(prestat) = fd_prestats_get_entry(prestats, fd) { - (*buf).pr_type = host::__WASI_PREOPENTYPE_DIR as host::__wasi_preopentype_t; + (*buf).pr_type = host::__WASI_PREOPENTYPE_DIR; unsafe { let dir_name = ::std::ffi::CStr::from_ptr((*prestat).dir).to_str().unwrap(); (*buf).u.dir.pr_name_len = dir_name.len(); From 2dd8708058d0adb0460707dbba94ac26287531fd Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 1 May 2019 17:05:59 +0200 Subject: [PATCH 3/3] Modify fd_prestat to hold C-str together with its length --- .../sandboxed-system-primitives/src/posix.c | 15 +++++++++------ .../sandboxed-system-primitives/src/posix.h | 3 ++- wasmtime-wasi/src/host_impls.rs | 7 +++---- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/wasmtime-wasi/sandboxed-system-primitives/src/posix.c b/wasmtime-wasi/sandboxed-system-primitives/src/posix.c index 31d379a1b394..95b5846009b8 100644 --- a/wasmtime-wasi/sandboxed-system-primitives/src/posix.c +++ b/wasmtime-wasi/sandboxed-system-primitives/src/posix.c @@ -274,8 +274,10 @@ static bool fd_prestats_grow( return false; // Mark all new file descriptors as unused. - for (size_t i = pt->size; i < size; ++i) - prestats[i].dir = NULL; + for (size_t i = pt->size; i < size; ++i) { + prestats[i].dir_name = NULL; + prestats[i].dir_name_len = 0; + } pt->prestats = prestats; pt->size = size; } @@ -295,7 +297,8 @@ bool fd_prestats_insert( return false; } - pt->prestats[fd].dir = strdup(dir); + pt->prestats[fd].dir_name = strdup(dir); + pt->prestats[fd].dir_name_len = strlen(dir); rwlock_unlock(&pt->lock); return true; } @@ -310,7 +313,7 @@ static __wasi_errno_t fd_prestats_get_entry( if (fd >= pt->size) return __WASI_EBADF; struct fd_prestat *prestat = &pt->prestats[fd]; - if (prestat->dir == NULL) + if (prestat->dir_name == NULL) return __WASI_EBADF; *ret = prestat; @@ -664,12 +667,12 @@ __wasi_errno_t wasmtime_ssp_fd_prestat_dir_name( rwlock_unlock(&prestats->lock); return error; } - if (path_len != strlen(prestat->dir)) { + if (path_len != prestat->dir_name_len) { rwlock_unlock(&prestats->lock); return EINVAL; } - memcpy(path, prestat->dir, path_len); + memcpy(path, prestat->dir_name, path_len); rwlock_unlock(&prestats->lock); diff --git a/wasmtime-wasi/sandboxed-system-primitives/src/posix.h b/wasmtime-wasi/sandboxed-system-primitives/src/posix.h index b3519da78a63..6eb7011f7abc 100644 --- a/wasmtime-wasi/sandboxed-system-primitives/src/posix.h +++ b/wasmtime-wasi/sandboxed-system-primitives/src/posix.h @@ -21,7 +21,8 @@ struct fd_entry; struct syscalls; struct fd_prestat { - const char *dir; + const char *dir_name; + size_t dir_name_len; }; struct fd_table { diff --git a/wasmtime-wasi/src/host_impls.rs b/wasmtime-wasi/src/host_impls.rs index eb0bae29e083..c73b1e845a14 100644 --- a/wasmtime-wasi/src/host_impls.rs +++ b/wasmtime-wasi/src/host_impls.rs @@ -98,7 +98,7 @@ fn fd_prestats_get_entry( } let prestat = unsafe { &*pt.prestats.add(fd as usize) }; - if prestat.dir == ::std::ptr::null() { + if prestat.dir_name == ::std::ptr::null() { return None; } @@ -133,10 +133,9 @@ pub fn wasmtime_ssp_fd_prestat_get( rwlock_rdlock!(prestats); let ret_code = if let Some(prestat) = fd_prestats_get_entry(prestats, fd) { - (*buf).pr_type = host::__WASI_PREOPENTYPE_DIR; + buf.pr_type = host::__WASI_PREOPENTYPE_DIR; unsafe { - let dir_name = ::std::ffi::CStr::from_ptr((*prestat).dir).to_str().unwrap(); - (*buf).u.dir.pr_name_len = dir_name.len(); + buf.u.dir.pr_name_len = prestat.dir_name_len; } host::__WASI_ESUCCESS } else {