Skip to content

Harden Wasmtime's compiled GC code against corruption #13321

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.

Sign up for GitHub

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

Open
alexcrichton wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Harden Wasmtime's compiled GC code against corruption #13321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions crates/cli-flags/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,13 @@ wasmtime_option_group! {
#[serde(deserialize_with = "crate::opt::cli_parse_wrapper")]
pub inlining: Option<wasmtime::Inlining>,

/// Whether or not trap metadata is present for wasm internal assertions
/// in compiled code.
pub metadata_for_internal_asserts: Option<bool>,
/// Whether or not trap metadata is present for detection of gc
/// corruption in compiled code.
pub metadata_for_gc_heap_corruption: Option<bool>,

#[prefixed = "cranelift"]
#[serde(default)]
/// Set a cranelift-specific option. Use `wasmtime settings` to see
Expand Down Expand Up @@ -975,6 +982,12 @@ impl CommonOptions {
if let Some(enable) = self.codegen.inlining {
config.compiler_inlining(enable);
}
if let Some(enable) = self.codegen.metadata_for_internal_asserts {
config.metadata_for_internal_asserts(enable);
}
if let Some(enable) = self.codegen.metadata_for_gc_heap_corruption {
config.metadata_for_gc_heap_corruption(enable);
}

// async_stack_size enabled by either async or stack-switching, so
// cannot directly use match_feature!
Expand Down
11 changes: 9 additions & 2 deletions crates/cranelift/src/compiled_function.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use cranelift_codegen::{
};
use wasmtime_environ::{
FilePos, FrameStateSlotBuilder, InstructionAddressMap, ModulePC, PrimaryMap, TrapInformation,
Tunables,
};

#[derive(Debug, Clone, PartialEq, Eq, Default)]
Expand Down Expand Up @@ -144,8 +145,14 @@ impl CompiledFunction {
}

/// Returns an iterator to the function's trap information.
pub fn traps(&self) -> impl Iterator<Item = TrapInformation> + '_ {
self.buffer.traps().iter().filter_map(mach_trap_to_trap)
pub fn traps<'a>(
&'a self,
tunables: &'a Tunables,
) -> impl Iterator<Item = TrapInformation> + 'a {
self.buffer
.traps()
.iter()
.filter_map(move |t| mach_trap_to_trap(t, tunables))
}

/// Get the function's address map from the metadata.
Expand Down
5 changes: 4 additions & 1 deletion crates/cranelift/src/compiler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,10 @@ impl wasmtime_environ::Compiler for Compiler {
func.buffer.user_stack_maps(),
);

