Repository: bytecodealliance/wasmtime
Pull request #8338 (2024-04-11 18:40:18)

[18.0.0] Handle out-of-bounds component sections 
Backport of #8323


Patch:
diff --git a/cranelift/codegen/src/isa/aarch64/abi.rs b/cranelift/codegen/src/isa/aarch64/abi.rs
index d1279921d..c7f4c8450 100644
--- a/cranelift/codegen/src/isa/aarch64/abi.rs
+++ b/cranelift/codegen/src/isa/aarch64/abi.rs
@@ -1164,7 +1164,6 @@ impl ABIMachineSpec for AArch64MachineDeps {
         };
 
         // Return FrameLayout structure.
-        debug_assert!(outgoing_args_size == 0);
         FrameLayout {
             stack_args_size,
             setup_area_size,
diff --git a/cranelift/codegen/src/isa/riscv64/abi.rs b/cranelift/codegen/src/isa/riscv64/abi.rs
index c4e16fcd4..3d50a443a 100644
--- a/cranelift/codegen/src/isa/riscv64/abi.rs
+++ b/cranelift/codegen/src/isa/riscv64/abi.rs
@@ -696,7 +696,6 @@ impl ABIMachineSpec for Riscv64MachineDeps {
         };
 
         // Return FrameLayout structure.
-        debug_assert!(outgoing_args_size == 0);
         FrameLayout {
             stack_args_size,
             setup_area_size,
diff --git a/cranelift/codegen/src/isa/x64/abi.rs b/cranelift/codegen/src/isa/x64/abi.rs
index d7df15c61..1d9f72a83 100644
--- a/cranelift/codegen/src/isa/x64/abi.rs
+++ b/cranelift/codegen/src/isa/x64/abi.rs
@@ -858,7 +858,6 @@ impl ABIMachineSpec for X64ABIMachineSpec {
         let setup_area_size = 16; // RBP, return address
 
         // Return FrameLayout structure.
-        debug_assert!(outgoing_args_size == 0);
         FrameLayout {
             stack_args_size,
             setup_area_size,
diff --git a/cranelift/codegen/src/machinst/abi.rs b/cranelift/codegen/src/machinst/abi.rs
index afeedec7d..5a9259a0e 100644
--- a/cranelift/codegen/src/machinst/abi.rs
+++ b/cranelift/codegen/src/machinst/abi.rs
@@ -1883,9 +1883,25 @@ impl<M: ABIMachineSpec> Callee<M> {
             &frame_layout,
         ));
 
+        // The stack limit check needs to cover all the stack adjustments we
+        // might make, up to the next stack limit check in any function we
+        // call. Since this happens after frame setup, the current function's
+        // setup area needs to be accounted for in the caller's stack limit
+        // check, but we need to account for any setup area that our callees
+        // might need. Note that s390x may also use the outgoing args area for
+        // backtrace support even in leaf functions, so that should be accounted
+        // for unconditionally.
+        let total_stacksize = frame_layout.clobber_size
+            + frame_layout.fixed_frame_storage_size
+            + frame_layout.outgoing_args_size
+            + if self.is_leaf {
+                0
+            } else {
+                frame_layout.setup_area_size
+            };
+
         // Leaf functions with zero stack don't need a stack check if one's
         // specified, otherwise always insert the stack check.
-        let total_stacksize = frame_layout.fixed_frame_storage_size;
         if total_stacksize > 0 || !self.is_leaf {
             if let Some((reg, stack_limit_load)) = &self.stack_limit {
                 insts.extend(stack_limit_load.clone());
diff --git a/cranelift/codegen/src/machinst/isle.rs b/cranelift/codegen/src/machinst/isle.rs
index c1f543b9e..9ec201b8e 100644
--- a/cranelift/codegen/src/machinst/isle.rs
+++ b/cranelift/codegen/src/machinst/isle.rs
@@ -838,6 +838,12 @@ macro_rules! isle_prelude_method_helpers {
             mut caller: $abicaller,
             args: ValueSlice,
         ) -> InstOutput {
+            let off = self.lower_ctx.sigs()[abi].sized_stack_arg_space()
+                + self.lower_ctx.sigs()[abi].sized_stack_ret_space();
+            self.lower_ctx
+                .abi_mut()
+                .accumulate_outgoing_args_size(off as u32);
+
             caller.emit_stack_pre_adjust(self.lower_ctx);
 
             self.gen_call_common_args(&mut caller, args);
diff --git a/crates/environ/src/component/translate.rs b/crates/environ/src/component/translate.rs
index 3334b1736..873105b34 100644
--- a/crates/environ/src/component/translate.rs
+++ b/crates/environ/src/component/translate.rs
@@ -4,6 +4,7 @@ use crate::{
     EntityIndex, ModuleEnvironment, ModuleTranslation, ModuleTypesBuilder, PrimaryMap, Tunables,
     TypeConvert, WasmHeapType, WasmValType,
 };
+use anyhow::anyhow;
 use anyhow::{bail, Result};
 use indexmap::IndexMap;
 use std::collections::HashMap;
@@ -534,7 +535,18 @@ impl<'a, 'data> Translator<'a, 'data> {
                     self.validator,
                     self.types.module_types_builder(),
                 )
-                .translate(parser, &component[range.start..range.end])?;
+                .translate(
+                    parser,
+                    component.get(range.start..range.end).ok_or_else(|| {
+                        anyhow!(
+                            "section range {}..{} is out of bounds (bound = {})",
+                            range.start,
+                            range.end,
+                            component.len()
+                        )
+                        .context("wasm component contains an invalid module section")
+                    })?,
+                )?;
                 let static_idx = self.static_modules.push(translation);
                 self.result
                     .initializers


Test patch:
diff --git a/cranelift/filetests/filetests/isa/aarch64/stack-limit.clif b/cranelift/filetests/filetests/isa/aarch64/stack-limit.clif
index 6bd7520ba..e30a32152 100644
--- a/cranelift/filetests/filetests/isa/aarch64/stack-limit.clif
+++ b/cranelift/filetests/filetests/isa/aarch64/stack-limit.clif
@@ -55,7 +55,8 @@ block0(v0: i64):
 ; VCode:
 ;   stp fp, lr, [sp, #-16]!
 ;   mov fp, sp
-;   subs xzr, sp, x0, UXTX
+;   add x16, x0, #16
+;   subs xzr, sp, x16, UXTX
 ;   b.lo #trap=stk_ovf
 ; block0:
 ;   load_ext_name x2, TestCase(%foo)+0
@@ -67,11 +68,12 @@ block0(v0: i64):
 ; block0: ; offset 0x0
 ;   stp x29, x30, [sp, #-0x10]!
 ;   mov x29, sp
-;   cmp sp, x0
-;   b.lo #0x2c
-; block1: ; offset 0x10
-;   ldr x2, #0x18
-;   b #0x20
+;   add x16, x0, #0x10
+;   cmp sp, x16
+;   b.lo #0x30
+; block1: ; offset 0x14
+;   ldr x2, #0x1c
+;   b #0x24
 ;   .byte 0x00, 0x00, 0x00, 0x00 ; reloc_external Abs8 %foo 0
 ;   .byte 0x00, 0x00, 0x00, 0x00
 ;   blr x2
@@ -95,6 +97,7 @@ block0(v0: i64):
 ;   mov fp, sp
 ;   ldr x16, [x0]
 ;   ldr x16, [x16, #4]
+;   add x16, x16, #16
 ;   subs xzr, sp, x16, UXTX
 ;   b.lo #trap=stk_ovf
 ; block0:
@@ -109,11 +112,12 @@ block0(v0: i64):
 ;   mov x29, sp
 ;   ldur x16, [x0]
 ;   ldur x16, [x16, #4]
+;   add x16, x16, #0x10
 ;   cmp sp, x16
-;   b.lo #0x34
-; block1: ; offset 0x18
-;   ldr x2, #0x20
-;   b #0x28
+;   b.lo #0x38
+; block1: ; offset 0x1c
+;   ldr x2, #0x24
+;   b #0x2c
 ;   .byte 0x00, 0x00, 0x00, 0x00 ; reloc_external Abs8 %foo 0
 ;   .byte 0x00, 0x00, 0x00, 0x00
 ;   blr x2
diff --git a/cranelift/filetests/filetests/isa/riscv64/stack-limit.clif b/cranelift/filetests/filetests/isa/riscv64/stack-limit.clif
index be0c64af3..6dd424c2a 100644
--- a/cranelift/filetests/filetests/isa/riscv64/stack-limit.clif
+++ b/cranelift/filetests/filetests/isa/riscv64/stack-limit.clif
@@ -58,7 +58,8 @@ block0(v0: i64):
 ;   sd ra,8(sp)
 ;   sd fp,0(sp)
 ;   mv fp,sp
-;   trap_if stk_ovf##(sp ult a0)
+;   addi t6,a0,16
+;   trap_if stk_ovf##(sp ult t6)
 ; block0:
 ;   load_sym a2,%foo+0
 ;   callind a2
@@ -73,9 +74,10 @@ block0(v0: i64):
 ;   sd ra, 8(sp)
 ;   sd s0, 0(sp)
 ;   mv s0, sp
-;   bgeu sp, a0, 8
+;   addi t6, a0, 0x10
+;   bgeu sp, t6, 8
 ;   .byte 0x00, 0x00, 0x00, 0x00 ; trap: stk_ovf
-; block1: ; offset 0x18
+; block1: ; offset 0x1c
 ;   auipc a2, 0
 ;   ld a2, 0xc(a2)
 ;   j 0xc
@@ -105,6 +107,7 @@ block0(v0: i64):
 ;   mv fp,sp
 ;   ld t6,0(a0)
 ;   ld t6,4(t6)
+;   addi t6,t6,16
 ;   trap_if stk_ovf##(sp ult t6)
 ; block0:
 ;   load_sym a2,%foo+0
@@ -122,9 +125,10 @@ block0(v0: i64):
 ;   mv s0, sp
 ;   ld t6, 0(a0)
 ;   ld t6, 4(t6)
+;   addi t6, t6, 0x10
 ;   bgeu sp, t6, 8
 ;   .byte 0x00, 0x00, 0x00, 0x00 ; trap: stk_ovf
-; block1: ; offset 0x20
+; block1: ; offset 0x24
 ;   auipc a2, 0
 ;   ld a2, 0xc(a2)
 ;   j 0xc
diff --git a/cranelift/filetests/filetests/isa/s390x/stack-limit.clif b/cranelift/filetests/filetests/isa/s390x/stack-limit.clif
index 0a2387b2f..9f69853de 100644
--- a/cranelift/filetests/filetests/isa/s390x/stack-limit.clif
+++ b/cranelift/filetests/filetests/isa/s390x/stack-limit.clif
@@ -52,7 +52,8 @@ block0(v0: i64):
 }
 
 ; VCode:
-;   clgrtle %r15, %r2
+;   la %r1, 160(%r2)
+;   clgrtle %r15, %r1
 ;   stmg %r14, %r15, 112(%r15)
 ;   aghi %r15, -160
 ;   virtual_sp_offset_adjust 160
@@ -64,11 +65,12 @@ block0(v0: i64):
 ;
 ; Disassembled:
 ; block0: ; offset 0x0
-;   clgrtle %r15, %r2 ; trap: stk_ovf
+;   la %r1, 0xa0(%r2)
+;   clgrtle %r15, %r1 ; trap: stk_ovf
 ;   stmg %r14, %r15, 0x70(%r15)
 ;   aghi %r15, -0xa0
-; block1: ; offset 0xe
-;   bras %r1, 0x1a
+; block1: ; offset 0x12
+;   bras %r1, 0x1e
 ;   .byte 0x00, 0x00 ; reloc_external Abs8 %foo 0
 ;   .byte 0x00, 0x00
 ;   .byte 0x00, 0x00
@@ -92,6 +94,7 @@ block0(v0: i64):
 ; VCode:
 ;   lg %r1, 0(%r2)
 ;   lg %r1, 4(%r1)
+;   la %r1, 160(%r1)
 ;   clgrtle %r15, %r1
 ;   stmg %r14, %r15, 112(%r15)
 ;   aghi %r15, -160
@@ -106,11 +109,12 @@ block0(v0: i64):
 ; block0: ; offset 0x0
 ;   lg %r1, 0(%r2)
 ;   lg %r1, 4(%r1)
+;   la %r1, 0xa0(%r1)
 ;   clgrtle %r15, %r1 ; trap: stk_ovf
 ;   stmg %r14, %r15, 0x70(%r15)
 ;   aghi %r15, -0xa0
-; block1: ; offset 0x1a
-;   bras %r1, 0x26
+; block1: ; offset 0x1e
+;   bras %r1, 0x2a
 ;   .byte 0x00, 0x00 ; reloc_external Abs8 %foo 0
 ;   .byte 0x00, 0x00
 ;   .byte 0x00, 0x00
diff --git a/tests/all/component_model/aot.rs b/tests/all/component_model/aot.rs
index 5fc5c8890..8b38ae151 100644
--- a/tests/all/component_model/aot.rs
+++ b/tests/all/component_model/aot.rs
@@ -133,3 +133,46 @@ fn detect_precompiled() -> Result<()> {
     );
     Ok(())
 }
+
+#[test]
+#[cfg_attr(miri, ignore)]
+fn truncated_component_binaries_dont_panic() -> Result<()> {
+    let engine = super::engine();
+
+    let binary = wat::parse_str(
+        r#"
+        (component
+            (import "a" (core module $m0
+                (import "" "" (func))
+            ))
+
+            (core module $m1
+                (func (export ""))
+            )
+            (core instance $i1 (instantiate (module $m1)))
+            (func $f (canon lift (core func $i1 "f")))
+
+            (component $c1
+                (import "f" (func))
+                (core module $m2
+                    (func (export "g"))
+                )
+                (core instance $i2 (instantiate $m2))
+                (func (export "g")
+                    (canon lift (core func $i2 "g"))
+                )
+            )
+            (instance $i3 (instantiate $c1 (with "f" (func $f))))
+            (func (export "g") (alias export $i3 "g"))
+        )
+        "#,
+    )?;
+
+    // Check that if we feed each truncation of the component binary into
+    // `Component::new` we don't get any panics.
+    for i in 1..binary.len() - 1 {
+        let _ = Component::from_binary(&engine, &binary[0..i]);
+    }
+
+    Ok(())
+}