Fix memory leak in runtime interface (#4837)

* Fix memory leak in runtime interface

We used `slice::from_raw_parts` in runtime-interface which did not free
the memory afterwards. This pr changes it to `Vec::from_raw_parts` to
make sure `drop` is called properly and the values are freed.

* Check that `len` is non-zero

* Adds comment

substrate/primitives/runtime-interface/src/impls.rs

@@ -38,9 +38,6 @@ use sp_std::{any::TypeId, mem, vec::Vec};
 #[cfg(feature = "std")]
 use sp_std::borrow::Cow;
 
-#[cfg(not(feature = "std"))]
-use sp_std::{slice, boxed::Box};
-
 // Make sure that our assumptions for storing a pointer + its size in `u64` is valid.
 #[cfg(all(not(feature = "std"), not(feature = "disable_target_static_assertions")))]
 assert_eq_size!(usize, u32);
@@ -196,11 +193,16 @@ impl<T: 'static + Decode> FromFFIValue for Vec<T> {
 		let (ptr, len) = unpack_ptr_and_len(arg);
 		let len = len as usize;
 
+		if len == 0 {
+			return Vec::new();
+		}
+
+		let data = unsafe { Vec::from_raw_parts(ptr as *mut u8, len, len) };
+
 		if TypeId::of::<T>() == TypeId::of::<u8>() {
-			unsafe { mem::transmute(Vec::from_raw_parts(ptr as *mut u8, len, len)) }
+			unsafe { mem::transmute(data) }
 		} else {
-			let slice = unsafe { slice::from_raw_parts(ptr as *const u8, len) };
-			Self::decode(&mut &slice[..]).expect("Host to wasm values are encoded correctly; qed")
+			Self::decode(&mut &data[..]).expect("Host to wasm values are encoded correctly; qed")
 		}
 	}
 }
@@ -302,10 +304,9 @@ macro_rules! impl_traits_for_arrays {
 			impl FromFFIValue for [u8; $n] {
 				fn from_ffi_value(arg: u32) -> [u8; $n] {
 					let mut res = [0u8; $n];
-					res.copy_from_slice(unsafe { slice::from_raw_parts(arg as *const u8, $n) });
+					let data = unsafe { Vec::from_raw_parts(arg as *mut u8, $n, $n) };
 
-					// Make sure we free the pointer.
-					let _ = unsafe { Box::from_raw(arg as *mut u8) };
+					res.copy_from_slice(&data);
 
 					res
 				}

substrate/primitives/runtime-interface/src/pass_by.rs

@@ -32,7 +32,7 @@ use sp_wasm_interface::{FunctionContext, Pointer, Result};
 use sp_std::{marker::PhantomData, convert::TryFrom};
 
 #[cfg(not(feature = "std"))]
-use sp_std::{slice, vec::Vec};
+use sp_std::vec::Vec;
 
 /// Derive macro for implementing [`PassBy`] with the [`Codec`] strategy.
 ///
@@ -255,8 +255,13 @@ impl<T: codec::Codec> PassByImpl<T> for Codec<T> {
 		let (ptr, len) = unpack_ptr_and_len(arg);
 		let len = len as usize;
 
-		let slice = unsafe { slice::from_raw_parts(ptr as *const u8, len) };
-		T::decode(&mut &slice[..]).expect("Host to wasm values are encoded correctly; qed")
+		let encoded = if len == 0 {
+			Vec::new()
+		} else {
+			unsafe { Vec::from_raw_parts(ptr as *mut u8, len, len) }
+		};
+
+		T::decode(&mut &encoded[..]).expect("Host to wasm values are encoded correctly; qed")
 	}
 }
 

substrate/primitives/runtime-interface/test-wasm/src/lib.rs

@@ -39,6 +39,17 @@ pub trait TestApi {
 		data
 	}
 
+	/// Returns 16kb data.
+	///
+	/// # Note
+	///
+	/// We return a `Vec<u32>` because this will use the code path that uses SCALE
+	/// to pass the data between native/wasm. (Vec<u8> is passed without encoding the
+	/// data)
+	fn return_16kb() -> Vec<u32> {
+		vec![0; 4 * 1024]
+	}
+
 	/// Set the storage at key with value.
 	fn set_storage(&mut self, key: &[u8], data: &[u8]) {
 		self.place_storage(key.to_vec(), Some(data.to_vec()));
@@ -211,4 +222,28 @@ wasm_export_functions! {
 			assert_eq!(*val, test_api::get_and_return_i128(*val));
 		}
 	}
+
+	fn test_vec_return_value_memory_is_freed() {
+		let mut len = 0;
+		for _ in 0..1024 {
+			len += test_api::return_16kb().len();
+		}
+		assert_eq!(1024 * 1024 * 4, len);
+	}
+
+	fn test_encoded_return_value_memory_is_freed() {
+		let mut len = 0;
+		for _ in 0..1024 {
+			len += test_api::return_option_input(vec![0; 16 * 1024]).map(|v| v.len()).unwrap();
+		}
+		assert_eq!(1024 * 1024 * 16, len);
+	}
+
+	fn test_array_return_value_memory_is_freed() {
+		let mut len = 0;
+		for _ in 0..1024 * 1024 {
+			len += test_api::get_and_return_array([0; 34])[1];
+		}
+		assert_eq!(0, len);
+	}
 }

substrate/primitives/runtime-interface/test/src/lib.rs

@@ -113,3 +113,18 @@ fn test_overwrite_native_function_implementation() {
 fn test_u128_i128_as_parameter_and_return_value() {
 	call_wasm_method::<HostFunctions>("test_u128_i128_as_parameter_and_return_value");
 }
+
+#[test]
+fn test_vec_return_value_memory_is_freed() {
+	call_wasm_method::<HostFunctions>("test_vec_return_value_memory_is_freed");
+}
+
+#[test]
+fn test_encoded_return_value_memory_is_freed() {
+	call_wasm_method::<HostFunctions>("test_encoded_return_value_memory_is_freed");
+}
+
+#[test]
+fn test_array_return_value_memory_is_freed() {
+	call_wasm_method::<HostFunctions>("test_array_return_value_memory_is_freed");
+}