Wasmtime GitHub notifications bot (Jan 04 2022 at 18:54): Wasmtime GitHub notifications bot (Jan 04 2022 at 18:54):cfallin opened PR #3645 from fix-xmm-spillslot-fuzzbug to main:
Fixes a recently-received fuzzbug exposed by differential fuzzing against V8.
This patch makes spillslot allocation, spilling and reloading all based
on register class only. Hence when we have a 32- or 64-bit value in a
128-bit XMM register on x86-64 or vector register on aarch64, this
results in larger spillslots and spills/restores.Why make this change, if it results in less efficient stack-frame usage?
Simply put, it is safer: there is always a risk when allocating
spillslots or spilling/reloading that we get the wrong type and make the
spillslot or the store/load too small. This was one contributing factor
to CVE-2021-32629, and is now the source of a fuzzbug in SIMD code that
puns an arbitrary user-controlled vector constant over another
stackslot. (If this were a pointer, that could result in RCE. SIMD is
not yet on by default in a release, fortunately.)In particular, we have not been particularly careful about using moves
between values of different types, for example withraw_bitcastor
with certain SIMD operations, and such moves indicate to regalloc.rs
that vregs are in equivalence classes and some arbitrary vreg in the
class is provided when allocating the spillslot or spilling/reloading.
Since regalloc.rs does not track actual type, and since we haven't been
careful about moves, we can't really trust this "arbitrary vreg in
equivalence class" to provide accurate type information.In the fix to CVE-2021-32629 we fixed this for integer registers by
always spilling/reloading 64 bits; this fix can be seen as the analogous
change for FP/vector regs.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
Wasmtime GitHub notifications bot (Jan 04 2022 at 18:54):cfallin requested abrown for a review on PR #3645.
Wasmtime GitHub notifications bot (Jan 04 2022 at 18:54):cfallin requested alexcrichton for a review on PR #3645.
Wasmtime GitHub notifications bot (Jan 04 2022 at 19:08):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jan 04 2022 at 19:08):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jan 04 2022 at 19:08):alexcrichton created PR review comment:
I don't think this actually runs anything without
; run(...)comments, right?
Wasmtime GitHub notifications bot (Jan 04 2022 at 19:37):cfallin updated PR #3645 from fix-xmm-spillslot-fuzzbug to main.
Wasmtime GitHub notifications bot (Jan 04 2022 at 19:38):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Jan 04 2022 at 19:38):cfallin created PR review comment:
Ah, yes, good point. I played with this for a bit but ran into other issues (with the vmctx) and concluded it was simpler just to add the Wasm test directly, rather than capture its CLIF.
Wasmtime GitHub notifications bot (Jan 04 2022 at 19:40):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jan 04 2022 at 20:42):cfallin updated PR #3645 from fix-xmm-spillslot-fuzzbug to main.
Wasmtime GitHub notifications bot (Jan 04 2022 at 21:24):cfallin updated PR #3645 from fix-xmm-spillslot-fuzzbug to main.
Wasmtime GitHub notifications bot (Jan 04 2022 at 22:12):cfallin merged PR #3645.
Last updated: Dec 23 2024 at 13:07 UTC