From ebd9ca8decd726562acd3d8f6846bcd869adb096 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sun, 17 Jul 2016 00:34:26 +0000 Subject: [PATCH] Allocate guard page under existing stack, not in it. This fixes a segfault when the allocated stack is just one page long. This also refactors the fringe::os module to use Result consistently. close #22 --- Cargo.toml | 2 +- src/os/mod.rs | 33 +++++++++++++-------------------- src/os/sys/unix.rs | 27 ++++++++++++++++++--------- src/os/sys/windows.rs | 32 ++++++++++++++++++++++---------- tests/stack.rs | 13 +++++++++++++ 5 files changed, 67 insertions(+), 40 deletions(-) create mode 100644 tests/stack.rs diff --git a/Cargo.toml b/Cargo.toml index 9f1c2fb..735232e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ name = "fringe" version = "0.0.1" [target.'cfg(unix)'.dependencies] -libc = "0.1.6" +libc = "0.2.14" [target.'cfg(windows)'.dependencies] kernel32-sys = "0.2.2" diff --git a/src/os/mod.rs b/src/os/mod.rs index 56ef6bf..70ccec4 100644 --- a/src/os/mod.rs +++ b/src/os/mod.rs @@ -1,10 +1,10 @@ // This file is part of libfringe, a low-level green threading library. // Copyright (c) edef // See the LICENSE file included in this distribution. - extern crate std; use self::std::io::Error as IoError; use stack; + mod sys; /// This object represents a stack from the operating system's @@ -23,24 +23,22 @@ impl Stack { pub fn new(size: usize) -> Result { let page_size = sys::page_size(); - // round the page size up, - // using the fact that it is a power of two + // Round the length one page size up, using the fact that the page size + // is a power of two. let len = (size + page_size - 1) & !(page_size - 1); - let stack = unsafe { - let ptr = try!(match sys::map_stack(size) { - None => Err(IoError::last_os_error()), - Some(ptr) => Ok(ptr) - }); + // Increase the length to fit the guard page. + let len = len + page_size; - Stack { ptr: ptr as *mut u8, len: len } + // Allocate a stack. + let stack = Stack { + ptr: try!(unsafe { sys::map_stack(len) }), + len: len }; - try!(if unsafe { sys::protect_stack(stack.ptr) } { - Ok(()) - } else { - Err(IoError::last_os_error()) - }); + // Mark the guard page. If this fails, `stack` will be dropped, + // unmapping it. + try!(unsafe { sys::protect_stack(stack.ptr) }); Ok(stack) } @@ -62,11 +60,6 @@ impl stack::Stack for Stack { impl Drop for Stack { fn drop(&mut self) { - unsafe { - if !sys::unmap_stack(self.ptr, self.len) { - panic!("munmap for stack {:p} of size {} failed: {}", - self.ptr, self.len, IoError::last_os_error()) - } - } + unsafe { sys::unmap_stack(self.ptr, self.len) }.expect("cannot unmap stack") } } diff --git a/src/os/sys/unix.rs b/src/os/sys/unix.rs index fd7697e..dfd629c 100644 --- a/src/os/sys/unix.rs +++ b/src/os/sys/unix.rs @@ -1,7 +1,9 @@ // This file is part of libfringe, a low-level green threading library. // Copyright (c) edef // See the LICENSE file included in this distribution. +extern crate std; extern crate libc; +use self::std::io::Error as IoError; use self::libc::{c_void, c_int, size_t}; use self::libc::{mmap, mprotect, munmap}; use self::libc::MAP_FAILED; @@ -29,20 +31,27 @@ const STACK_FLAGS: c_int = libc::MAP_STACK const STACK_FLAGS: c_int = libc::MAP_PRIVATE | libc::MAP_ANON; -pub unsafe fn map_stack(len: usize) -> Option<*mut u8> { - let ptr = mmap(ptr::null_mut(), len as size_t, - STACK_PROT, STACK_FLAGS, -1, 0); +pub unsafe fn map_stack(len: usize) -> Result<*mut u8, IoError> { + let ptr = mmap(ptr::null_mut(), len as size_t, STACK_PROT, STACK_FLAGS, -1, 0); if ptr == MAP_FAILED { - None + Err(IoError::last_os_error()) } else { - Some(ptr as *mut u8) + Ok(ptr as *mut u8) } } -pub unsafe fn protect_stack(ptr: *mut u8) -> bool { - mprotect(ptr as *mut c_void, page_size() as size_t, GUARD_PROT) == 0 +pub unsafe fn protect_stack(ptr: *mut u8) -> Result<(), IoError> { + if mprotect(ptr as *mut c_void, page_size() as size_t, GUARD_PROT) == 0 { + Ok(()) + } else { + Err(IoError::last_os_error()) + } } -pub unsafe fn unmap_stack(ptr: *mut u8, len: usize) -> bool { - munmap(ptr as *mut c_void, len as size_t) == 0 +pub unsafe fn unmap_stack(ptr: *mut u8, len: usize) -> Result<(), IoError> { + if munmap(ptr as *mut c_void, len as size_t) == 0 { + Ok(()) + } else { + Err(IoError::last_os_error()) + } } diff --git a/src/os/sys/windows.rs b/src/os/sys/windows.rs index eb289a7..e2ba7ef 100644 --- a/src/os/sys/windows.rs +++ b/src/os/sys/windows.rs @@ -1,15 +1,17 @@ // This file is part of libfringe, a low-level green threading library. // Copyright (c) edef // See the LICENSE file included in this distribution. +extern crate std; extern crate winapi; extern crate kernel32; -use core::{mem, ptr}; +use self::std::io::Error as IoError; +use self::std::{mem, ptr}; use self::winapi::basetsd::SIZE_T; use self::winapi::minwindef::{DWORD, LPVOID}; use self::winapi::winnt::{MEM_COMMIT, MEM_RESERVE, MEM_RELEASE, PAGE_READWRITE, PAGE_NOACCESS}; use super::page_size; -#[cfg(windows)] +#[cold] pub fn sys_page_size() -> usize { unsafe { let mut info = mem::zeroed(); @@ -18,20 +20,30 @@ pub fn sys_page_size() -> usize { } } -pub unsafe fn map_stack(len: usize) -> Option<*mut u8> { - let ptr = kernel32::VirtualAlloc(ptr::null_mut(), len as SIZE_T, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); +pub unsafe fn map_stack(len: usize) -> Result<*mut u8, IoError> { + let ptr = kernel32::VirtualAlloc(ptr::null_mut(), len as SIZE_T, + MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); if ptr == ptr::null_mut() { - None + Err(IoError::last_os_error()) } else { - Some(ptr as *mut u8) + Ok(ptr as *mut u8) } } -pub unsafe fn protect_stack(ptr: *mut u8) -> bool { +pub unsafe fn protect_stack(ptr: *mut u8) -> Result<(), IoError> { let mut old_prot: DWORD = 0; - kernel32::VirtualProtect(ptr as LPVOID, page_size() as SIZE_T, PAGE_NOACCESS, &mut old_prot) != 0 + if kernel32::VirtualProtect(ptr as LPVOID, page_size() as SIZE_T, + PAGE_NOACCESS, &mut old_prot) == 0 { + Err(IoError::last_os_error()) + } else { + Ok(()) + } } -pub unsafe fn unmap_stack(ptr: *mut u8, _len: usize) -> bool { - kernel32::VirtualFree(ptr as LPVOID, 0, MEM_RELEASE) != 0 +pub unsafe fn unmap_stack(ptr: *mut u8, _len: usize) -> Result<(), IoError> { + if kernel32::VirtualFree(ptr as LPVOID, 0, MEM_RELEASE) == 0 { + Err(IoError::last_os_error()) + } else { + Ok(()) + } } diff --git a/tests/stack.rs b/tests/stack.rs new file mode 100644 index 0000000..9315784 --- /dev/null +++ b/tests/stack.rs @@ -0,0 +1,13 @@ +// This file is part of libfringe, a low-level green threading library. +// Copyright (c) whitequark +// See the LICENSE file included in this distribution. +extern crate fringe; + +use fringe::{Stack, OsStack}; + +#[test] +fn stack_accessible() { + let stack = OsStack::new(4096).unwrap(); + // Make sure the topmost page of the stack, at least, is accessible. + unsafe { *(stack.top().offset(-1)) = 0; } +}