Merge #1914

1914: Handle overflow errors in Bytes -> Pages conversion r=syrusakbary a=webmaster128

# Description

Before the Bytes to Pages conversion silently returned wrong results when the number of pages was too large.

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Simon Warta <simon@warta.it>

CHANGELOG.md

@@ -21,6 +21,7 @@
 ### Fixed
 
 - [#1870](https://github.com/wasmerio/wasmer/pull/1870) Fixed Trap instruction address maps in Singlepass
+* [#1914](https://github.com/wasmerio/wasmer/pull/1914) Implemented `TryFrom<Bytes> for Pages` instead of `From<Bytes> for Pages` to properly handle overflow errors
 
 ## 1.0.0-beta1 - 2020-12-01
 

lib/vm/src/memory.rs

@@ -318,7 +318,7 @@ impl Memory for LinearMemory {
         unsafe {
             let md_ptr = self.get_vm_memory_definition();
             let md = md_ptr.as_ref();
-            Bytes::from(md.current_length).into()
+            Bytes::from(md.current_length).try_into().unwrap()
         }
     }
 
@@ -376,7 +376,7 @@ impl Memory for LinearMemory {
                     .checked_add(guard_bytes)
                     .ok_or_else(|| MemoryError::CouldNotGrow {
                         current: new_pages,
-                        attempted_delta: Bytes(guard_bytes).into(),
+                        attempted_delta: Bytes(guard_bytes).try_into().unwrap(),
                     })?;
 
             let mut new_mmap =

lib/wasmer-types/Cargo.toml

@@ -15,6 +15,7 @@ edition = "2018"
 # some useful data structures
 cranelift-entity = "0.68"
 serde = { version = "1.0", features = ["derive"], optional = true, default-features = false }
+thiserror = "1.0"
 
 [features]
 default = ["std", "enable-serde"]

lib/wasmer-types/src/lib.rs

@@ -77,7 +77,9 @@ pub use crate::initializers::{
 pub use crate::memory_view::{Atomically, MemoryView};
 pub use crate::native::{NativeWasmType, ValueType};
 pub use crate::r#ref::{ExternRef, HostInfo, HostRef};
-pub use crate::units::{Bytes, Pages, WASM_MAX_PAGES, WASM_MIN_PAGES, WASM_PAGE_SIZE};
+pub use crate::units::{
+    Bytes, PageCountOutOfRange, Pages, WASM_MAX_PAGES, WASM_MIN_PAGES, WASM_PAGE_SIZE,
+};
 pub use crate::values::Value;
 pub use types::{
     ExportType, ExternType, FunctionType, GlobalInit, GlobalType, ImportType, MemoryType,

lib/wasmer-types/src/units.rs

@@ -1,8 +1,10 @@
+use crate::lib::std::convert::TryFrom;
 use crate::lib::std::fmt;
 use crate::lib::std::ops::{Add, Sub};
 #[cfg(feature = "enable-serde")]
 use serde::{Deserialize, Serialize};
 use std::convert::TryInto;
+use thiserror::Error;
 
 /// WebAssembly page sizes are fixed to be 64KiB.
 /// Note: large page support may be added in an opt-in manner in the [future].
@@ -108,9 +110,19 @@ where
     }
 }
 
-impl From<Bytes> for Pages {
-    fn from(bytes: Bytes) -> Self {
-        Self((bytes.0 / WASM_PAGE_SIZE) as u32)
+/// The only error that can happen when converting `Bytes` to `Pages`
+#[derive(Debug, Clone, Copy, PartialEq, Error)]
+#[error("Number of pages exceeds uint32 range")]
+pub struct PageCountOutOfRange;
+
+impl TryFrom<Bytes> for Pages {
+    type Error = PageCountOutOfRange;
+
+    fn try_from(bytes: Bytes) -> Result<Self, Self::Error> {
+        let pages: u32 = (bytes.0 / WASM_PAGE_SIZE)
+            .try_into()
+            .or(Err(PageCountOutOfRange))?;
+        Ok(Self(pages))
     }
 }
 
@@ -133,3 +145,35 @@ where
         Self(self.0 + rhs.into().0)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn convert_bytes_to_pages() {
+        // rounds down
+        let pages = Pages::try_from(Bytes(0)).unwrap();
+        assert_eq!(pages, Pages(0));
+        let pages = Pages::try_from(Bytes(1)).unwrap();
+        assert_eq!(pages, Pages(0));
+        let pages = Pages::try_from(Bytes(WASM_PAGE_SIZE - 1)).unwrap();
+        assert_eq!(pages, Pages(0));
+        let pages = Pages::try_from(Bytes(WASM_PAGE_SIZE)).unwrap();
+        assert_eq!(pages, Pages(1));
+        let pages = Pages::try_from(Bytes(WASM_PAGE_SIZE + 1)).unwrap();
+        assert_eq!(pages, Pages(1));
+        let pages = Pages::try_from(Bytes(28 * WASM_PAGE_SIZE + 42)).unwrap();
+        assert_eq!(pages, Pages(28));
+        let pages = Pages::try_from(Bytes((u32::MAX as usize) * WASM_PAGE_SIZE)).unwrap();
+        assert_eq!(pages, Pages(u32::MAX));
+        let pages = Pages::try_from(Bytes((u32::MAX as usize) * WASM_PAGE_SIZE + 1)).unwrap();
+        assert_eq!(pages, Pages(u32::MAX));
+
+        // Errors when page count cannot be represented as u32
+        let result = Pages::try_from(Bytes((u32::MAX as usize + 1) * WASM_PAGE_SIZE));
+        assert_eq!(result.unwrap_err(), PageCountOutOfRange);
+        let result = Pages::try_from(Bytes(usize::MAX));
+        assert_eq!(result.unwrap_err(), PageCountOutOfRange);
+    }
+}