wasmtime / issue #3693 Initial forward-edge CFI implement... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #3693 Initial forward-edge CFI implement...

Wasmtime GitHub notifications bot (May 04 2022 at 13:56):

github-actions[bot] commented on issue #3693:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "fuzzing"

Thus the following users have been cc'd because of the following labels:

fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (May 04 2022 at 17:37):

akirilov-arm commented on issue #3693:

Note that the region crate lacks support for BTI (reported in darfink/region-rs#21), so I have used a work-around - a direct call to libc::mprotect().

Wasmtime GitHub notifications bot (Jun 23 2022 at 09:40):

akirilov-arm commented on issue #3693:

@alexcrichton I added you as reviewer for the Wasmtime bits.

Wasmtime GitHub notifications bot (Aug 16 2022 at 13:25):

akirilov-arm commented on issue #3693:

@alexcrichton I wanted to finish with the pointer authentication PR before coming back to this one:
* If a module is compiled with bti support and loaded into an engine that has bti support disabled, that's fine right? (since everything is a nop)
Yes, that is correct; we just have to make sure that we do not request the PROT_BTI memory protection mode from the kernel, since the system call is going to fail.
* If a module is not compiled with bti support, but is loaded into an engine with bti support, that's bad right? (in that none of the branches are protected but the page permissions say that all branches need protection?)
Yes, assuming that the engine applies PROT_BTI unconditionally to all executable memory pages.

The current implementation supports the full flexibility the BTI extension enables, so we have to decouple the following:

whether the user passed --cranelift-enable use_bti on the Wasmtime command line

whether the environment (i.e. CPU and OS) supports BTI

whether the module that the engine is trying to load has been compiled to use BTI instructions; also, if I am not mistaken, the engine could be trying to load several modules, which might not necessarily have been compiled with the same settings

Wasmtime GitHub notifications bot (Aug 16 2022 at 14:25):

afonso360 commented on issue #3693:

We should add a target aarch64 use_bti to runtests/br_table.clif, we now use the cranelift JIT to run runtests so we can also test the JIT portion of this PR.

We don't yet support calls, but that's something I'm addressing in #4667, and can add some bti runtests there as well.

Wasmtime GitHub notifications bot (Aug 16 2022 at 14:25):

afonso360 edited a comment on issue #3693:

We should add a target aarch64 use_bti to runtests/br_table.clif, we now use the cranelift JIT to run runtests so we can also test the cranelift-jit portion of this PR.

We don't yet support calls, but that's something I'm addressing in #4667, and can add some bti runtests there as well.

Wasmtime GitHub notifications bot (Aug 16 2022 at 14:25):

afonso360 edited a comment on issue #3693:

We should add a target aarch64 use_bti to runtests/br_table.clif, we now use the cranelift JIT to run runtests so we can also test the cranelift-jit portion of this PR.

We don't yet support call's, but that's something I'm addressing in #4667, and can add some bti runtests there as well.

Wasmtime GitHub notifications bot (Aug 16 2022 at 14:29):

afonso360 edited a comment on issue #3693:

We should add a target aarch64 use_bti to runtests/br_table.clif, we now use the cranelift-jit to run runtests so we can also test the cranelift-jit portion of this PR.

We don't yet support call's, but that's something I'm addressing in #4667, and can add some bti runtests there as well.

Wasmtime GitHub notifications bot (Aug 16 2022 at 14:31):

afonso360 edited a comment on issue #3693:

We should add a target aarch64 use_bti to runtests/br_table.clif, we now use the cranelift-jit to run runtests so we can also test the cranelift-jit portion of this PR.

We don't yet support call's, but that's something I'm addressing in #4667, and can add some bti runtests there as well.

Also, should we add bti detection in cranelift-native?

Wasmtime GitHub notifications bot (Aug 16 2022 at 14:32):

afonso360 edited a comment on issue #3693:

We should add a target aarch64 use_bti to runtests/br_table.clif, we now use the cranelift-jit to run runtests so we can also test the cranelift-jit portion of this PR.

We don't yet support call's, but that's something I'm addressing in #4667, and can add some bti runtests there as well.

Wasmtime GitHub notifications bot (Aug 16 2022 at 20:23):

alexcrichton commented on issue #3693:

Ah sorry ok I see why this can't be an Engine-level thing. If that's the case though instead of ferrying a separate bool through the system could this get attached to CompiledModuleInfo somewhere internally?

Wasmtime GitHub notifications bot (Aug 22 2022 at 12:08):

akirilov-arm commented on issue #3693:

@cfallin I think that I have resolved all of @alexcrichton's comments that are related to Wasmtime, so do you have any feedback on the Cranelift side of things?

Last updated: Dec 23 2024 at 12:05 UTC