Bug 1359462 - Ensure we don't pass a nullptr to slice::from_raw_parts. r?rhunt draft
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 25 Apr 2017 14:35:43 -0400
changeset 568054 09b5294d43bc5f84a5c193e1f4bbf54c25019030
parent 568038 708b9a30040654c6ead5d03a18966d11181cee1d
child 625808 8ea595a818c9edfb6bc0c36ca8107e5f8eeca6dc
push id55744
push userkgupta@mozilla.com
push dateTue, 25 Apr 2017 18:35:54 +0000
reviewersrhunt
bugs1359462
milestone55.0a1
Bug 1359462 - Ensure we don't pass a nullptr to slice::from_raw_parts. r?rhunt Apparently passing a nullptr to slice::from_raw_parts is unsafe. So we wrap those calls into a helper function that returns an empty slice instead. MozReview-Commit-ID: Dzu2aVY6DgE
gfx/webrender_bindings/src/bindings.rs
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@@ -71,32 +71,48 @@ const GL_FORMAT_BGRA_GLES: gl::GLuint = 
 
 fn get_gl_format_bgra(gl: &gl::Gl) -> gl::GLuint {
     match gl.get_type() {
         gl::GlType::Gl => GL_FORMAT_BGRA_GL,
         gl::GlType::Gles => GL_FORMAT_BGRA_GLES,
     }
 }
 
+fn make_slice<'a, T>(ptr: *const T, len: usize) -> &'a [T] {
+    if ptr.is_null() {
+        &[]
+    } else {
+        unsafe { slice::from_raw_parts(ptr, len) }
+    }
+}
+
+fn make_slice_mut<'a, T>(ptr: *mut T, len: usize) -> &'a mut [T] {
+    if ptr.is_null() {
+        &mut []
+    } else {
+        unsafe { slice::from_raw_parts_mut(ptr, len) }
+    }
+}
+
 #[repr(C)]
 pub struct WrByteSlice {
     buffer: *const u8,
     len: usize,
 }
 
 impl WrByteSlice {
     pub fn new(slice: &[u8]) -> WrByteSlice {
         WrByteSlice {
             buffer: &slice[0],
             len: slice.len(),
         }
     }
 
     pub fn as_slice(&self) -> &[u8] {
-        unsafe { slice::from_raw_parts(self.buffer, self.len) }
+        make_slice(self.buffer, self.len)
     }
 }
 
 #[repr(C)]
 pub struct MutByteSlice {
     buffer: *mut u8,
     len: usize,
 }
@@ -106,17 +122,17 @@ impl MutByteSlice {
         let len = slice.len();
         MutByteSlice {
             buffer: &mut slice[0],
             len: len,
         }
     }
 
     pub fn as_mut_slice(&mut self) -> &mut [u8] {
-        unsafe { slice::from_raw_parts_mut(self.buffer, self.len) }
+        make_slice_mut(self.buffer, self.len)
     }
 }
 
 #[repr(u32)]
 pub enum WrGradientExtendMode {
     Clamp,
     Repeat,
 }
