From 6c30ce5697ad75a17c1327be5d97d8dbd4e94bcd Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Wed, 30 Jul 2014 12:11:10 -0700 Subject: [PATCH 1/3] Fix use-after-free on Android. Fixes servo/servo#2916 (broken rendering on Android), which happened because the `bitmap` pointer outlived the Vec that it pointed to. --- platform/android/surface.rs | 41 +++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/platform/android/surface.rs b/platform/android/surface.rs index c0777df..e7aad62 100644 --- a/platform/android/surface.rs +++ b/platform/android/surface.rs @@ -52,8 +52,8 @@ impl NativeCompositingGraphicsContext { } pub struct NativeSurface { - image: Option, - bitmap: *c_void, + image: Option, // For GPU rendering + bitmap: Option>, // For CPU rendering will_leak: bool, } @@ -65,7 +65,7 @@ impl NativeSurface { } NativeSurface { image : _image, - bitmap : ptr::null(), + bitmap: None, will_leak: true, } } @@ -74,15 +74,13 @@ impl NativeSurface { impl NativeSurfaceMethods for NativeSurface { /// This may only be called on the case of CPU rendering. fn new(_native_context: &NativePaintingGraphicsContext, size: Size2D, _stride: i32) -> NativeSurface { - unsafe { - let len = size.width * size.height * 4; - let bitmap: Vec = Vec::from_elem(len as uint, 0 as u8); + let len = size.width * size.height * 4; + let bitmap: Vec = Vec::from_elem(len as uint, 0 as u8); - NativeSurface { - image: None, - bitmap: mem::transmute(bitmap.as_ptr()), - will_leak : true, - } + NativeSurface { + image: None, + bitmap: Some(bitmap), + will_leak : true, } } @@ -95,11 +93,12 @@ impl NativeSurfaceMethods for NativeSurface { unsafe { match self.image { - None => { - if self.bitmap != ptr::null() { - glTexImage2D(TEXTURE_2D, 0, BGRA as i32, _size.width as i32, _size.height as i32, 0, BGRA as u32, UNSIGNED_BYTE, self.bitmap); + None => match self.bitmap { + Some(ref bitmap) => { + let data: *c_void = mem::transmute(bitmap.as_ptr()); + glTexImage2D(TEXTURE_2D, 0, BGRA as i32, _size.width as i32, _size.height as i32, 0, BGRA as u32, UNSIGNED_BYTE, data); } - else { + None => { debug!("Cannot bind the buffer(CPU rendering), there is no bitmap"); } }, @@ -112,12 +111,14 @@ impl NativeSurfaceMethods for NativeSurface { /// This may only be called on the painting side. fn upload(&self, _graphics_context: &NativePaintingGraphicsContext, data: &[u8]) { - unsafe { - if self.bitmap != ptr::null() { - let dest:&mut [u8] = mem::transmute((self.bitmap, data.len())); - dest.copy_memory(data); + match self.bitmap { + Some(ref bitmap) => { + unsafe { + let dest:&mut [u8] = mem::transmute((bitmap.as_ptr(), data.len())); + dest.copy_memory(data); + } } - else { + None => { debug!("Cannot upload the buffer(CPU rendering), there is no bitmap"); } } From 752c6da761d6ae095a1ea45428ce5da7c69993dd Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Wed, 30 Jul 2014 12:36:56 -0700 Subject: [PATCH 2/3] Make NativeSurface::upload a mut method --- platform/android/surface.rs | 6 +++--- platform/linux/surface.rs | 2 +- platform/macos/surface.rs | 2 +- platform/surface.rs | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/platform/android/surface.rs b/platform/android/surface.rs index e7aad62..b3b9934 100644 --- a/platform/android/surface.rs +++ b/platform/android/surface.rs @@ -110,11 +110,11 @@ impl NativeSurfaceMethods for NativeSurface { } /// This may only be called on the painting side. - fn upload(&self, _graphics_context: &NativePaintingGraphicsContext, data: &[u8]) { + fn upload(&mut self, _graphics_context: &NativePaintingGraphicsContext, data: &[u8]) { match self.bitmap { - Some(ref bitmap) => { + Some(ref mut bitmap) => { unsafe { - let dest:&mut [u8] = mem::transmute((bitmap.as_ptr(), data.len())); + let dest: &mut [u8] = mem::transmute((bitmap.as_ptr(), bitmap.len())); dest.copy_memory(data); } } diff --git a/platform/linux/surface.rs b/platform/linux/surface.rs index d19a3bd..f858c1a 100644 --- a/platform/linux/surface.rs +++ b/platform/linux/surface.rs @@ -266,7 +266,7 @@ impl NativeSurfaceMethods for NativeSurface { } /// This may only be called on the painting side. - fn upload(&self, graphics_context: &NativePaintingGraphicsContext, data: &[u8]) { + fn upload(&mut self, graphics_context: &NativePaintingGraphicsContext, data: &[u8]) { unsafe { // Ensure that we're running on the render task. Take the display. let pixmap = self.pixmap; diff --git a/platform/macos/surface.rs b/platform/macos/surface.rs index fcd118b..2f1dd5a 100644 --- a/platform/macos/surface.rs +++ b/platform/macos/surface.rs @@ -206,7 +206,7 @@ impl NativeSurfaceMethods for NativeSurface { io_surface.bind_to_gl_texture(size) } - fn upload(&self, _: &NativePaintingGraphicsContext, data: &[u8]) { + fn upload(&mut self, _: &NativePaintingGraphicsContext, data: &[u8]) { let io_surface = io_surface::lookup(self.io_surface_id.unwrap()); io_surface.upload(data) } diff --git a/platform/surface.rs b/platform/surface.rs index 0397a77..7672127 100644 --- a/platform/surface.rs +++ b/platform/surface.rs @@ -54,7 +54,7 @@ pub trait NativeSurfaceMethods { size: Size2D); /// Uploads pixel data to the surface. Painting task only. - fn upload(&self, native_context: &NativePaintingGraphicsContext, data: &[u8]); + fn upload(&mut self, native_context: &NativePaintingGraphicsContext, data: &[u8]); /// Returns an opaque ID identifying the surface for debugging. fn get_id(&self) -> int; From 6f3053a99a95d079261336eaa647cb357eff64f4 Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Wed, 30 Jul 2014 12:48:30 -0700 Subject: [PATCH 3/3] Replace transmute with as and as_mut_slice --- platform/android/surface.rs | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/platform/android/surface.rs b/platform/android/surface.rs index b3b9934..80f525f 100644 --- a/platform/android/surface.rs +++ b/platform/android/surface.rs @@ -90,21 +90,21 @@ impl NativeSurfaceMethods for NativeSurface { texture: &Texture, _size: Size2D) { let _bound = texture.bind(); - - unsafe { - match self.image { - None => match self.bitmap { - Some(ref bitmap) => { - let data: *c_void = mem::transmute(bitmap.as_ptr()); - glTexImage2D(TEXTURE_2D, 0, BGRA as i32, _size.width as i32, _size.height as i32, 0, BGRA as u32, UNSIGNED_BYTE, data); - } - None => { - debug!("Cannot bind the buffer(CPU rendering), there is no bitmap"); + match self.image { + None => match self.bitmap { + Some(ref bitmap) => { + let data = bitmap.as_ptr() as *c_void; + unsafe { + glTexImage2D(TEXTURE_2D, 0, BGRA as i32, _size.width as i32, _size.height as i32, + 0, BGRA as u32, UNSIGNED_BYTE, data); } - }, - Some(image_khr) => { - egl_image_target_texture2d_oes(TEXTURE_2D, image_khr); } + None => { + debug!("Cannot bind the buffer(CPU rendering), there is no bitmap"); + } + }, + Some(image_khr) => { + egl_image_target_texture2d_oes(TEXTURE_2D, image_khr); } } } @@ -114,8 +114,7 @@ impl NativeSurfaceMethods for NativeSurface { match self.bitmap { Some(ref mut bitmap) => { unsafe { - let dest: &mut [u8] = mem::transmute((bitmap.as_ptr(), bitmap.len())); - dest.copy_memory(data); + bitmap.as_mut_slice().copy_memory(data); } } None => {