Skip to content

Commit

Permalink
Merge #2241
Browse files Browse the repository at this point in the history 
2241: Fix UB in setting memory in emscripten EmEnv r=syrusakbary a=MarkMcCaskey

Fixes the UB and another issue caused by the reftypes PR merging in

Resolves #2122

Co-authored-by: Mark McCaskey <[email protected]>
  • Loading branch information
@bors
bors[bot] and Mark McCaskey authored Apr 16, 2021
2 parents 0daa7e2 + 065bfb3 commit 8d5c7bc
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 65 deletions.
33 changes: 14 additions & 19 deletions lib/emscripten/src/env/unix/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,11 @@ pub fn _gai_strerror(ctx: &EmEnv, ecode: i32) -> i32 {
let cstr = unsafe { std::ffi::CStr::from_ptr(libc::gai_strerror(ecode)) };
let bytes = cstr.to_bytes_with_nul();
let string_on_guest: WasmPtr<c_char, Array> = call_malloc_with_cast(ctx, bytes.len() as _);
let memory = ctx.memory(0);

let writer = unsafe {
string_on_guest
.deref_mut(ctx.memory(0), 0, bytes.len() as _)
.deref_mut(&memory, 0, bytes.len() as _)
.unwrap()
};
for (i, byte) in bytes.iter().enumerate() {
Expand All @@ -175,7 +176,7 @@ pub fn _getaddrinfo(
let memory = ctx.memory(0);
debug!(" => node = {}", unsafe {
node_ptr
.deref(memory)
.deref(&memory)
.map(|np| {
std::ffi::CStr::from_ptr(np as *const Cell<c_char> as *const c_char)
.to_string_lossy()
Expand All @@ -184,15 +185,15 @@ pub fn _getaddrinfo(
});
debug!(" => server_str = {}", unsafe {
service_str_ptr
.deref(memory)
.deref(&memory)
.map(|np| {
std::ffi::CStr::from_ptr(np as *const Cell<c_char> as *const c_char)
.to_string_lossy()
})
.unwrap_or(std::borrow::Cow::Borrowed("null"))
});

let hints = hints_ptr.deref(memory).map(|hints_memory| {
let hints = hints_ptr.deref(&memory).map(|hints_memory| {
let hints_guest = hints_memory.get();
addrinfo {
ai_flags: hints_guest.ai_flags,
Expand All @@ -212,11 +213,11 @@ pub fn _getaddrinfo(
let result = unsafe {
libc::getaddrinfo(
(node_ptr
.deref(memory)
.deref(&memory)
.map(|m| m as *const Cell<c_char> as *const c_char))
.unwrap_or(std::ptr::null()),
service_str_ptr
.deref(memory)
.deref(&memory)
.map(|m| m as *const Cell<c_char> as *const c_char)
.unwrap_or(std::ptr::null()),
hints
Expand Down Expand Up @@ -245,7 +246,7 @@ pub fn _getaddrinfo(

// connect list
if let Some(prev_guest) = previous_guest_node {
let mut pg = prev_guest.deref_mut(ctx.memory(0)).unwrap().get_mut();
let mut pg = prev_guest.deref_mut(&memory).unwrap().get_mut();
pg.ai_next = current_guest_node_ptr;
}

Expand All @@ -257,10 +258,7 @@ pub fn _getaddrinfo(
let host_sockaddr_ptr = (*current_host_node).ai_addr;
let guest_sockaddr_ptr: WasmPtr<EmSockAddr> =
call_malloc_with_cast(ctx, host_addrlen as _);
let guest_sockaddr = guest_sockaddr_ptr
.deref_mut(ctx.memory(0))
.unwrap()
.get_mut();
let guest_sockaddr = guest_sockaddr_ptr.deref_mut(&memory).unwrap().get_mut();

guest_sockaddr.sa_family = (*host_sockaddr_ptr).sa_family as i16;
guest_sockaddr.sa_data = (*host_sockaddr_ptr).sa_data;
Expand All @@ -277,9 +275,8 @@ pub fn _getaddrinfo(
let guest_canonname: WasmPtr<c_char, Array> =
call_malloc_with_cast(ctx, str_size as _);

let guest_canonname_writer = guest_canonname
.deref(ctx.memory(0), 0, str_size as _)
.unwrap();
let guest_canonname_writer =
guest_canonname.deref(&memory, 0, str_size as _).unwrap();
for (i, b) in canonname_bytes.into_iter().enumerate() {
guest_canonname_writer[i].set(*b as _)
}
Expand All @@ -290,10 +287,8 @@ pub fn _getaddrinfo(
}
};

let mut current_guest_node = current_guest_node_ptr
.deref_mut(ctx.memory(0))
.unwrap()
.get_mut();
let mut current_guest_node =
current_guest_node_ptr.deref_mut(&memory).unwrap().get_mut();
current_guest_node.ai_flags = (*current_host_node).ai_flags;
current_guest_node.ai_family = (*current_host_node).ai_family;
current_guest_node.ai_socktype = (*current_host_node).ai_socktype;
Expand All @@ -311,7 +306,7 @@ pub fn _getaddrinfo(
head_of_list.unwrap_or_else(|| WasmPtr::new(0))
};

res_val_ptr.deref(ctx.memory(0)).unwrap().set(head_of_list);
res_val_ptr.deref(&memory).unwrap().set(head_of_list);

0
}
20 changes: 13 additions & 7 deletions lib/emscripten/src/env/windows/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ extern "C" {
/// emscripten: _getenv // (name: *const char) -> *const c_char;
pub fn _getenv(ctx: &EmEnv, name: u32) -> u32 {
debug!("emscripten::_getenv");
let name_string = read_string_from_wasm(ctx.memory(0), name);
let memory = ctx.memory(0);
let name_string = read_string_from_wasm(&memory, name);
debug!("=> name({:?})", name_string);
let c_str = unsafe { getenv(name_string.as_ptr() as *const libc::c_char) };
if c_str.is_null() {
Expand All @@ -31,9 +32,10 @@ pub fn _getenv(ctx: &EmEnv, name: u32) -> u32 {
/// emscripten: _setenv // (name: *const char, name: *const value, overwrite: int);
pub fn _setenv(ctx: &EmEnv, name: u32, value: u32, _overwrite: u32) -> c_int {
debug!("emscripten::_setenv");
let memory = ctx.memory(0);
// setenv does not exist on windows, so we hack it with _putenv
let name = read_string_from_wasm(ctx.memory(0), name);
let value = read_string_from_wasm(ctx.memory(0), value);
let name = read_string_from_wasm(&memory, name);
let value = read_string_from_wasm(&memory, value);
let putenv_string = format!("{}={}", name, value);
let putenv_cstring = CString::new(putenv_string).unwrap();
let putenv_raw_ptr = putenv_cstring.as_ptr();
Expand All @@ -45,7 +47,8 @@ pub fn _setenv(ctx: &EmEnv, name: u32, value: u32, _overwrite: u32) -> c_int {
/// emscripten: _putenv // (name: *const char);
pub fn _putenv(ctx: &EmEnv, name: c_int) -> c_int {
debug!("emscripten::_putenv");
let name_addr = emscripten_memory_pointer!(ctx.memory(0), name) as *const c_char;
let memory = ctx.memory(0);
let name_addr = emscripten_memory_pointer!(&memory, name) as *const c_char;
debug!("=> name({:?})", unsafe {
std::ffi::CStr::from_ptr(name_addr)
});
Expand All @@ -55,7 +58,8 @@ pub fn _putenv(ctx: &EmEnv, name: c_int) -> c_int {
/// emscripten: _unsetenv // (name: *const char);
pub fn _unsetenv(ctx: &EmEnv, name: u32) -> c_int {
debug!("emscripten::_unsetenv");
let name = read_string_from_wasm(ctx.memory(0), name);
let memory = ctx.memory(0);
let name = read_string_from_wasm(&memory, name);
// no unsetenv on windows, so use putenv with an empty value
let unsetenv_string = format!("{}=", name);
let unsetenv_cstring = CString::new(unsetenv_string).unwrap();
Expand All @@ -69,6 +73,7 @@ pub fn _getpwnam(ctx: &EmEnv, name_ptr: c_int) -> c_int {
debug!("emscripten::_getpwnam {}", name_ptr);
#[cfg(not(feature = "debug"))]
let _ = name_ptr;
let memory = ctx.memory(0);

#[repr(C)]
struct GuestPasswd {
Expand All @@ -85,7 +90,7 @@ pub fn _getpwnam(ctx: &EmEnv, name_ptr: c_int) -> c_int {
unsafe {
let passwd_struct_offset = call_malloc(ctx, mem::size_of::<GuestPasswd>() as _);
let passwd_struct_ptr =
emscripten_memory_pointer!(ctx.memory(0), passwd_struct_offset) as *mut GuestPasswd;
emscripten_memory_pointer!(&memory, passwd_struct_offset) as *mut GuestPasswd;
(*passwd_struct_ptr).pw_name = 0;
(*passwd_struct_ptr).pw_passwd = 0;
(*passwd_struct_ptr).pw_gecos = 0;
Expand All @@ -103,6 +108,7 @@ pub fn _getgrnam(ctx: &EmEnv, name_ptr: c_int) -> c_int {
debug!("emscripten::_getgrnam {}", name_ptr);
#[cfg(not(feature = "debug"))]
let _ = name_ptr;
let memory = ctx.memory(0);

#[repr(C)]
struct GuestGroup {
Expand All @@ -116,7 +122,7 @@ pub fn _getgrnam(ctx: &EmEnv, name_ptr: c_int) -> c_int {
unsafe {
let group_struct_offset = call_malloc(ctx, mem::size_of::<GuestGroup>() as _);
let group_struct_ptr =
emscripten_memory_pointer!(ctx.memory(0), group_struct_offset) as *mut GuestGroup;
emscripten_memory_pointer!(&memory, group_struct_offset) as *mut GuestGroup;
(*group_struct_ptr).gr_name = 0;
(*group_struct_ptr).gr_passwd = 0;
(*group_struct_ptr).gr_gid = 0;
Expand Down
19 changes: 8 additions & 11 deletions lib/emscripten/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use lazy_static::lazy_static;
use std::collections::HashMap;
use std::f64;
use std::path::PathBuf;
use std::sync::{Arc, Mutex};
use std::sync::{Arc, Mutex, RwLock};
use wasmer::{
imports, namespace, Exports, Function, FunctionType, Global, ImportObject, Instance, LazyInit,
Memory, MemoryType, Module, NativeFunc, Pages, RuntimeError, Store, Table, TableType, Val,
Expand Down Expand Up @@ -71,7 +71,7 @@ pub use self::utils::{
#[derive(Clone)]
/// The environment provided to the Emscripten imports.
pub struct EmEnv {
memory: Arc<Option<Memory>>,
memory: Arc<RwLock<Option<Memory>>>,
data: Arc<Mutex<EmscriptenData>>,
}

Expand All @@ -86,21 +86,19 @@ impl WasmerEnv for EmEnv {
impl EmEnv {
pub fn new(data: &EmscriptenGlobalsData, mapped_dirs: HashMap<String, PathBuf>) -> Self {
Self {
memory: Arc::new(None),
memory: Arc::new(RwLock::new(None)),
data: Arc::new(Mutex::new(EmscriptenData::new(data.clone(), mapped_dirs))),
}
}

pub fn set_memory(&mut self, memory: Memory) {
let ptr = Arc::as_ptr(&self.memory) as *mut _;
unsafe {
*ptr = Some(memory);
}
let mut w = self.memory.write().unwrap();
*w = Some(memory);
}

/// Get a reference to the memory
pub fn memory(&self, _mem_idx: u32) -> &Memory {
(*self.memory).as_ref().unwrap()
pub fn memory(&self, _mem_idx: u32) -> Memory {
(&*self.memory.read().unwrap()).as_ref().cloned().unwrap()
}
}

Expand Down Expand Up @@ -477,8 +475,7 @@ impl EmscriptenGlobals {
minimum: table_min,
maximum: table_max,
};
// TODO: review init value
let table = Table::new(store, table_type, Val::null()).unwrap();
let table = Table::new(store, table_type, Val::FuncRef(None)).unwrap();

let data = {
let static_bump = STATIC_BUMP;
Expand Down
6 changes: 4 additions & 2 deletions lib/emscripten/src/syscalls/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,9 @@ pub fn ___syscall183(ctx: &EmEnv, _which: c_int, mut varargs: VarArgs) -> i32 {
let path = get_current_directory(ctx);
let path_string = path.unwrap().display().to_string();
let len = path_string.len();
let memory = ctx.memory(0);

let buf_writer = buf_offset.deref(ctx.memory(0), 0, len as u32 + 1).unwrap();
let buf_writer = buf_offset.deref(&memory, 0, len as u32 + 1).unwrap();
for (i, byte) in path_string.bytes().enumerate() {
buf_writer[i].set(byte as _);
}
Expand Down Expand Up @@ -411,8 +412,9 @@ pub fn ___syscall140(ctx: &EmEnv, _which: i32, mut varargs: VarArgs) -> i32 {
let whence: i32 = varargs.get(ctx);
let offset = offset_low;
let ret = unsafe { lseek(fd, offset as _, whence) as i64 };
let memory = ctx.memory(0);

let result_ptr = result_ptr_value.deref(ctx.memory(0)).unwrap();
let result_ptr = result_ptr_value.deref(&memory).unwrap();
result_ptr.set(ret);

debug!(
Expand Down
Loading

0 comments on commit 8d5c7bc

Please sign in to comment.