Stream: git-wasmtime

Topic: wasmtime / issue #9402 Move constant pools in compiled co...

Wasmtime GitHub notifications bot (Oct 08 2024 at 21:23):

alexcrichton opened issue #9402:

Today Cranelift's constant-pools are located in the .text section of the executable, typically located after the function itself. While convenient for code generation this exposes a possible attack vector in Wasmtime where it's trivial to put a "gadget" somewhere in memory. For example using a sequence of v128.const it would be pretty easy to assemble "machine code" at the end of a function. In the face of a bug in Cranelift this could make it possibly easier to amplify into a sandbox escape perhaps.

As a defense-in-depth measure we should try to move the constant pools out of the .text section and into a .data or otherwise read-only section. (not writable or executable). This won't be trivial to do due to the fact that relocations from the text section point at the data section and the relocation range may not always be large enough for the entire text section. Regardless though I wanted to file an issue about this idea.

Wasmtime GitHub notifications bot (Oct 08 2024 at 21:30):

cfallin commented on issue #9402:

It would definitely be nice to have support for this -- in principle we could return two blobs of bytes as the result of per-function compilation instead of one, and have a relocation type that is "offset from start of this function's code to start of this function's constants".

Out of curiosity, do you happen to know how ld handles .rodata references today for very large aarch64/riscv64/... binaries? I wonder if it uses its support for relaxation (assuming most pessimistic range sequence then shrinking if able) -- it'd be unfortunate to have to use adrp/adr/ldr rather than the immediate-pcrel form of ldr for every constant. I'm not able to find anything on this at the moment...

Wasmtime GitHub notifications bot (Oct 08 2024 at 22:52):

alexcrichton commented on issue #9402:

That's an excellent question, and one I don't know the answer to myself. I can try to play around with an assembler though and see what happens perhaps!

Wasmtime GitHub notifications bot (Oct 08 2024 at 23:13):

cfallin commented on issue #9402:

I tried briefly to trigger something interesting, but got stuck at trying to get clang (on macOS/aarch64) to use the short-form LDR-with-immediate instruction; for any load from rodata it seems to use an adrp/adr pair.

For example with (separate files to avoid a neat optimization where clang const-folds the load of constant data):

% cat test.c
extern const char* s;

int foo() {
    return *((int*)s);
}

% cat data.c
const char* s = "1234";

% cat main.c
#include <stdio.h>
extern int foo();
int main() {
    printf("%d\n", foo());
}

I see _foo's body as

0000000100003f44 <_foo>:
100003f44: b0000028     adrp    x8, 0x100008000 <_s>
100003f48: 91000108     add x8, x8, #0x0
100003f4c: f9400108     ldr x8, [x8]
100003f50: b9400100     ldr w0, [x8]
100003f54: d65f03c0     ret

Maybe it wouldn't be so bad to unconditionally emit that form actually; loads from constant pools will be relatively rare. It does burn a register though to compute the address.

Wasmtime GitHub notifications bot (Oct 09 2024 at 00:00):

alexcrichton commented on issue #9402:

Good point!

Looks like

#[no_mangle]
pub extern "C" fn foo() ->f64 {
    1.3484
}

generates

.LCPI0_0:
        .xword  0x3ff5930be0ded289
foo:
        adrp    x8, .LCPI0_0
        ldr     d0, [x8, :lo12:.LCPI0_0]
        ret

