From 59fb850181a67cad458c7d33508ce8ada3c4e51e Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Thu, 10 Dec 2020 14:53:24 +0100 Subject: [PATCH] Handle overflow errors in Bytes -> Pages conversion --- CHANGELOG.md | 1 + Cargo.lock | 1 + lib/vm/src/memory.rs | 4 +-- lib/wasmer-types/Cargo.toml | 1 + lib/wasmer-types/src/lib.rs | 4 ++- lib/wasmer-types/src/units.rs | 50 ++++++++++++++++++++++++++++++++--- 6 files changed, 55 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8c5cc0b61d..e093356fb2b 100644 --- a/CHANGELOG.md +++ b/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 for Pages` instead of `From for Pages` to properly handle overflow errors ## 1.0.0-beta1 - 2020-12-01 diff --git a/Cargo.lock b/Cargo.lock index 3046aafb0a4..e67b235f73f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2723,6 +2723,7 @@ version = "1.0.0-beta1" dependencies = [ "cranelift-entity 0.68.0", "serde", + "thiserror", ] [[package]] diff --git a/lib/vm/src/memory.rs b/lib/vm/src/memory.rs index 137c03f6b34..96b3292d746 100644 --- a/lib/vm/src/memory.rs +++ b/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 = diff --git a/lib/wasmer-types/Cargo.toml b/lib/wasmer-types/Cargo.toml index d80945d7de2..3504a23c962 100644 --- a/lib/wasmer-types/Cargo.toml +++ b/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"] diff --git a/lib/wasmer-types/src/lib.rs b/lib/wasmer-types/src/lib.rs index 8e158f1bf6d..c197486dc4c 100644 --- a/lib/wasmer-types/src/lib.rs +++ b/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, diff --git a/lib/wasmer-types/src/units.rs b/lib/wasmer-types/src/units.rs index 22aed031141..93e9f04d473 100644 --- a/lib/wasmer-types/src/units.rs +++ b/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 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 for Pages { + type Error = PageCountOutOfRange; + + fn try_from(bytes: Bytes) -> Result { + 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); + } +}