Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cranelift: riscv64 having memory side-effects when trapping #8216

Closed
candymate opened this issue Mar 22, 2024 · 2 comments
Closed

Cranelift: riscv64 having memory side-effects when trapping #8216

candymate opened this issue Mar 22, 2024 · 2 comments
Labels
bug Incorrect behavior in the current implementation that needs fixing fuzz-bug Bugs found by a fuzzer

Comments

@candymate
Copy link

Test Case

// main.rs
use wasmtime::*;

fn main() -> Result<()> {
    let mut config = Config::default();
    config.strategy(Strategy::Cranelift);
    config.cranelift_opt_level(OptLevel::None);

    let engine = Engine::new(&config)?;
    let wat = r#"
    (module
        (type (;0;) (func (param) (result)))
        (import "mem" "mem" (memory (;0;) 1))
        (func (;0;) (type 0) (param) (result)
          i32.const 65521
          v128.const i32x4 0xffffffff 0xffffffff 0xffffffff 0xffffffff
          v128.store align=1)
        (export "main" (func 0)))
    "#;
    let module = Module::new(&engine, wat)?;
    let mut store = Store::new(&engine, ());

    let memory_ty = MemoryType::new(1, None);
    let memory = Memory::new(&mut store, memory_ty.clone())?;

    let instance = Instance::new(&mut store, &module, &[memory.into()])?;
    let main = instance.get_func(&mut store, "main")
        .expect("`main` was not an exported function");
    let params = vec![];
    let mut results = vec![];
    println!("Opt level None: {:?}", main.call(
        &mut store,
        &params,
        &mut results
    ));
    for i in 65521..65536 {
        print!("{} ", memory.data(&store)[i]);
    }
    println!("");

    Ok(())
}
[package]
name = "wasmtime-wrapper"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
# wasmtime = "19.0.0"
wasmtime = { path = "../wasmtime/crates/wasmtime" } # commit: 6c5184809db3a92de4ee0c718c403bedc9a9ff4f (current latest, Date:   Thu Mar 21 18:24:49 2024 -0700)

Steps to reproduce

Compare the following executions:

cargo run --release
cargo run --release --target=riscv64gc-unknown-linux-gnu

QEMU run options (riscv64) I'm currently using is the following:

qemu-riscv64 -cpu rv64,v=true,vlen=128,vext_spec=v1.0,zba=true,zbb=true,zbs=true,zbc=true,zbkb=true,zcb=true,zicond=true -L /usr/riscv64-linux-gnu -E LD_LIBRARY_PATH=/usr/riscv64-linux-gnu/lib -E WASMTIME_TEST_NO_HOG_MEMORY=1 target/riscv64gc-unknown-linux-gnu/release/wasmtime-wrapper

Expected Results

RISC-V result should not leave side-effect when trapping. By looking at the spec, we can know that store instruction should check first if the memory address (offset + byte-width) is valid, then perform the memory operation.

Opt level None: Err(error while executing at wasm backtrace:
    0:   0x45 - <unknown>!<wasm function 0>

Caused by:
    0: memory fault at wasm address 0x10000 in linear memory of size 0x10000
    1: wasm trap: out of bounds memory access)
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0

Actual Results

RISC-V leaves memory side-effects when the program traps due to invalid memory address. The PoC code posted above leaves 15 bytes of 255 at the end of the memory. (indices from 65521 to 65535)

Opt level None: Err(error while executing at wasm backtrace:
    0:   0x45 - <unknown>!<wasm function 0>

Caused by:
    0: memory fault at wasm address 0x10000 in linear memory of size 0x10000
    1: wasm trap: out of bounds memory access)
255 255 255 255 255 255 255 255 255 255 255 255 255 255 255

Versions and Environment

  • wasmtime version
    • commit: 6c51848(current latest, Date: Thu Mar 21 18:24:49 2024 -0700)
    • However, also checked on v19.0.0
  • Operating system & architecture: Ubuntu 22.04.3 LTS, Arch: x86_64
  • QEMU version: qemu-riscv64 version 8.2.1 (v8.2.1)

Extra Info

  • Works fine on other architectures (x86_64, aarch64, s390x)
  • RISC-V is not the tier 1 platform, so releasing this bug as public (not a security bug)
@candymate candymate added bug Incorrect behavior in the current implementation that needs fixing fuzz-bug Bugs found by a fuzzer labels Mar 22, 2024
@afonso360
Copy link
Contributor

afonso360 commented Mar 22, 2024

If I'm not mistaken this is the same as #7237, right? We are writing to an unaligned address, crossing a page boundary and one of the pages triggers a fault.

I'm surprised that it works on AArch64 since that one is also supposed to be affected by this.

@alexcrichton
Copy link
Member

I would agree with @afonso360, but thanks regardless @candymate!

I'm going to close this in favor of that issue, and I'll also drop a link to WebAssembly/design#1490 which is upstream spec discussion on this topic too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior in the current implementation that needs fixing fuzz-bug Bugs found by a fuzzer
Projects
None yet
Development

No branches or pull requests

3 participants