so yeah it looks like we may want to be a tiny bit clever (don't always "just" materialize the address) but otherwise looks like solving this issue would involve always doing adrp on aarch64 and the equivalent on riscv64

Wasmtime GitHub notifications bot (Oct 10 2024 at 22:54):

alexcrichton added the cranelift:area:security label to Issue #9402.

Wasmtime GitHub notifications bot (Oct 08 2026 at 21:06):

ilyas-mallah commented on issue #9402:

I'd like to work on this. What I have in mind follows @cfallin's idea of returning the constants as a second blob. MachBuffer would get an option to hand back a function's constants separately instead of emitting them after the code, with every constant use becoming a relocation into that blob. Wasmtime then places the blobs in a read-only section after .text, page-aligned so no constant shares an executable page with code.

x64, riscv64 and s390x already reach far enough. AArch64 is the hard part, since ldr literal would have to become an adrp-based load, which costs an instruction and, for float and vector constants, a GPR. I'd benchmark the options with Sightglass before picking one.

With every function's constants in one section, identical constants could also be coalesced across functions (the part @fitzgen moved here from #1385), and constants would no longer ride along in islands, which was part of what made #12968 tricky.

I'd land it as a few small PRs, starting with the buffer support, then a PR each for the x64, AArch64, riscv64 and s390x backends, and finally turning it on in Wasmtime.

A few things I'm unsure about. Should this be a Cranelift setting or something Wasmtime always enables? Would you prefer a separate section or a page-aligned tail of .text? And would you want an RFC first?

Wasmtime GitHub notifications bot (Oct 08 2026 at 22:20):

alexcrichton commented on issue #9402:

It'd be awesome if you're willing to pick this up @ilyas-mallah! As a heads up we as maintainers have a lot of issues in flight right now so we may be a bit slower to respond here or on PRs, but it'd be great to resolve this still. What you describe all sounds reasonable to me, and IIRC some time ago we were thinking of changing all data relocations to adrp+add or similar for aarch64, although maybe that was in relation to this issue as well... A sightglass run would be definitely helpful to evaluate!

As for landing, piecemeal is totally fine. I wouldn't necessarily go out of your way for a Cranelift setting for this, but if it's natural to implement then it seems reasonable to have and would pave the path for testing this easily without committing to it just yet. No need for an RFC on this one, good to just work on and have PRs!

One thing of note -- Wasmtime at least already has some page-aligned stuff after the .text section, for example wasm data segments and such. That should be relatively easy to piggy-back on. Wasmtime also puts the .eh_frame section (IIRC) in a page-aligned section after .text so it's not executable as well, which could perhaps show inspiration for how to do this since that also has to do with relocations across the sections.

Wasmtime GitHub notifications bot (Oct 08 2026 at 23:23):

ilyas-mallah commented on issue #9402:

Thanks, that's really helpful, and no worries about review speed. Good pointers on the data segments and .eh_frame, I'll look at how they handle relocations across sections first, and I'll include Sightglass numbers with the PRs. I'm planning to start early next year.

Wasmtime GitHub notifications bot (Oct 09 2026 at 20:54):

ilyas-mallah commented on issue #9402:

I'd like to split this into PRs this way, following my earlier comment:

1. Cranelift: MachBuffer support for the separate constant blob, off by default.
2. x64: RIP-relative constant references become relocations into the blob.
3. AArch64: adrp-based loads in place of ldr literal, using whichever sequence Sightglass favors.
4. riscv64: auipc plus a load into the blob.
5. s390x: 32-bit PC-relative references into the blob.
6. Winch, since it registers its constants in the same MachBuffer. Pulley bytecode isn't run as machine code, so it stays as it is.
7. Wasmtime: emit the new section following the .eh_frame model and turn it on by default. This covers serialized modules, a test that no executable page holds constants, and an update to docs/security.md.

I'd leave coalescing identical constants across functions for later, it's easier once all of this has landed.

I'd also like to take on #11181, opting wasmtime modules into the unsafe Clippy lints one PR at a time. Each PR adds # Safety docs and SAFETY comments, with a safe wrapper where a contract can be checked locally. The modules in order are vm/cow.rs with pooling/decommit_queue.rs, vm/mmap.rs with vm/sys/unix/mmap.rs, vm/libcalls.rs, vm/component/libcalls.rs, vm/table.rs, vm/traphandlers.rs with its backtrace.rs, and vm/sys/unix/signals.rs.

@alexcrichton you mentioned moving all AArch64 data relocations to adrp+add. Should the AArch64 PR handle LoadExtNameFar too (it puts an 8-byte address inline in .text), or just the constant pools? And would you rather the last PR turn this on by default, or add a Config option that stays off for now?

Last updated: Oct 11 2026 at 04:10 UTC