traps.push(range.clone(), &func.traps().collect::<Vec<_>>());
traps.push(
range.clone(),
&func.traps(&self.tunables).collect::<Vec<_>>(),
);
clif_to_env_exception_tables(
&mut exception_tables,
range.clone(),
Expand Down
56 changes: 21 additions & 35 deletions crates/cranelift/src/func_environ/gc/enabled.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::bounds_checks::BoundsCheck;
use crate::func_environ::{Extension, FuncEnvironment};
use crate::translate::{Heap, HeapData, MemoryKind, StructFieldsVec, TargetEnvironment};
use crate::trap::TranslateTrap;
use crate::{Reachability, TRAP_INTERNAL_ASSERT};
use crate::{Reachability, TRAP_GC_HEAP_CORRUPT, TRAP_INTERNAL_ASSERT};
use cranelift_codegen::ir::immediates::Offset32;
use cranelift_codegen::ir::{BlockArg, ExceptionTableData, ExceptionTableItem};
use cranelift_codegen::{
Expand All @@ -27,6 +27,9 @@ mod null;
#[cfg(feature = "gc-copying")]
mod copying;

const GC_MEMFLAGS: ir::MemFlags =
ir::MemFlags::new().with_trap_code(Some(crate::TRAP_GC_HEAP_CORRUPT));

/// Get the default GC compiler.
pub fn gc_compiler(func_env: &mut FuncEnvironment<'_>) -> WasmResult<Box<dyn GcCompiler>> {
// If this function requires a GC compiler, that is not too bad of an
Expand Down Expand Up @@ -161,12 +164,9 @@ fn emit_gc_kind_assert(
object_size: wasmtime_environ::VM_GC_HEADER_SIZE,
},
);
let kind_and_reserved_bits = builder.ins().load(
ir::types::I32,
ir::MemFlags::trusted().with_readonly(),
kind_addr,
0,
);
let kind_and_reserved_bits = builder
.ins()
.load(ir::types::I32, GC_MEMFLAGS, kind_addr, 0);
let kind_mask = builder
.ins()
.iconst(ir::types::I32, i64::from(VMGcKind::MASK));
Expand Down Expand Up @@ -202,7 +202,7 @@ fn read_field_at_addr(
);

// Data inside GC objects is always little endian.
let flags = ir::MemFlags::trusted().with_endianness(ir::Endianness::Little);
let flags = GC_MEMFLAGS.with_endianness(ir::Endianness::Little);

let value = match ty {
WasmStorageType::I8 => builder.ins().load(ir::types::I8, flags, addr, 0),
Expand Down Expand Up @@ -321,7 +321,7 @@ fn write_field_at_addr(
new_val: ir::Value,
) -> WasmResult<()> {
// Data inside GC objects is always little endian.
let flags = ir::MemFlags::trusted().with_endianness(ir::Endianness::Little);
let flags = GC_MEMFLAGS.with_endianness(ir::Endianness::Little);

match field_ty {
WasmStorageType::I8 => {
Expand Down Expand Up @@ -928,12 +928,9 @@ pub fn translate_array_len(
access_size: u8::try_from(ir::types::I32.bytes()).unwrap(),
},
);
let result = builder.ins().load(
ir::types::I32,
ir::MemFlags::trusted().with_readonly(),
len_field,
0,
);
let result = builder
.ins()
.load(ir::types::I32, GC_MEMFLAGS, len_field, 0);
log::trace!("translate_array_len(..) -> {result:?}");
Ok(result)
}
Expand Down Expand Up @@ -973,7 +970,7 @@ fn emit_array_size_info(
let all_elems_size = builder.ins().imul(one_elem_size, array_len);

let high_bits = builder.ins().ushr_imm(all_elems_size, 32);
builder.ins().trapnz(high_bits, TRAP_INTERNAL_ASSERT);
builder.ins().trapnz(high_bits, TRAP_GC_HEAP_CORRUPT);

let all_elems_size = builder.ins().ireduce(ir::types::I32, all_elems_size);
let base_size = builder
Expand All @@ -982,7 +979,7 @@ fn emit_array_size_info(
let obj_size =
builder
.ins()
.uadd_overflow_trap(all_elems_size, base_size, TRAP_INTERNAL_ASSERT);
.uadd_overflow_trap(all_elems_size, base_size, TRAP_GC_HEAP_CORRUPT);

let one_elem_size = builder.ins().ireduce(ir::types::I32, one_elem_size);

Expand Down Expand Up @@ -1244,12 +1241,9 @@ pub fn translate_ref_test(
object_size: wasmtime_environ::VM_GC_HEADER_SIZE,
},
);
let actual_kind = builder.ins().load(
ir::types::I32,
ir::MemFlags::trusted().with_readonly(),
kind_addr,
0,
);
let actual_kind = builder
.ins()
.load(ir::types::I32, GC_MEMFLAGS, kind_addr, 0);
let expected_kind = builder
.ins()
.iconst(ir::types::I32, i64::from(expected_kind.as_u32()));
Expand Down Expand Up @@ -1301,12 +1295,7 @@ pub fn translate_ref_test(
access_size: func_env.offsets.size_of_vmshared_type_index(),
},
);
let actual_shared_ty = builder.ins().load(
ir::types::I32,
ir::MemFlags::trusted().with_readonly(),
ty_addr,
0,
);
let actual_shared_ty = builder.ins().load(ir::types::I32, GC_MEMFLAGS, ty_addr, 0);

func_env.is_subtype(builder, actual_shared_ty, expected_shared_ty)
}
Expand All @@ -1319,11 +1308,8 @@ pub fn translate_ref_test(
let expected_shared_ty =
func_env.module_interned_to_shared_ty(&mut builder.cursor(), expected_interned_ty);

let actual_shared_ty = func_env.load_funcref_type_index(
&mut builder.cursor(),
ir::MemFlags::trusted().with_readonly(),
val,
);
let actual_shared_ty =
func_env.load_funcref_type_index(&mut builder.cursor(), GC_MEMFLAGS, val);

func_env.is_subtype(builder, actual_shared_ty, expected_shared_ty)
}
Expand Down Expand Up @@ -1594,7 +1580,7 @@ impl FuncEnvironment<'_> {
&gc_heap,
gc_ref,
bounds_check,
crate::TRAP_INTERNAL_ASSERT,
crate::TRAP_GC_HEAP_CORRUPT,
) {
Reachability::Reachable(v) => v,
Reachability::Unreachable => {
Expand Down
6 changes: 2 additions & 4 deletions crates/cranelift/src/func_environ/gc/enabled/copying.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl CopyingCompiler {
val: ir::Value,
) -> WasmResult<()> {
// Data inside GC objects is always little endian.
let flags = ir::MemFlags::trusted().with_endianness(ir::Endianness::Little);
let flags = GC_MEMFLAGS.with_endianness(ir::Endianness::Little);

match ty {
WasmStorageType::Val(WasmValType::Ref(r)) => match r.heap_type.top() {
Expand Down Expand Up @@ -109,9 +109,7 @@ impl GcCompiler for CopyingCompiler {
let object_addr = builder.ins().iadd(base, extended_array_ref);
let len_addr = builder.ins().iadd_imm(object_addr, i64::from(len_offset));
let len = init.len(&mut builder.cursor());
builder
.ins()
.store(ir::MemFlags::trusted(), len, len_addr, 0);
builder.ins().store(GC_MEMFLAGS, len, len_addr, 0);

// Initialize elements.
let len_to_elems_delta = builder.ins().iconst(ptr_ty, i64::from(len_to_elems_delta));
Expand Down
34 changes: 10 additions & 24 deletions crates/cranelift/src/func_environ/gc/enabled/drc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ impl DrcCompiler {
access_size: u8::try_from(ir::types::I64.bytes()).unwrap(),
},
);
builder
.ins()
.load(ir::types::I64, ir::MemFlags::trusted(), pointer, 0)
builder.ins().load(ir::types::I64, GC_MEMFLAGS, pointer, 0)
}

/// Generate code to update the given GC reference's ref count to the new
Expand All @@ -64,9 +62,7 @@ impl DrcCompiler {
access_size: u8::try_from(ir::types::I64.bytes()).unwrap(),
},
);
builder
.ins()
.store(ir::MemFlags::trusted(), new_ref_count, pointer, 0);
builder.ins().store(GC_MEMFLAGS, new_ref_count, pointer, 0);
}

/// Generate code to increment or decrement the given GC reference's ref
Expand Down Expand Up @@ -108,9 +104,7 @@ impl DrcCompiler {

// Load the current first list element, which will be our new next list
// element.
let next = builder
.ins()
.load(ir::types::I32, ir::MemFlags::trusted(), head, 0);
let next = builder.ins().load(ir::types::I32, GC_MEMFLAGS, head, 0);

// Update our object's header to point to `next` and consider itself part of the list.
self.set_next_over_approximated_stack_root(func_env, builder, gc_ref, next);
Expand All @@ -120,9 +114,7 @@ impl DrcCompiler {
self.mutate_ref_count(func_env, builder, gc_ref, 1);

// Commit this object as the new head of the list.
builder
.ins()
.store(ir::MemFlags::trusted(), gc_ref, head, 0);
builder.ins().store(GC_MEMFLAGS, gc_ref, head, 0);
}

/// Load a pointer to the first element of the DRC heap's
Expand All @@ -137,7 +129,7 @@ impl DrcCompiler {
let vmctx = builder.ins().global_value(ptr_ty, vmctx);
builder.ins().load(
ptr_ty,
ir::MemFlags::trusted().with_readonly(),
GC_MEMFLAGS,
vmctx,
i32::from(func_env.offsets.ptr.vmctx_gc_heap_data()),
)
Expand All @@ -163,7 +155,7 @@ impl DrcCompiler {
access_size: u8::try_from(ir::types::I32.bytes()).unwrap(),
},
);
builder.ins().store(ir::MemFlags::trusted(), next, ptr, 0);
builder.ins().store(GC_MEMFLAGS, next, ptr, 0);
}

/// Set the in-over-approximated-stack-roots list bit in a `VMDrcHeader`'s
Expand Down Expand Up @@ -199,9 +191,7 @@ impl DrcCompiler {
access_size: u8::try_from(ir::types::I32.bytes()).unwrap(),
},
);
builder
.ins()
.store(ir::MemFlags::trusted(), new_reserved, ptr, 0);
builder.ins().store(GC_MEMFLAGS, new_reserved, ptr, 0);
}

/// Write to an uninitialized field or element inside a GC object.
Expand All @@ -214,7 +204,7 @@ impl DrcCompiler {
val: ir::Value,
) -> WasmResult<()> {
// Data inside GC objects is always little endian.
let flags = ir::MemFlags::trusted().with_endianness(ir::Endianness::Little);
let flags = GC_MEMFLAGS.with_endianness(ir::Endianness::Little);

match ty {
WasmStorageType::Val(WasmValType::Ref(r)) => match r.heap_type.top() {
Expand Down Expand Up @@ -396,9 +386,7 @@ impl GcCompiler for DrcCompiler {
let object_addr = builder.ins().iadd(base, extended_array_ref);
let len_addr = builder.ins().iadd_imm(object_addr, i64::from(len_offset));
let len = init.len(&mut builder.cursor());
builder
.ins()
.store(ir::MemFlags::trusted(), len, len_addr, 0);
builder.ins().store(GC_MEMFLAGS, len, len_addr, 0);

// Finally, initialize the elements.
let len_to_elems_delta = builder.ins().iconst(ptr_ty, i64::from(len_to_elems_delta));
Expand Down Expand Up @@ -666,9 +654,7 @@ impl GcCompiler for DrcCompiler {
access_size: u8::try_from(ir::types::I32.bytes()).unwrap(),
},
);
let reserved = builder
.ins()
.load(ir::types::I32, ir::MemFlags::trusted(), ptr, 0);
let reserved = builder.ins().load(ir::types::I32, GC_MEMFLAGS, ptr, 0);
let in_set_bit = builder.ins().iconst(
ir::types::I32,
i64::from(wasmtime_environ::drc::HEADER_IN_OVER_APPROX_LIST_BIT),
Expand Down
14 changes: 6 additions & 8 deletions crates/cranelift/src/func_environ/gc/enabled/null.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ impl NullCompiler {
let vmctx = func_env.vmctx_val(&mut builder.cursor());
let ptr_to_next = builder.ins().load(
pointer_type,
ir::MemFlags::trusted().with_readonly(),
GC_MEMFLAGS,
vmctx,
i32::from(func_env.offsets.ptr.vmctx_gc_heap_data()),
);
let next = builder
.ins()
.load(ir::types::I32, ir::MemFlags::trusted(), ptr_to_next, 0);
.load(ir::types::I32, GC_MEMFLAGS, ptr_to_next, 0);

// Increment the bump "pointer" to the requested alignment:
//
Expand Down Expand Up @@ -157,20 +157,20 @@ impl NullCompiler {
),
};
builder.ins().store(
ir::MemFlags::trusted(),
GC_MEMFLAGS,
kind_and_size,
ptr_to_object,
i32::try_from(wasmtime_environ::VM_GC_HEADER_KIND_OFFSET).unwrap(),
);
builder.ins().store(
ir::MemFlags::trusted(),
GC_MEMFLAGS,
ty,
ptr_to_object,
i32::try_from(wasmtime_environ::VM_GC_HEADER_TYPE_INDEX_OFFSET).unwrap(),
);
builder
.ins()
.store(ir::MemFlags::trusted(), end_of_object, ptr_to_next, 0);
.store(GC_MEMFLAGS, end_of_object, ptr_to_next, 0);

log::trace!("emit_inline_alloc(..) -> ({aligned}, {ptr_to_object})");
(aligned, ptr_to_object)
Expand Down Expand Up @@ -223,9 +223,7 @@ impl GcCompiler for NullCompiler {
// any pointers or offsets out from the (untrusted) GC heap.
let len_addr = builder.ins().iadd_imm(ptr_to_object, i64::from(len_offset));
let len = init.len(&mut builder.cursor());
builder
.ins()
.store(ir::MemFlags::trusted(), len, len_addr, 0);
builder.ins().store(GC_MEMFLAGS, len, len_addr, 0);

// Finally, initialize the elements.
let len_to_elems_delta = builder.ins().iconst(ptr_ty, i64::from(len_to_elems_delta));
Expand Down
Loading
Loading