From 48424da89bd02889347a3416e18bf642ca65ffd4 Mon Sep 17 00:00:00 2001 From: Victor Berger Date: Mon, 13 Feb 2017 08:38:08 +0100 Subject: [PATCH 1/8] shm: API & wayland structure of the code shm/pool.rs is left WIP and will handle the actual memory-map logic. --- Cargo.toml | 2 +- src/lib.rs | 5 + src/shm/mod.rs | 242 ++++++++++++++++++++++++++++++++++++++++++++++++ src/shm/pool.rs | 17 ++++ 4 files changed, 265 insertions(+), 1 deletion(-) create mode 100644 src/shm/mod.rs create mode 100644 src/shm/pool.rs diff --git a/Cargo.toml b/Cargo.toml index f1b8067..0e4ddf7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,4 +4,4 @@ version = "0.1.0" authors = ["Victor Berger "] [dependencies] -wayland-server = "0.7.6" +wayland-server = "0.8.4" diff --git a/src/lib.rs b/src/lib.rs index 7b1e487..f6df6e7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1 +1,6 @@ +#![warn(missing_docs)] + +#[macro_use] extern crate wayland_server; + +pub mod shm; diff --git a/src/shm/mod.rs b/src/shm/mod.rs new file mode 100644 index 0000000..ee9a7bd --- /dev/null +++ b/src/shm/mod.rs @@ -0,0 +1,242 @@ +//! SHM handling helpers +//! +//! This module provides helpers to handle SHM-based buffers from wayland clients. +//! +//! To use it, first add a `ShmGlobal` to your event loop, specifying the formats +//! you want to support (ARGB8888 and XRGB8888 are always considered as supported, +//! as specified by the wayland protocol) and obtain its `ShmGlobalToken`. +//! +//! ```ignore +//! let handler_id = event_loop.add_handler(ShmGlobal::new()); +//! let shm_global = event_loop.register_global::(handler_id, 3); +//! let shm_token = { +//! let state = event_loop.state(); +//! state.get_handler::(handler_id).get_token() +//! }; +//! ``` +//! +//! Then, when you have a `WlBuffer` and need to retrieve its contents, use the token method to +//! do it: +//! +//! ```ignore +//! shm_token.with_buffer_contents(&buffer, +//! |slice: &[u8], buffer_metadata: &BufferData| { +//! // do something to draw it on the screen +//! } +//! ); +//! ``` + +use std::os::unix::io::RawFd; +use std::sync::Arc; + +use wayland_server::{GlobalHandler, EventLoopHandle, Client, Init, Resource, Destroy, resource_is_registered}; +use wayland_server::protocol::{wl_shm, wl_shm_pool, wl_buffer}; + +use self::pool::Pool; + +mod pool; + +/// A global for handling SHM pool and buffers +/// +/// You must register it to an event loop using `register_with_init`, or it will +/// quickly panic. +pub struct ShmGlobal { + formats: Vec, + handler_id: Option +} + +impl ShmGlobal { + /// Create a new SHM global advertizing given supported formats. + /// + /// This global will always advertize `ARGB8888` and `XRGB8888` format + /// as they are required by the protocol. Formats given as argument + /// as additionnaly advertized. + pub fn new(mut formats: Vec) -> ShmGlobal { + // always add the mandatory formats + formats.push(wl_shm::Format::Argb8888); + formats.push(wl_shm::Format::Xrgb8888); + ShmGlobal { + formats: formats, + handler_id: None + } + } + + /// Retreive a token from the SHM global. + /// + /// This can only be called once the `ShmGlobal` has been added to and event loop + /// and has been initialized. If it is not the case, this method will panic. + /// + /// This is needed to retrieve the contents of the shm pools and buffers. + pub fn get_token(&self) -> ShmGlobalToken { + ShmGlobalToken { + hid: self.handler_id.clone().expect("ShmGlobal was not initialized.") + } + } +} + +/// An SHM global token +/// +/// It is needed to access the contents of the buffers & pools managed by the +/// associated ShmGlobal. +pub struct ShmGlobalToken { + hid: usize +} + +/// Error that can occur when accessing an SHM buffer +pub enum BufferAccessError { + /// This buffer is not managed by the SHM handler + NotManaged, + /// An error occured while accessing the memory map + /// + /// This can happen if the client advertized a wrong size + /// for the memory map. + /// + /// If this error occurs, the client has been killed as a result. + BadMap +} + +impl ShmGlobalToken { + /// Call given closure with the contents of the given buffer + /// + /// If the buffer is managed by the associated ShmGlobal, its contents are + /// extracted and the closure is extracted with them: + /// + /// - The first argument is a data slice of the contents of the pool + /// - The second argument is the specification of this buffer is this pool + /// + /// If the buffer is not managed by the associated ShmGlobal, the closure is not called + /// and this method will return `Err(())` (this will be the case for an EGL buffer for example). + pub fn with_buffer_contents(&self, buffer: &wl_buffer::WlBuffer, f: F) -> Result<(), BufferAccessError> + where F: FnOnce(&[u8], BufferData) + { + if !resource_is_registered::<_, ShmHandler>(buffer, self.hid) { + return Err(BufferAccessError::NotManaged) + } + let data = unsafe { &* (buffer.get_user_data() as *mut InternalBufferData) }; + + if data.pool.with_data_slice(|slice| f(slice, data.data.clone())).is_err() { + // SIGBUS error occured + return Err(BufferAccessError::BadMap) + } + Ok(()) + } +} + +impl Init for ShmGlobal { + fn init(&mut self, evqh: &mut EventLoopHandle, _index: usize) { + let id = evqh.add_handler_with_init(ShmHandler { + my_id: ::std::usize::MAX, + valid_formats: self.formats.clone() + }); + self.handler_id = Some(id); + } +} + +impl GlobalHandler for ShmGlobal { + fn bind(&mut self, evqh: &mut EventLoopHandle, _: &Client, global: wl_shm::WlShm) { + let hid = self.handler_id.clone().expect("ShmGlobal was not initialized."); + // register an handler for this shm + evqh.register::<_, ShmHandler>(&global, hid); + // and then the custom formats + for f in &self.formats { + global.format(*f); + } + } +} + +struct ShmHandler { + my_id: usize, + valid_formats: Vec +} + +impl Init for ShmHandler { + fn init(&mut self, _evqh: &mut EventLoopHandle, index: usize) { + self.my_id = index; + } +} + +impl wl_shm::Handler for ShmHandler { + fn create_pool(&mut self, evqh: &mut EventLoopHandle, _client: &Client, _shm: &wl_shm::WlShm, + pool: wl_shm_pool::WlShmPool, fd: RawFd, size: i32) { + let arc_pool = Box::new(Arc::new(Pool::new(fd, size))); + evqh.register_with_destructor::<_, ShmHandler, ShmHandler>(&pool, self.my_id); + pool.set_user_data(Box::into_raw(arc_pool) as *mut ()); + } +} + +impl Destroy for ShmHandler { + fn destroy(pool: &wl_shm_pool::WlShmPool) { + let arc_pool = unsafe { Box::from_raw(pool.get_user_data() as *mut Arc) }; + drop(arc_pool) + } +} + +declare_handler!(ShmHandler, wl_shm::Handler, wl_shm::WlShm); + +/// Details of the contents of a buffer relative to its pool +#[derive(Copy,Clone,Debug)] +pub struct BufferData { + /// Offset of the start of the buffer relative to the beginning of the pool in bytes + pub offset: i32, + /// Wwidth of the buffer in bytes + pub width: i32, + /// Height of the buffer in bytes + pub height: i32, + /// Stride of the buffer in bytes + pub stride: i32, + /// Format used by this buffer + pub format: wl_shm::Format +} + +struct InternalBufferData { + pool: Arc, + data: BufferData +} + +impl wl_shm_pool::Handler for ShmHandler { + fn create_buffer(&mut self, evqh: &mut EventLoopHandle, _client: &Client, + pool: &wl_shm_pool::WlShmPool, buffer: wl_buffer::WlBuffer, offset: i32, + width: i32, height: i32, stride: i32, format: wl_shm::Format) + { + if !self.valid_formats.contains(&format) { + buffer.post_error(wl_shm::Error::InvalidFormat as u32, String::new()); + return + } + let arc_pool = unsafe { &*(pool.get_user_data() as *mut Arc) }; + let data = Box::into_raw(Box::new(InternalBufferData { + pool: arc_pool.clone(), + data: BufferData { + offset: offset, + width: width, + height: height, + stride: stride, + format: format + } + })); + evqh.register_with_destructor::<_, ShmHandler, ShmHandler>(&buffer, self.my_id); + buffer.set_user_data(data as *mut ()); + } + + fn resize(&mut self, _evqh: &mut EventLoopHandle, _client: &Client, + pool: &wl_shm_pool::WlShmPool, size: i32) + { + let arc_pool = unsafe { &*(pool.get_user_data() as *mut Arc) }; + if arc_pool.resize(size).is_err() { + pool.post_error(wl_shm::Error::InvalidFd as u32, "Invalid new size for a wl_shm_pool.".into()) + } + } +} + +impl Destroy for ShmHandler { + fn destroy(buffer: &wl_buffer::WlBuffer) { + let buffer_data = unsafe { Box::from_raw(buffer.get_user_data() as *mut InternalBufferData) }; + drop(buffer_data) + } +} + +declare_handler!(ShmHandler, wl_shm_pool::Handler, wl_shm_pool::WlShmPool); + +impl wl_buffer::Handler for ShmHandler { +} + +declare_handler!(ShmHandler, wl_buffer::Handler, wl_buffer::WlBuffer); diff --git a/src/shm/pool.rs b/src/shm/pool.rs new file mode 100644 index 0000000..07a0198 --- /dev/null +++ b/src/shm/pool.rs @@ -0,0 +1,17 @@ +use std::os::unix::io::RawFd; + +pub struct Pool; + +impl Pool { + pub fn new(fd: RawFd, size: i32) -> Pool { + unimplemented!() + } + + pub fn resize(&self, newsize: i32) -> Result<(),()> { + unimplemented!() + } + + pub fn with_data_slice(&self, f: F) -> Result<(),()> { + unimplemented!() + } +} From 12dc3b65d8c08139d193f8ace1ad7ae1b8fc0682 Mon Sep 17 00:00:00 2001 From: Victor Berger Date: Mon, 20 Feb 2017 22:32:03 +0100 Subject: [PATCH 2/8] shm: mmap logic --- Cargo.toml | 1 + src/lib.rs | 1 + src/shm/mod.rs | 27 ++++++++++++--- src/shm/pool.rs | 88 +++++++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 106 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0e4ddf7..40217c4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,3 +5,4 @@ authors = ["Victor Berger "] [dependencies] wayland-server = "0.8.4" +nix = "0.7.0" diff --git a/src/lib.rs b/src/lib.rs index f6df6e7..8ac17eb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,5 +2,6 @@ #[macro_use] extern crate wayland_server; +extern crate nix; pub mod shm; diff --git a/src/shm/mod.rs b/src/shm/mod.rs index ee9a7bd..3624c54 100644 --- a/src/shm/mod.rs +++ b/src/shm/mod.rs @@ -32,7 +32,7 @@ use std::sync::Arc; use wayland_server::{GlobalHandler, EventLoopHandle, Client, Init, Resource, Destroy, resource_is_registered}; use wayland_server::protocol::{wl_shm, wl_shm_pool, wl_buffer}; -use self::pool::Pool; +use self::pool::{Pool, ResizeError}; mod pool; @@ -156,9 +156,20 @@ impl Init for ShmHandler { } impl wl_shm::Handler for ShmHandler { - fn create_pool(&mut self, evqh: &mut EventLoopHandle, _client: &Client, _shm: &wl_shm::WlShm, + fn create_pool(&mut self, evqh: &mut EventLoopHandle, _client: &Client, shm: &wl_shm::WlShm, pool: wl_shm_pool::WlShmPool, fd: RawFd, size: i32) { - let arc_pool = Box::new(Arc::new(Pool::new(fd, size))); + if size <= 0 { + shm.post_error(wl_shm::Error::InvalidFd as u32, "Invalid size for a new wl_shm_pool.".into()); + return + } + let mmap_pool = match Pool::new(fd, size as usize) { + Ok(p) => p, + Err(()) => { + shm.post_error(wl_shm::Error::InvalidFd as u32, format!("Failed mmap of fd {}.", fd)); + return + } + }; + let arc_pool = Box::new(Arc::new(mmap_pool)); evqh.register_with_destructor::<_, ShmHandler, ShmHandler>(&pool, self.my_id); pool.set_user_data(Box::into_raw(arc_pool) as *mut ()); } @@ -221,8 +232,14 @@ impl wl_shm_pool::Handler for ShmHandler { pool: &wl_shm_pool::WlShmPool, size: i32) { let arc_pool = unsafe { &*(pool.get_user_data() as *mut Arc) }; - if arc_pool.resize(size).is_err() { - pool.post_error(wl_shm::Error::InvalidFd as u32, "Invalid new size for a wl_shm_pool.".into()) + match arc_pool.resize(size) { + Ok(()) => {}, + Err(ResizeError::InvalidSize) => { + pool.post_error(wl_shm::Error::InvalidFd as u32, "Invalid new size for a wl_shm_pool.".into()); + }, + Err(ResizeError::MremapFailed) => { + pool.post_error(wl_shm::Error::InvalidFd as u32, "mremap failed.".into()); + } } } } diff --git a/src/shm/pool.rs b/src/shm/pool.rs index 07a0198..4aa07f3 100644 --- a/src/shm/pool.rs +++ b/src/shm/pool.rs @@ -1,17 +1,93 @@ use std::os::unix::io::RawFd; +use std::sync::RwLock; +use std::ptr; -pub struct Pool; +use nix::sys::mman; + +pub struct Pool { + map: RwLock +} + +pub enum ResizeError { + InvalidSize, + MremapFailed +} impl Pool { - pub fn new(fd: RawFd, size: i32) -> Pool { - unimplemented!() + pub fn new(fd: RawFd, size: usize) -> Result { + let memmap = MemMap::new(fd, size)?; + Ok(Pool { + map: RwLock::new(memmap) + }) } - pub fn resize(&self, newsize: i32) -> Result<(),()> { - unimplemented!() + pub fn resize(&self, newsize: i32) -> Result<(),ResizeError> { + let mut guard = self.map.write().unwrap(); + if newsize <= 0 || guard.size() > (newsize as usize) { + return Err(ResizeError::InvalidSize) + } + guard.remap(newsize as usize).map_err(|()| ResizeError::MremapFailed) } pub fn with_data_slice(&self, f: F) -> Result<(),()> { - unimplemented!() + // TODO: handle SIGBUS + let guard = self.map.read().unwrap(); + + let slice = guard.get_slice(); + f(slice); + + Ok(()) } } + +struct MemMap { + ptr: *mut u8, + fd: RawFd, + size: usize +} + +impl MemMap { + fn new(fd: RawFd, size: usize) -> Result { + Ok(MemMap { + ptr: map(fd, size)?, + fd: fd, + size: size + }) + } + + fn remap(&mut self, newsize: usize) -> Result<(),()> { + unmap(self.ptr, self.size)?; + self.ptr = map(self.fd, newsize)?; + self.size = newsize; + Ok(()) + } + + fn size(&self) -> usize { + self.size + } + + fn get_slice(&self) -> &[u8] { + unsafe { ::std::slice::from_raw_parts(self.ptr, self.size) } + } +} + +// mman::mmap should really be unsafe... why isn't it? +#[allow(unused_unsafe)] +fn map(fd: RawFd, size: usize) -> Result<*mut u8, ()> { + let ret = unsafe { mman::mmap( + ptr::null_mut(), + size, + mman::PROT_READ, + mman::MAP_SHARED, + fd, + 0 + ) }; + ret.map(|p| p as *mut u8).map_err(|_| ()) +} + +// mman::munmap should really be unsafe... why isn't it? +#[allow(unused_unsafe)] +fn unmap(ptr: *mut u8, size: usize) -> Result<(),()> { + let ret = unsafe { mman::munmap(ptr as *mut _, size) }; + ret.map_err(|_| ()) +} From 64a4fcb699d412cf730b214f13312d43d8c4d826 Mon Sep 17 00:00:00 2001 From: Victor Berger Date: Tue, 21 Feb 2017 11:35:58 +0100 Subject: [PATCH 3/8] shm: add a proper Drop implementation to MemMap --- src/shm/pool.rs | 49 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/src/shm/pool.rs b/src/shm/pool.rs index 4aa07f3..d4d5e21 100644 --- a/src/shm/pool.rs +++ b/src/shm/pool.rs @@ -49,17 +49,34 @@ struct MemMap { impl MemMap { fn new(fd: RawFd, size: usize) -> Result { Ok(MemMap { - ptr: map(fd, size)?, + ptr: unsafe { map(fd, size) }?, fd: fd, size: size }) } fn remap(&mut self, newsize: usize) -> Result<(),()> { - unmap(self.ptr, self.size)?; - self.ptr = map(self.fd, newsize)?; - self.size = newsize; - Ok(()) + if self.ptr.is_null() { + return Err(()) + } + // memunmap cannot fail, as we are unmapping a pre-existing map + let _ = unsafe { unmap(self.ptr, self.size) }; + // remap the fd with the new size + match unsafe { map(self.fd, newsize) } { + Ok(ptr) => { + // update the parameters + self.ptr = ptr; + self.size = newsize; + Ok(()) + }, + Err(()) => { + // set ourselves in an empty state + self.ptr = ptr::null_mut(); + self.size = 0; + self.fd = -1; + Err(()) + } + } } fn size(&self) -> usize { @@ -67,27 +84,35 @@ impl MemMap { } fn get_slice(&self) -> &[u8] { + // if we are in the 'invalid state', self.size == 0 and we return &[] + // which is perfectly safe even if self.ptr is null unsafe { ::std::slice::from_raw_parts(self.ptr, self.size) } } } +impl Drop for MemMap { + fn drop(&mut self) { + if !self.ptr.is_null() { + let _ = unsafe { unmap(self.ptr, self.size) }; + } + } +} + // mman::mmap should really be unsafe... why isn't it? -#[allow(unused_unsafe)] -fn map(fd: RawFd, size: usize) -> Result<*mut u8, ()> { - let ret = unsafe { mman::mmap( +unsafe fn map(fd: RawFd, size: usize) -> Result<*mut u8, ()> { + let ret = mman::mmap( ptr::null_mut(), size, mman::PROT_READ, mman::MAP_SHARED, fd, 0 - ) }; + ); ret.map(|p| p as *mut u8).map_err(|_| ()) } // mman::munmap should really be unsafe... why isn't it? -#[allow(unused_unsafe)] -fn unmap(ptr: *mut u8, size: usize) -> Result<(),()> { - let ret = unsafe { mman::munmap(ptr as *mut _, size) }; +unsafe fn unmap(ptr: *mut u8, size: usize) -> Result<(),()> { + let ret = mman::munmap(ptr as *mut _, size); ret.map_err(|_| ()) } From 9d27537633d655b8bc6bf7ed66119f6c47b8b404 Mon Sep 17 00:00:00 2001 From: Victor Berger Date: Tue, 21 Feb 2017 16:09:06 +0100 Subject: [PATCH 4/8] shm: SIGBUS handling machinery --- src/shm/pool.rs | 123 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 118 insertions(+), 5 deletions(-) diff --git a/src/shm/pool.rs b/src/shm/pool.rs index d4d5e21..07dcc21 100644 --- a/src/shm/pool.rs +++ b/src/shm/pool.rs @@ -1,8 +1,16 @@ +use std::cell::Cell; use std::os::unix::io::RawFd; -use std::sync::RwLock; +use std::sync::{RwLock, Once, ONCE_INIT}; use std::ptr; +use nix::{c_int, c_void, libc}; use nix::sys::mman; +use nix::sys::signal::{self, SigAction, Signal, SigHandler}; + +thread_local!(static SIGBUS_GUARD: Cell<(*const MemMap, bool)> = Cell::new((ptr::null_mut(), false))); + +static SIGBUS_INIT: Once = ONCE_INIT; +static mut OLD_SIGBUS_HANDLER: *mut SigAction = 0 as *mut SigAction; pub struct Pool { map: RwLock @@ -30,13 +38,32 @@ impl Pool { } pub fn with_data_slice(&self, f: F) -> Result<(),()> { - // TODO: handle SIGBUS - let guard = self.map.read().unwrap(); + // Place the sigbus handler + SIGBUS_INIT.call_once(|| { + unsafe { place_sigbus_handler(); } + }); - let slice = guard.get_slice(); + let pool_guard = self.map.read().unwrap(); + + // Prepare the access + SIGBUS_GUARD.with(|guard| { + let (p,_) = guard.get(); + if !p.is_null() { + // Recursive call of this method is not supported + panic!("Recursive access to a SHM pool content is not supported."); + } + guard.set((&*pool_guard as *const MemMap, false)) + }); + + let slice = pool_guard.get_slice(); f(slice); - Ok(()) + // Cleanup Post-access + SIGBUS_GUARD.with(|guard| { + let (_, triggered) = guard.get(); + guard.set((ptr::null_mut(), false)); + if triggered { Err(()) } else { Ok(()) } + }) } } @@ -88,6 +115,14 @@ impl MemMap { // which is perfectly safe even if self.ptr is null unsafe { ::std::slice::from_raw_parts(self.ptr, self.size) } } + + fn contains(&self, ptr: *mut u8) -> bool { + ptr >= self.ptr && ptr < unsafe { self.ptr.offset(self.size as isize) } + } + + fn nullify(&self) -> Result<(),()> { + unsafe { nullify_map(self.ptr, self.size) } + } } impl Drop for MemMap { @@ -116,3 +151,81 @@ unsafe fn unmap(ptr: *mut u8, size: usize) -> Result<(),()> { let ret = mman::munmap(ptr as *mut _, size); ret.map_err(|_| ()) } + +unsafe fn nullify_map(ptr: *mut u8, size: usize) -> Result<(), ()> { + let ret = mman::mmap( + ptr as *mut _, + size, + mman::PROT_READ, + mman::MAP_ANONYMOUS | mman::MAP_PRIVATE | mman::MAP_FIXED, + -1, + 0 + ); + ret.map(|_| ()).map_err(|_| ()) +} + +unsafe fn place_sigbus_handler() { + // create our sigbus handler + let action = SigAction::new( + SigHandler::SigAction(sigbus_handler), + signal::SA_NODEFER, + signal::SigSet::empty() + ); + match signal::sigaction(Signal::SIGBUS, &action) { + Ok(old_signal) => { + OLD_SIGBUS_HANDLER = Box::into_raw(Box::new(old_signal)); + }, + Err(e) => { + panic!("sigaction failed sor SIGBUS handler: {:?}", e) + } + } +} + +unsafe fn reraise_sigbus() { + // reset the old sigaction + let _ = signal::sigaction(Signal::SIGBUS, &*OLD_SIGBUS_HANDLER); + let _ = signal::raise(Signal::SIGBUS); +} + +extern "C" fn sigbus_handler(_signum: c_int, info: *mut libc::siginfo_t, _context: *mut c_void) { + let faulty_ptr = unsafe { siginfo_si_addr(info) } as *mut u8; + SIGBUS_GUARD.with(|guard| { + let (memmap, _) = guard.get(); + match unsafe { memmap.as_ref() }.map(|m| (m, m.contains(faulty_ptr))) { + Some((m, true)) => { + // we are in a faulty memory pool ! + // remember that it was faulty + guard.set((memmap, true)); + // nullify the pool + if m.nullify().is_err() { + // something terrible occured ! + unsafe { reraise_sigbus() } + } + }, + _ => { + // something else occured, let's die honorably + unsafe { reraise_sigbus() } + } + } + }); +} + +// This was shamelessly stolen from rustc's source +// so I expect it to work whevener rust works +// I guess it's good enough? + +#[cfg(any(target_os = "linux", target_os = "android"))] +unsafe fn siginfo_si_addr(info: *mut libc::siginfo_t) -> *mut c_void { + #[repr(C)] + struct siginfo_t { + a: [libc::c_int; 3], // si_signo, si_errno, si_code + si_addr: *mut libc::c_void, + } + + (*(info as *const siginfo_t)).si_addr +} + +#[cfg(not(any(target_os = "linux", target_os = "android")))] +unsafe fn siginfo_si_addr(info: *mut libc::siginfo_t) -> *mut c_void { + (*info).si_addr +} From 0499c4b8ed12c94bb0672ba774df30b84316c9c8 Mon Sep 17 00:00:00 2001 From: Victor Berger Date: Tue, 21 Feb 2017 16:23:45 +0100 Subject: [PATCH 5/8] shm: kill bad client & document SIGBUS --- src/shm/mod.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/shm/mod.rs b/src/shm/mod.rs index 3624c54..4b0f073 100644 --- a/src/shm/mod.rs +++ b/src/shm/mod.rs @@ -25,6 +25,13 @@ //! } //! ); //! ``` +//! +//! **Note** +//! +//! This handler makes itself safe regading the client providing a wring size for the memory pool +//! by using a SIGBUS handler. +//! +//! If you are already using an handler for this signal, you probably don't want to use this handler. use std::os::unix::io::RawFd; use std::sync::Arc; @@ -116,6 +123,7 @@ impl ShmGlobalToken { if data.pool.with_data_slice(|slice| f(slice, data.data.clone())).is_err() { // SIGBUS error occured + buffer.post_error(wl_shm::Error::InvalidFd as u32, "Bad pool size.".into()); return Err(BufferAccessError::BadMap) } Ok(()) From 78ba42bdb114beca81c4cad907ef895dcdcd2202 Mon Sep 17 00:00:00 2001 From: Victor Berger Date: Wed, 22 Feb 2017 10:59:44 +0100 Subject: [PATCH 6/8] shm: improve doc --- src/shm/mod.rs | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/shm/mod.rs b/src/shm/mod.rs index 4b0f073..dd2a956 100644 --- a/src/shm/mod.rs +++ b/src/shm/mod.rs @@ -6,13 +6,32 @@ //! you want to support (ARGB8888 and XRGB8888 are always considered as supported, //! as specified by the wayland protocol) and obtain its `ShmGlobalToken`. //! -//! ```ignore -//! let handler_id = event_loop.add_handler(ShmGlobal::new()); -//! let shm_global = event_loop.register_global::(handler_id, 3); +//! ``` +//! extern crate wayland_server; +//! extern crate smithay; +//! +//! use smithay::shm::ShmGlobal; +//! use wayland_server::protocol::wl_shm; +//! +//! # fn main() { +//! # let (_, mut event_loop) = wayland_server::create_display(); +//! +//! // Insert the ShmGlobal as a handler to your event loop +//! // Here, we specify that Yuyv and C8 format are supported +//! // additionnaly to the standart Argb8888 and Xrgb8888. +//! let handler_id = event_loop.add_handler_with_init(ShmGlobal::new( +//! vec![wl_shm::Format::Yuyv, wl_shm::Format::C8], +//! None // we don't provide a logger here +//! )); +//! // Register this handler to advertise a wl_shm global of version 1 +//! let shm_global = event_loop.register_global::(handler_id, 1); +//! // Retrieve the shm token for later use to access the buffers //! let shm_token = { //! let state = event_loop.state(); //! state.get_handler::(handler_id).get_token() //! }; +//! +//! # } //! ``` //! //! Then, when you have a `WlBuffer` and need to retrieve its contents, use the token method to From a51a780e77c39b5741a5e34d4a58e5f3d9cfd911 Mon Sep 17 00:00:00 2001 From: Victor Berger Date: Wed, 22 Feb 2017 11:00:03 +0100 Subject: [PATCH 7/8] Add slog integration --- Cargo.toml | 5 +++++ src/lib.rs | 4 ++++ src/shm/mod.rs | 27 ++++++++++++++++++++------- src/shm/pool.rs | 29 +++++++++++++++++++++++------ 4 files changed, 52 insertions(+), 13 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 40217c4..47eeab4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,3 +6,8 @@ authors = ["Victor Berger "] [dependencies] wayland-server = "0.8.4" nix = "0.7.0" +slog = { version = "~1.5.2", features = ["max_level_trace", "release_max_level_info"] } +slog-stdlog = "~1.1.0" + +[dev-dependencies] +slog-term = "~1.5" diff --git a/src/lib.rs b/src/lib.rs index 8ac17eb..3a7d7aa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,4 +4,8 @@ extern crate wayland_server; extern crate nix; +#[macro_use] +extern crate slog; +extern crate slog_stdlog; + pub mod shm; diff --git a/src/shm/mod.rs b/src/shm/mod.rs index dd2a956..74be8c6 100644 --- a/src/shm/mod.rs +++ b/src/shm/mod.rs @@ -68,7 +68,8 @@ mod pool; /// quickly panic. pub struct ShmGlobal { formats: Vec, - handler_id: Option + handler_id: Option, + log: ::slog::Logger } impl ShmGlobal { @@ -77,13 +78,22 @@ impl ShmGlobal { /// This global will always advertize `ARGB8888` and `XRGB8888` format /// as they are required by the protocol. Formats given as argument /// as additionnaly advertized. - pub fn new(mut formats: Vec) -> ShmGlobal { + /// + /// An optionnal `slog::Logger` can be provided and will be used as a drain + /// for logging. If `None` is provided, it'll log to `slog-stdlog`. + pub fn new(mut formats: Vec, logger: L) -> ShmGlobal + where L: Into> + { + use slog::DrainExt; + let log = logger.into().unwrap_or(::slog::Logger::root(::slog_stdlog::StdLog.fuse(), o!())); + // always add the mandatory formats formats.push(wl_shm::Format::Argb8888); formats.push(wl_shm::Format::Xrgb8888); ShmGlobal { formats: formats, - handler_id: None + handler_id: None, + log: log.new(o!("smithay_module" => "shm_handler")) } } @@ -95,7 +105,7 @@ impl ShmGlobal { /// This is needed to retrieve the contents of the shm pools and buffers. pub fn get_token(&self) -> ShmGlobalToken { ShmGlobalToken { - hid: self.handler_id.clone().expect("ShmGlobal was not initialized.") + hid: self.handler_id.clone().expect("ShmGlobal was not initialized."), } } } @@ -153,7 +163,8 @@ impl Init for ShmGlobal { fn init(&mut self, evqh: &mut EventLoopHandle, _index: usize) { let id = evqh.add_handler_with_init(ShmHandler { my_id: ::std::usize::MAX, - valid_formats: self.formats.clone() + valid_formats: self.formats.clone(), + log: self.log.clone() }); self.handler_id = Some(id); } @@ -173,12 +184,14 @@ impl GlobalHandler for ShmGlobal { struct ShmHandler { my_id: usize, - valid_formats: Vec + valid_formats: Vec, + log: ::slog::Logger } impl Init for ShmHandler { fn init(&mut self, _evqh: &mut EventLoopHandle, index: usize) { self.my_id = index; + debug!(self.log, "Init finished") } } @@ -189,7 +202,7 @@ impl wl_shm::Handler for ShmHandler { shm.post_error(wl_shm::Error::InvalidFd as u32, "Invalid size for a new wl_shm_pool.".into()); return } - let mmap_pool = match Pool::new(fd, size as usize) { + let mmap_pool = match Pool::new(fd, size as usize, self.log.clone()) { Ok(p) => p, Err(()) => { shm.post_error(wl_shm::Error::InvalidFd as u32, format!("Failed mmap of fd {}.", fd)); diff --git a/src/shm/pool.rs b/src/shm/pool.rs index 07dcc21..c2fae05 100644 --- a/src/shm/pool.rs +++ b/src/shm/pool.rs @@ -13,7 +13,9 @@ static SIGBUS_INIT: Once = ONCE_INIT; static mut OLD_SIGBUS_HANDLER: *mut SigAction = 0 as *mut SigAction; pub struct Pool { - map: RwLock + map: RwLock, + fd: i32, + log: ::slog::Logger } pub enum ResizeError { @@ -22,19 +24,27 @@ pub enum ResizeError { } impl Pool { - pub fn new(fd: RawFd, size: usize) -> Result { + pub fn new(fd: RawFd, size: usize, log: ::slog::Logger) -> Result { let memmap = MemMap::new(fd, size)?; + trace!(log, "Creating new shm pool"; "fd" => fd as i32, "size" => size); Ok(Pool { - map: RwLock::new(memmap) + map: RwLock::new(memmap), + fd: fd as i32, + log: log }) } pub fn resize(&self, newsize: i32) -> Result<(),ResizeError> { let mut guard = self.map.write().unwrap(); - if newsize <= 0 || guard.size() > (newsize as usize) { + let oldsize = guard.size(); + if newsize <= 0 || oldsize > (newsize as usize) { return Err(ResizeError::InvalidSize) } - guard.remap(newsize as usize).map_err(|()| ResizeError::MremapFailed) + trace!(self.log, "Resizing shm pool"; "fd" => self.fd as i32, "oldsize" => oldsize, "newsize" => newsize); + guard.remap(newsize as usize).map_err(|()| { + debug!(self.log, "SHM pool resize failed"; "fd" => self.fd as i32, "oldsize" => oldsize, "newsize" => newsize); + ResizeError::MremapFailed + }) } pub fn with_data_slice(&self, f: F) -> Result<(),()> { @@ -45,6 +55,8 @@ impl Pool { let pool_guard = self.map.read().unwrap(); + trace!(self.log, "Buffer access on shm pool"; "fd" => self.fd as i32); + // Prepare the access SIGBUS_GUARD.with(|guard| { let (p,_) = guard.get(); @@ -62,7 +74,12 @@ impl Pool { SIGBUS_GUARD.with(|guard| { let (_, triggered) = guard.get(); guard.set((ptr::null_mut(), false)); - if triggered { Err(()) } else { Ok(()) } + if triggered { + debug!(self.log, "SIGBUS caught on access on shm pool"; "fd" => self.fd); + Err(()) + } else { + Ok(()) + } }) } } From 34accc7da3f9f4cf689563c73a9bffd9ee148fe1 Mon Sep 17 00:00:00 2001 From: Victor Berger Date: Wed, 22 Feb 2017 11:39:13 +0100 Subject: [PATCH 8/8] Properly close FD on memmap drop --- src/shm/pool.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/shm/pool.rs b/src/shm/pool.rs index c2fae05..886007c 100644 --- a/src/shm/pool.rs +++ b/src/shm/pool.rs @@ -3,7 +3,7 @@ use std::os::unix::io::RawFd; use std::sync::{RwLock, Once, ONCE_INIT}; use std::ptr; -use nix::{c_int, c_void, libc}; +use nix::{c_int, c_void, libc, unistd}; use nix::sys::mman; use nix::sys::signal::{self, SigAction, Signal, SigHandler}; @@ -14,7 +14,7 @@ static mut OLD_SIGBUS_HANDLER: *mut SigAction = 0 as *mut SigAction; pub struct Pool { map: RwLock, - fd: i32, + fd: RawFd, log: ::slog::Logger } @@ -29,7 +29,7 @@ impl Pool { trace!(log, "Creating new shm pool"; "fd" => fd as i32, "size" => size); Ok(Pool { map: RwLock::new(memmap), - fd: fd as i32, + fd: fd, log: log }) } @@ -84,6 +84,13 @@ impl Pool { } } +impl Drop for Pool { + fn drop(&mut self) { + trace!(self.log, "Deleting SHM pool"; "fd" => self.fd); + let _ = unsafe { unistd::close(self.fd) }; + } +} + struct MemMap { ptr: *mut u8, fd: RawFd,