@@ -565,17 +581,17 @@ impl ExternalImageHandler for WrExternal
                 }
             },
             WrExternalImageType::RawData => {
                 ExternalImage {
                     u0: image.u0,
                     v0: image.v0,
                     u1: image.u1,
                     v1: image.v1,
-                    source: ExternalImageSource::RawData(unsafe { slice::from_raw_parts(image.buff, image.size) }),
+                    source: ExternalImageSource::RawData(make_slice(image.buff, image.size)),
                 }
             },
         }
     }
 
     fn unlock(&mut self,
               id: ExternalImageId,
               _channel_index: u8) {
@@ -757,17 +773,17 @@ pub unsafe extern "C" fn wr_renderer_rea
                                               width: u32,
                                               height: u32,
                                               dst_buffer: *mut u8,
                                               buffer_size: usize) {
     assert!(is_in_render_thread());
 
     renderer.gl().flush();
 
-    let mut slice = slice::from_raw_parts_mut(dst_buffer, buffer_size);
+    let mut slice = make_slice_mut(dst_buffer, buffer_size);
     renderer.gl()
             .read_pixels_into_buffer(0,
                                      0,
                                      width as gl::GLsizei,
                                      height as gl::GLsizei,
                                      get_gl_format_bgra(renderer.gl()),
                                      gl::UNSIGNED_BYTE,
                                      slice);
@@ -1003,23 +1019,23 @@ pub unsafe extern "C" fn wr_api_set_root
                                                       aux_data: *mut u8,
                                                       aux_size: usize) {
     let root_background_color = ColorF::new(0.3, 0.0, 0.0, 1.0);
     // See the documentation of set_display_list in api.rs. I don't think
     // it makes a difference in gecko at the moment(until APZ is figured out)
     // but I suppose it is a good default.
     let preserve_frame_state = true;
 
-    let dl_slice = slice::from_raw_parts(dl_data, dl_size);
+    let dl_slice = make_slice(dl_data, dl_size);
     let mut dl_vec = Vec::new();
     // XXX: see if we can get rid of the copy here
     dl_vec.extend_from_slice(dl_slice);
     let dl = BuiltDisplayList::from_data(dl_vec, dl_descriptor);
 
-    let aux_slice = slice::from_raw_parts(aux_data, aux_size);
+    let aux_slice = make_slice(aux_data, aux_size);
     let mut aux_vec = Vec::new();
     // XXX: see if we can get rid of the copy here
     aux_vec.extend_from_slice(aux_slice);
     let aux = AuxiliaryLists::from_data(aux_vec, aux_descriptor);
 
     api.set_display_list(Some(root_background_color),
                          epoch,
                          LayoutSize::new(viewport_width, viewport_height),
@@ -1054,30 +1070,30 @@ pub extern "C" fn wr_api_generate_frame_
                                                         transform_array: *const WrTransformProperty,
                                                         transform_count: usize) {
     let mut properties = DynamicProperties {
         transforms: Vec::new(),
         floats: Vec::new(),
     };
 
     if transform_count > 0 {
-        let transform_slice = unsafe { slice::from_raw_parts(transform_array, transform_count) };
+        let transform_slice = make_slice(transform_array, transform_count);
 
         for element in transform_slice.iter() {
             let prop = PropertyValue {
                 key: PropertyBindingKey::new(element.id),
                 value: element.transform.into(),
             };
 
             properties.transforms.push(prop);
         }
     }
 
     if opacity_count > 0 {
-        let opacity_slice = unsafe { slice::from_raw_parts(opacity_array, opacity_count) };
+        let opacity_slice = make_slice(opacity_array, opacity_count);
 
         for element in opacity_slice.iter() {
             let prop = PropertyValue {
                 key: PropertyBindingKey::new(element.id),
                 value: element.opacity,
             };
             properties.floats.push(prop);
         }
@@ -1098,17 +1114,17 @@ pub extern "C" fn wr_api_send_external_e
 #[no_mangle]
 pub extern "C" fn wr_api_add_raw_font(api: &mut WrAPI,
                                       key: WrFontKey,
                                       font_buffer: *mut u8,
                                       buffer_size: usize,
                                       index: u32) {
     assert!(unsafe { is_in_compositor_thread() });
 
-    let font_slice = unsafe { slice::from_raw_parts(font_buffer, buffer_size as usize) };
+    let font_slice = make_slice(font_buffer, buffer_size);
     let mut font_vector = Vec::new();
     font_vector.extend_from_slice(font_slice);
 
     api.add_raw_font(key, font_vector, index);
 }
 
 #[no_mangle]
 pub extern "C" fn wr_api_delete_font(api: &mut WrAPI,
@@ -1210,17 +1226,17 @@ pub extern "C" fn wr_dp_new_clip_region(
                                         main: WrRect,
                                         complex: *const WrComplexClipRegion,
                                         complex_count: usize,
                                         image_mask: *const WrImageMask)
                                         -> WrClipRegion {
     assert!(unsafe { is_in_main_thread() });
 
     let main = main.into();
-    let complex_slice = unsafe { slice::from_raw_parts(complex, complex_count) };
+    let complex_slice = make_slice(complex, complex_count);
     let complex_vector = complex_slice.iter().map(|x| x.into()).collect();
     let mask = unsafe { image_mask.as_ref() }.map(|x| x.into());
 
     let clip_region = state.frame_builder.dl_builder.new_clip_region(&main, complex_vector, mask);
 
     clip_region.into()
 }
 
@@ -1336,17 +1352,17 @@ pub extern "C" fn wr_dp_push_text(state:
                                   clip: WrClipRegion,
                                   color: WrColor,
                                   font_key: WrFontKey,
                                   glyphs: *const WrGlyphInstance,
                                   glyph_count: u32,
                                   glyph_size: f32) {
     assert!(unsafe { is_in_main_thread() });
 
-    let glyph_slice = unsafe { slice::from_raw_parts(glyphs, glyph_count as usize) };
+    let glyph_slice = make_slice(glyphs, glyph_count as usize);
     let glyph_vector: Vec<_> = glyph_slice.iter().map(|x| x.into()).collect();
 
     let colorf = ColorF::new(color.r, color.g, color.b, color.a);
 
     let glyph_options = None; // TODO
     state.frame_builder
          .dl_builder
          .push_text(bounds.into(),
@@ -1415,17 +1431,17 @@ pub extern "C" fn wr_dp_push_border_grad
                                              start_point: WrPoint,
                                              end_point: WrPoint,
                                              stops: *const WrGradientStop,
                                              stops_count: usize,
                                              extend_mode: WrGradientExtendMode,
                                              outset: WrSideOffsets2Df32) {
     assert!(unsafe { is_in_main_thread() });
 
-    let stops_slice = unsafe { slice::from_raw_parts(stops, stops_count) };
+    let stops_slice = make_slice(stops, stops_count);
     let stops_vector = stops_slice.iter().map(|x| x.into()).collect();
 
     let border_details = BorderDetails::Gradient(GradientBorder {
                                                      gradient:
                                                          state.frame_builder
                                                               .dl_builder
                                                               .create_gradient(start_point.into(),
                                                                                end_point.into(),
@@ -1446,17 +1462,17 @@ pub extern "C" fn wr_dp_push_border_radi
                                                     center: WrPoint,
                                                     radius: WrSize,
                                                     stops: *const WrGradientStop,
                                                     stops_count: usize,
                                                     extend_mode: WrGradientExtendMode,
                                                     outset: WrSideOffsets2Df32) {
     assert!(unsafe { is_in_main_thread() });
 
-    let stops_slice = unsafe { slice::from_raw_parts(stops, stops_count) };
+    let stops_slice = make_slice(stops, stops_count);
     let stops_vector = stops_slice.iter().map(|x| x.into()).collect();
 
     let border_details =
         BorderDetails::RadialGradient(RadialGradientBorder {
                                           gradient:
                                               state.frame_builder
                                                    .dl_builder
                                                    .create_radial_gradient(center.into(),
@@ -1478,17 +1494,17 @@ pub extern "C" fn wr_dp_push_linear_grad
                                              end_point: WrPoint,
                                              stops: *const WrGradientStop,
                                              stops_count: usize,
                                              extend_mode: WrGradientExtendMode,
                                              tile_size: WrSize,
                                              tile_spacing: WrSize) {
     assert!(unsafe { is_in_main_thread() });
 
-    let stops_slice = unsafe { slice::from_raw_parts(stops, stops_count) };
+    let stops_slice = make_slice(stops, stops_count);
     let stops_vector = stops_slice.iter().map(|x| x.into()).collect();
 
     let gradient = state.frame_builder
                         .dl_builder
                         .create_gradient(start_point.into(),
                                          end_point.into(),
                                          stops_vector,
                                          extend_mode.into());
@@ -1508,17 +1524,17 @@ pub extern "C" fn wr_dp_push_radial_grad
                                              radius: WrSize,
                                              stops: *const WrGradientStop,
                                              stops_count: usize,
                                              extend_mode: WrGradientExtendMode,
                                              tile_size: WrSize,
                                              tile_spacing: WrSize) {
     assert!(unsafe { is_in_main_thread() });
 
-    let stops_slice = unsafe { slice::from_raw_parts(stops, stops_count) };
+    let stops_slice = make_slice(stops, stops_count);
     let stops_vector = stops_slice.iter().map(|x| x.into()).collect();
 
     let gradient = state.frame_builder
                         .dl_builder
                         .create_radial_gradient(center.into(),
                                                 radius.into(),
                                                 stops_vector,
                                                 extend_mode.into());