Ensure we comply with the non-NULL req for slice::from_raw_parts

When building a slice with `slice::from_raw_parts`, we're required
to pass in a non-NULL pointer even if the length is 0. While we did
this in most cases, we weren't always careful about it. Here we fix
the issue by using a simple wrapper which passes a dummy pointer
instead.

Author: TheBlueMatt

c-bindings-gen/src/blocks.rs

@@ -226,13 +226,13 @@ pub fn write_vec_block<W: std::io::Write>(w: &mut W, mangled_container: &str, in
 	writeln!(w, "impl {} {{", mangled_container).unwrap();
 	writeln!(w, "\t#[allow(unused)] pub(crate) fn into_rust(&mut self) -> Vec<{}> {{", inner_type).unwrap();
 	writeln!(w, "\t\tif self.datalen == 0 {{ return Vec::new(); }}").unwrap();
-	writeln!(w, "\t\tlet ret = unsafe {{ Box::from_raw(core::slice::from_raw_parts_mut(self.data, self.datalen)) }}.into();").unwrap();
+	writeln!(w, "\t\tlet ret = unsafe {{ Box::from_raw(crate::c_types::from_raw_parts_safer_mut(self.data, self.datalen)) }}.into();").unwrap();
 	writeln!(w, "\t\tself.data = core::ptr::null_mut();").unwrap();
 	writeln!(w, "\t\tself.datalen = 0;").unwrap();
 	writeln!(w, "\t\tret").unwrap();
 	writeln!(w, "\t}}").unwrap();
 	writeln!(w, "\t#[allow(unused)] pub(crate) fn as_slice(&self) -> &[{}] {{", inner_type).unwrap();
-	writeln!(w, "\t\tunsafe {{ core::slice::from_raw_parts_mut(self.data, self.datalen) }}").unwrap();
+	writeln!(w, "\t\tunsafe {{ crate::c_types::from_raw_parts_safer_mut(self.data, self.datalen) }}").unwrap();
 	writeln!(w, "\t}}").unwrap();
 	writeln!(w, "}}").unwrap();
 
@@ -250,15 +250,15 @@ pub fn write_vec_block<W: std::io::Write>(w: &mut W, mangled_container: &str, in
 	writeln!(w, "impl Drop for {} {{", mangled_container).unwrap();
 	writeln!(w, "\tfn drop(&mut self) {{").unwrap();
 	writeln!(w, "\t\tif self.datalen == 0 {{ return; }}").unwrap();
-	writeln!(w, "\t\tlet _ = unsafe {{ Box::from_raw(core::slice::from_raw_parts_mut(self.data, self.datalen)) }};").unwrap();
+	writeln!(w, "\t\tlet _ = unsafe {{ Box::from_raw(crate::c_types::from_raw_parts_safer_mut(self.data, self.datalen)) }};").unwrap();
 	writeln!(w, "\t}}").unwrap();
 	writeln!(w, "}}").unwrap();
 	if clonable {
 		writeln!(w, "impl Clone for {} {{", mangled_container).unwrap();
 		writeln!(w, "\tfn clone(&self) -> Self {{").unwrap();
 		writeln!(w, "\t\tlet mut res = Vec::new();").unwrap();
 		writeln!(w, "\t\tif self.datalen == 0 {{ return Self::from(res); }}").unwrap();
-		writeln!(w, "\t\tres.extend_from_slice(unsafe {{ core::slice::from_raw_parts_mut(self.data, self.datalen) }});").unwrap();
+		writeln!(w, "\t\tres.extend_from_slice(unsafe {{ crate::c_types::from_raw_parts_safer_mut(self.data, self.datalen) }});").unwrap();
 		writeln!(w, "\t\tSelf::from(res)").unwrap();
 		writeln!(w, "\t}}").unwrap();
 		writeln!(w, "}}").unwrap();

lightning-c-bindings/src/c_types/mod.rs

@@ -35,6 +35,30 @@ use alloc::{boxed::Box, vec::Vec, string::String};
 
 use core::convert::TryFrom;
 
+/// [`core::slice::from_raw_parts`] requires the pointer to be non-NULL even if the len is 0, which
+/// is annoying, so we wrap it here and fill in garbage in that case.
+pub(crate) unsafe fn from_raw_parts_safer_mut<'a, T>(data: *mut T, len: usize) -> &'a mut [T] {
+	if len == 0 {
+		core::slice::from_raw_parts_mut(core::ptr::NonNull::dangling().as_ptr(), len)
+	} else {
+		assert!(data != core::ptr::null_mut());
+		assert!(data != core::ptr::NonNull::dangling().as_ptr());
+		core::slice::from_raw_parts_mut(data, len)
+	}
+}
+
+/// [`core::slice::from_raw_parts`] requires the pointer to be non-NULL even if the len is 0, which
+/// is annoying, so we wrap it here and fill in garbage in that case.
+pub(crate) unsafe fn from_raw_parts_safer<'a, T>(data: *const T, len: usize) -> &'a [T] {
+	if len == 0 {
+		core::slice::from_raw_parts(core::ptr::NonNull::dangling().as_ptr(), len)
+	} else {
+		assert!(data != core::ptr::null());
+		assert!(data != core::ptr::NonNull::dangling().as_ptr());
+		core::slice::from_raw_parts(data, len)
+	}
+}
+
 #[repr(C)]
 /// A dummy struct of which an instance must never exist.
 /// This corresponds to the Rust type `Infallible`, or, in unstable rust, `!`
@@ -575,7 +599,7 @@ impl Transaction {
 	}
 	pub(crate) fn into_bitcoin(&self) -> BitcoinTransaction {
 		if self.datalen == 0 { panic!("0-length buffer can never represent a valid Transaction"); }
-		::bitcoin::consensus::encode::deserialize(unsafe { core::slice::from_raw_parts(self.data, self.datalen) }).unwrap()
+		::bitcoin::consensus::encode::deserialize(unsafe { from_raw_parts_safer(self.data, self.datalen) }).unwrap()
 	}
 	pub(crate) fn from_bitcoin(btc: &BitcoinTransaction) -> Self {
 		let vec = ::bitcoin::consensus::encode::serialize(btc);
@@ -591,7 +615,7 @@ impl Drop for Transaction {
 }
 impl Clone for Transaction {
 	fn clone(&self) -> Self {
-		let sl = unsafe { core::slice::from_raw_parts(self.data, self.datalen) };
+		let sl = unsafe { from_raw_parts_safer(self.data, self.datalen) };
 		let mut v = Vec::new();
 		v.extend_from_slice(&sl);
 		Self::from_vec(v)
@@ -624,7 +648,7 @@ impl Witness {
 		}
 	}
 	pub(crate) fn into_bitcoin(&self) -> BitcoinWitness {
-		::bitcoin::consensus::encode::deserialize(unsafe { core::slice::from_raw_parts(self.data, self.datalen) }).unwrap()
+		::bitcoin::consensus::encode::deserialize(unsafe { from_raw_parts_safer(self.data, self.datalen) }).unwrap()
 	}
 	pub(crate) fn from_bitcoin(btc: &BitcoinWitness) -> Self {
 		let vec = ::bitcoin::consensus::encode::serialize(btc);
@@ -641,7 +665,7 @@ impl Drop for Witness {
 }
 impl Clone for Witness {
 	fn clone(&self) -> Self {
-		let sl = unsafe { core::slice::from_raw_parts(self.data, self.datalen) };
+		let sl = unsafe { from_raw_parts_safer(self.data, self.datalen) };
 		let mut v = Vec::new();
 		v.extend_from_slice(&sl);
 		Self::from_vec(v)
@@ -797,12 +821,7 @@ impl u8slice {
 		}
 	}
 	pub(crate) fn to_slice(&self) -> &[u8] {
-		if self.datalen == 0 { return &[]; }
-		unsafe { core::slice::from_raw_parts(self.data, self.datalen) }
-	}
-	pub(crate) fn to_reader<'a>(&'a self) -> Cursor<&'a [u8]> {
-		let sl = self.to_slice();
-		Cursor::new(sl)
+		unsafe { from_raw_parts_safer(self.data, self.datalen) }
 	}
 	pub(crate) fn from_vec(v: &derived::CVec_u8Z) -> u8slice {
 		Self::from_slice(v.as_slice())
@@ -897,20 +916,20 @@ impl Into<Str> for &mut &str {
 impl Str {
 	pub(crate) fn into_str(&self) -> &'static str {
 		if self.len == 0 { return ""; }
-		core::str::from_utf8(unsafe { core::slice::from_raw_parts(self.chars, self.len) }).unwrap()
+		core::str::from_utf8(unsafe { from_raw_parts_safer(self.chars, self.len) }).unwrap()
 	}
 	pub(crate) fn into_string(mut self) -> String {
 		let bytes = if self.len == 0 {
 			Vec::new()
 		} else if self.chars_is_owned {
 			let ret = unsafe {
-				Box::from_raw(core::slice::from_raw_parts_mut(unsafe { self.chars as *mut u8 }, self.len))
+				Box::from_raw(from_raw_parts_safer_mut(unsafe { self.chars as *mut u8 }, self.len))
 			}.into();
 			self.chars_is_owned = false;
 			ret
 		} else {
 			let mut ret = Vec::with_capacity(self.len);
-			ret.extend_from_slice(unsafe { core::slice::from_raw_parts(self.chars, self.len) });
+			ret.extend_from_slice(unsafe { from_raw_parts_safer(self.chars, self.len) });
 			ret
 		};
 		String::from_utf8(bytes).unwrap()