Stream: git-wasmtime

Topic: wasmtime / PR #4092 Implement Spectre mitigations for tab...


view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2022 at 00:10):

cfallin opened PR #4092 from x64-spectre-table-bounds to main:

Currently, we have partial Spectre mitigation: we protect heap accesses
with dynamic bounds checks. Specifically, we guard against errant
accesses on the misspeculated path beyond the bounds-check conditional
branch by adding a conditional move that is also dependent on the
bounds-check condition. This data dependency on the condition is not
speculated and thus will always pick the "safe" value (in the heap case,
a NULL address) on the misspeculated path, until the pipeline flushes
and recovers onto the correct path.

This PR uses the same technique both for table accesses -- used to
implement Wasm tables -- and for jumptables, used to implement Wasm
br_table instructions.

In the case of Wasm tables, the cmove picks the table base address on
the misspeculated path. This is equivalent to reading the first table
entry. This prevents loads of arbitrary data addresses on the
misspeculated path.

In the case of br_table, the cmove picks index 0 on the misspeculated
path. This is safer than allowing a branch to an address loaded from an
index under misspeculation (i.e., it preserves control-flow integrity
even under misspeculation).

The table mitigation is controlled by a Cranelift setting, on by
default. The br_table mitigation is always on, because it is part of the
single lowering pseudoinstruction. In both cases, the impact should be
minimal: a single extra cmove in a (relatively) rarely-used operation.

The table mitigation is architecture-independent (happens during
legalization); the br_table mitigation has been implemented for both x64
and aarch64. (I don't know enough about s390x to implement this
confidently there, but would happily review a PR to do the same on that
platform.)

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2022 at 00:10):

cfallin requested abrown for a review on PR #4092.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2022 at 00:10):

cfallin requested akirilov-arm for a review on PR #4092.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2022 at 00:10):

cfallin requested fitzgen for a review on PR #4092.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2022 at 01:28):

cfallin updated PR #4092 from x64-spectre-table-bounds to main.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2022 at 06:34):

bjorn3 created PR review comment:

This removes new_inst and not inst, right?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2022 at 06:34):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2022 at 21:50):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2022 at 21:50):

cfallin created PR review comment:

No, it removes inst; you can verify by looking at the added tests in this PR that the conditional move (selectif_spectre_guard) is present. The reason is that replace_with_aliases replaces uses of inst's results but the cursor still remains at inst.

view this post on Zulip Wasmtime GitHub notifications bot (May 02 2022 at 17:35):

fitzgen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 02 2022 at 18:19):

cfallin merged PR #4092.


Last updated: Dec 23 2024 at 12:05 UTC