From 1199e89dca3a3b239fe33a192acfd4cddafd857f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Sun, 6 Dec 2020 19:39:28 +0200 Subject: [PATCH] gstreamer-base/adapter: Add error checking at the bindings level Instead of running into (inconsistent even) assertions inside the C code. Also don't return uninitialized data and signed offsets from the masked_scan() functions. Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/issues/298 --- gstreamer-base/Gir.toml | 45 ++--- gstreamer-base/src/adapter.rs | 299 +++++++++++++++++++++++++++-- gstreamer-base/src/auto/adapter.rs | 116 ----------- 3 files changed, 294 insertions(+), 166 deletions(-) diff --git a/gstreamer-base/Gir.toml b/gstreamer-base/Gir.toml index fd8151fc6..e5deb0b8f 100644 --- a/gstreamer-base/Gir.toml +++ b/gstreamer-base/Gir.toml @@ -42,7 +42,6 @@ name = "GstBase.Adapter" status = "generate" final_type = true concurrency = "send-unique" - [[object.function]] name = "map" # Unsafe @@ -54,7 +53,7 @@ concurrency = "send-unique" ignore = true [[object.function]] - name = "copy" + pattern = "copy.*" # Unsafe manual = true @@ -64,44 +63,24 @@ concurrency = "send-unique" manual = true [[object.function]] - name = "take" - # Useless copying of data + pattern = "take.*" + # Unsafe ignore = true [[object.function]] - name = "copy_bytes" - [object.function.return] - nullable_return_is_error = "Failed to copy bytes" + pattern = "get.*" + # Unsafe + ignore = true [[object.function]] - name = "get_buffer" - [object.function.return] - nullable_return_is_error = "Failed to get buffer" + pattern = "masked.*" + # Unsafe + ignore = true [[object.function]] - name = "get_buffer_fast" - [object.function.return] - nullable_return_is_error = "Failed to get buffer" - - [[object.function]] - name = "get_buffer_list" - [object.function.return] - nullable_return_is_error = "Failed to get buffer list" - - [[object.function]] - name = "take_buffer" - [object.function.return] - nullable_return_is_error = "Failed to take buffer" - - [[object.function]] - name = "take_buffer_fast" - [object.function.return] - nullable_return_is_error = "Failed to take buffer" - - [[object.function]] - name = "take_buffer_list" - [object.function.return] - nullable_return_is_error = "Failed to take buffer list" + name = "flush" + # Unsafe Buffer + manual = true [[object]] name = "GstBase.FlowCombiner" diff --git a/gstreamer-base/src/adapter.rs b/gstreamer-base/src/adapter.rs index cf713dd2b..70aa42724 100644 --- a/gstreamer-base/src/adapter.rs +++ b/gstreamer-base/src/adapter.rs @@ -9,10 +9,23 @@ use crate::Adapter; use glib::translate::*; use std::io; +use std::mem; use std::ops; impl Adapter { - pub fn copy(&self, offset: usize, dest: &mut [u8]) { + pub fn copy(&self, offset: usize, dest: &mut [u8]) -> Result<(), glib::BoolError> { + if offset + .checked_add(dest.len()) + .map(|end| end <= self.available()) + != Some(true) + { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + if dest.is_empty() { + return Ok(()); + } + unsafe { let size = dest.len(); ffi::gst_adapter_copy( @@ -22,6 +35,240 @@ impl Adapter { size, ); } + + Ok(()) + } + + pub fn copy_bytes(&self, offset: usize, size: usize) -> Result { + if offset.checked_add(size).map(|end| end <= self.available()) != Some(true) { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + if size == 0 { + return Ok(glib::Bytes::from_static(&[])); + } + + unsafe { + Ok(from_glib_full(ffi::gst_adapter_copy_bytes( + self.to_glib_none().0, + offset, + size, + ))) + } + } + + pub fn flush(&self, flush: usize) -> Result<(), glib::BoolError> { + if flush > self.available() { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + unsafe { + ffi::gst_adapter_flush(self.to_glib_none().0, flush); + } + + Ok(()) + } + + pub fn get_buffer(&self, nbytes: usize) -> Result { + if nbytes > self.available() { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + if nbytes == 0 { + return Ok(gst::Buffer::new()); + } + + unsafe { + Option::<_>::from_glib_full(ffi::gst_adapter_get_buffer(self.to_glib_none().0, nbytes)) + .ok_or_else(|| glib::glib_bool_error!("Failed to get buffer")) + } + } + + pub fn get_buffer_fast(&self, nbytes: usize) -> Result { + if nbytes > self.available() { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + if nbytes == 0 { + return Ok(gst::Buffer::new()); + } + + unsafe { + Option::<_>::from_glib_full(ffi::gst_adapter_get_buffer_fast( + self.to_glib_none().0, + nbytes, + )) + .ok_or_else(|| glib::glib_bool_error!("Failed to get buffer")) + } + } + + pub fn get_buffer_list(&self, nbytes: usize) -> Result { + if nbytes > self.available() { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + if nbytes == 0 { + return Ok(gst::BufferList::new()); + } + + unsafe { + Option::<_>::from_glib_full(ffi::gst_adapter_get_buffer_list( + self.to_glib_none().0, + nbytes, + )) + .ok_or_else(|| glib::glib_bool_error!("Failed to get buffer list")) + } + } + + pub fn get_list(&self, nbytes: usize) -> Result, glib::BoolError> { + if nbytes > self.available() { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + if nbytes == 0 { + return Ok(Vec::new()); + } + + unsafe { + Ok(FromGlibPtrContainer::from_glib_full( + ffi::gst_adapter_get_list(self.to_glib_none().0, nbytes), + )) + } + } + + pub fn masked_scan_uint32( + &self, + mask: u32, + pattern: u32, + offset: usize, + size: usize, + ) -> Result, glib::BoolError> { + if offset.checked_add(size).map(|end| end <= self.available()) != Some(true) { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + if size == 0 || ((!mask) & pattern) != 0 { + return Ok(None); + } + + unsafe { + let ret = ffi::gst_adapter_masked_scan_uint32( + self.to_glib_none().0, + mask, + pattern, + offset, + size, + ); + if ret == -1 { + Ok(None) + } else { + assert!(ret >= 0); + Ok(Some(ret as usize)) + } + } + } + + pub fn masked_scan_uint32_peek( + &self, + mask: u32, + pattern: u32, + offset: usize, + size: usize, + ) -> Result, glib::BoolError> { + if offset.checked_add(size).map(|end| end <= self.available()) != Some(true) { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + if size == 0 || ((!mask) & pattern) != 0 { + return Ok(None); + } + + unsafe { + let mut value = mem::MaybeUninit::uninit(); + let ret = ffi::gst_adapter_masked_scan_uint32_peek( + self.to_glib_none().0, + mask, + pattern, + offset, + size, + value.as_mut_ptr(), + ); + + if ret == -1 { + Ok(None) + } else { + assert!(ret >= 0); + let value = value.assume_init(); + Ok(Some((ret as usize, value))) + } + } + } + + pub fn take_buffer(&self, nbytes: usize) -> Result { + if nbytes > self.available() { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + if nbytes == 0 { + return Ok(gst::Buffer::new()); + } + + unsafe { + Option::<_>::from_glib_full(ffi::gst_adapter_take_buffer(self.to_glib_none().0, nbytes)) + .ok_or_else(|| glib::glib_bool_error!("Failed to take buffer")) + } + } + + pub fn take_buffer_fast(&self, nbytes: usize) -> Result { + if nbytes > self.available() { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + if nbytes == 0 { + return Ok(gst::Buffer::new()); + } + + unsafe { + Option::<_>::from_glib_full(ffi::gst_adapter_take_buffer_fast( + self.to_glib_none().0, + nbytes, + )) + .ok_or_else(|| glib::glib_bool_error!("Failed to take buffer")) + } + } + + pub fn take_buffer_list(&self, nbytes: usize) -> Result { + if nbytes > self.available() { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + if nbytes == 0 { + return Ok(gst::BufferList::new()); + } + + unsafe { + Option::<_>::from_glib_full(ffi::gst_adapter_take_buffer_list( + self.to_glib_none().0, + nbytes, + )) + .ok_or_else(|| glib::glib_bool_error!("Failed to take buffer list")) + } + } + + pub fn take_list(&self, nbytes: usize) -> Result, glib::BoolError> { + if nbytes > self.available() { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + if nbytes == 0 { + return Ok(Vec::new()); + } + + unsafe { + Ok(FromGlibPtrContainer::from_glib_full( + ffi::gst_adapter_take_list(self.to_glib_none().0, nbytes), + )) + } } pub fn push(&self, buf: gst::Buffer) { @@ -50,8 +297,11 @@ impl io::Read for Adapter { len = buf.len(); } - self.copy(0, &mut buf[0..len]); - self.flush(len); + self.copy(0, &mut buf[0..len]) + .map_err(|err| io::Error::new(io::ErrorKind::Other, err))?; + + // Can't happen because we checked the size first + self.flush(len).expect("Failed to flush"); Ok(len) } } @@ -80,8 +330,7 @@ impl UniqueAdapter { } pub fn copy_bytes(&self, offset: usize, size: usize) -> Result { - // TBD: https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/issues/298 - Ok(self.0.copy_bytes(offset, size)) + self.0.copy_bytes(offset, size) } #[cfg(any(feature = "v1_10", feature = "dox"))] @@ -96,8 +345,8 @@ impl UniqueAdapter { self.0.dts_at_discont() } - pub fn flush(&mut self, flush: usize) { - self.0.flush(flush); + pub fn flush(&mut self, flush: usize) -> Result<(), glib::BoolError> { + self.0.flush(flush) } pub fn get_buffer(&self, nbytes: usize) -> Result { @@ -112,11 +361,17 @@ impl UniqueAdapter { self.0.get_buffer_list(nbytes) } - pub fn get_list(&self, nbytes: usize) -> Vec { + pub fn get_list(&self, nbytes: usize) -> Result, glib::BoolError> { self.0.get_list(nbytes) } - pub fn masked_scan_uint32(&self, mask: u32, pattern: u32, offset: usize, size: usize) -> isize { + pub fn masked_scan_uint32( + &self, + mask: u32, + pattern: u32, + offset: usize, + size: usize, + ) -> Result, glib::BoolError> { self.0.masked_scan_uint32(mask, pattern, offset, size) } @@ -126,7 +381,7 @@ impl UniqueAdapter { pattern: u32, offset: usize, size: usize, - ) -> (isize, u32) { + ) -> Result, glib::BoolError> { self.0.masked_scan_uint32_peek(mask, pattern, offset, size) } @@ -176,12 +431,12 @@ impl UniqueAdapter { self.0.take_buffer_list(nbytes) } - pub fn take_list(&mut self, nbytes: usize) -> Vec { + pub fn take_list(&mut self, nbytes: usize) -> Result, glib::BoolError> { self.0.take_list(nbytes) } - pub fn copy(&self, offset: usize, dest: &mut [u8]) { - self.0.copy(offset, dest); + pub fn copy(&self, offset: usize, dest: &mut [u8]) -> Result<(), glib::BoolError> { + self.0.copy(offset, dest) } pub fn push(&mut self, buf: gst::Buffer) { @@ -191,13 +446,21 @@ impl UniqueAdapter { pub fn map(&mut self, nbytes: usize) -> Result { use std::slice; + if nbytes > self.available() { + return Err(glib::glib_bool_error!("Not enough data available")); + } + + if nbytes == 0 { + return Ok(UniqueAdapterMap(None, &[])); + } + unsafe { let ptr = ffi::gst_adapter_map(self.0.to_glib_none().0, nbytes); if ptr.is_null() { Err(glib::glib_bool_error!("size bytes are not available")) } else { Ok(UniqueAdapterMap( - self, + Some(self), slice::from_raw_parts(ptr as *const u8, nbytes), )) } @@ -206,12 +469,14 @@ impl UniqueAdapter { } #[derive(Debug)] -pub struct UniqueAdapterMap<'a>(&'a UniqueAdapter, &'a [u8]); +pub struct UniqueAdapterMap<'a>(Option<&'a UniqueAdapter>, &'a [u8]); impl<'a> Drop for UniqueAdapterMap<'a> { fn drop(&mut self) { - unsafe { - ffi::gst_adapter_unmap((self.0).0.to_glib_none().0); + if let Some(adapter) = self.0 { + unsafe { + ffi::gst_adapter_unmap(adapter.0.to_glib_none().0); + } } } } diff --git a/gstreamer-base/src/auto/adapter.rs b/gstreamer-base/src/auto/adapter.rs index 1b7dbffb8..90d98b32c 100644 --- a/gstreamer-base/src/auto/adapter.rs +++ b/gstreamer-base/src/auto/adapter.rs @@ -34,16 +34,6 @@ impl Adapter { } } - pub fn copy_bytes(&self, offset: usize, size: usize) -> glib::Bytes { - unsafe { - from_glib_full(ffi::gst_adapter_copy_bytes( - self.to_glib_none().0, - offset, - size, - )) - } - } - #[cfg(any(feature = "v1_10", feature = "dox"))] #[cfg_attr(feature = "dox", doc(cfg(feature = "v1_10")))] pub fn distance_from_discont(&self) -> u64 { @@ -56,76 +46,6 @@ impl Adapter { unsafe { from_glib(ffi::gst_adapter_dts_at_discont(self.to_glib_none().0)) } } - pub fn flush(&self, flush: usize) { - unsafe { - ffi::gst_adapter_flush(self.to_glib_none().0, flush); - } - } - - pub fn get_buffer(&self, nbytes: usize) -> Result { - unsafe { - Option::<_>::from_glib_full(ffi::gst_adapter_get_buffer(self.to_glib_none().0, nbytes)) - .ok_or_else(|| glib::glib_bool_error!("Failed to get buffer")) - } - } - - pub fn get_buffer_fast(&self, nbytes: usize) -> Result { - unsafe { - Option::<_>::from_glib_full(ffi::gst_adapter_get_buffer_fast( - self.to_glib_none().0, - nbytes, - )) - .ok_or_else(|| glib::glib_bool_error!("Failed to get buffer")) - } - } - - pub fn get_buffer_list(&self, nbytes: usize) -> Result { - unsafe { - Option::<_>::from_glib_full(ffi::gst_adapter_get_buffer_list( - self.to_glib_none().0, - nbytes, - )) - .ok_or_else(|| glib::glib_bool_error!("Failed to get buffer list")) - } - } - - pub fn get_list(&self, nbytes: usize) -> Vec { - unsafe { - FromGlibPtrContainer::from_glib_full(ffi::gst_adapter_get_list( - self.to_glib_none().0, - nbytes, - )) - } - } - - pub fn masked_scan_uint32(&self, mask: u32, pattern: u32, offset: usize, size: usize) -> isize { - unsafe { - ffi::gst_adapter_masked_scan_uint32(self.to_glib_none().0, mask, pattern, offset, size) - } - } - - pub fn masked_scan_uint32_peek( - &self, - mask: u32, - pattern: u32, - offset: usize, - size: usize, - ) -> (isize, u32) { - unsafe { - let mut value = mem::MaybeUninit::uninit(); - let ret = ffi::gst_adapter_masked_scan_uint32_peek( - self.to_glib_none().0, - mask, - pattern, - offset, - size, - value.as_mut_ptr(), - ); - let value = value.assume_init(); - (ret, value) - } - } - #[cfg(any(feature = "v1_10", feature = "dox"))] #[cfg_attr(feature = "dox", doc(cfg(feature = "v1_10")))] pub fn offset_at_discont(&self) -> u64 { @@ -198,42 +118,6 @@ impl Adapter { pub fn pts_at_discont(&self) -> gst::ClockTime { unsafe { from_glib(ffi::gst_adapter_pts_at_discont(self.to_glib_none().0)) } } - - pub fn take_buffer(&self, nbytes: usize) -> Result { - unsafe { - Option::<_>::from_glib_full(ffi::gst_adapter_take_buffer(self.to_glib_none().0, nbytes)) - .ok_or_else(|| glib::glib_bool_error!("Failed to take buffer")) - } - } - - pub fn take_buffer_fast(&self, nbytes: usize) -> Result { - unsafe { - Option::<_>::from_glib_full(ffi::gst_adapter_take_buffer_fast( - self.to_glib_none().0, - nbytes, - )) - .ok_or_else(|| glib::glib_bool_error!("Failed to take buffer")) - } - } - - pub fn take_buffer_list(&self, nbytes: usize) -> Result { - unsafe { - Option::<_>::from_glib_full(ffi::gst_adapter_take_buffer_list( - self.to_glib_none().0, - nbytes, - )) - .ok_or_else(|| glib::glib_bool_error!("Failed to take buffer list")) - } - } - - pub fn take_list(&self, nbytes: usize) -> Vec { - unsafe { - FromGlibPtrContainer::from_glib_full(ffi::gst_adapter_take_list( - self.to_glib_none().0, - nbytes, - )) - } - } } impl Default for Adapter {