cfallin opened PR #13184 from cfallin:cranelift-x64-reg-width-types to bytecodealliance:main:
This change addresses the kind of lowering mismatch that we saw in CVE-2026-24116 ([advisory]): a case where an instruction lowering reasons about a narrower value (e.g. a scalar
f64) in a wider register (e.g. a 128-bit XMM), and implicit conversions cause load-sinking to happen in a way that leads to a larger-than-expected load from memory.Background: in Cranelift in general, we have an "upper bits undefined" invariant. That is, a 32/64-bit FP value can live in a 128-bit XMM register, or 8/16/32-bit integer can live in a 64-bit GPR, with no problem. The instruction lowering sequences generally deal fine with this: e.g., an
addinstruction doesn't care what is in the upper bits (garbage in, garbage out, and carry only flows from lower to upper, not the other way).However, load-sinking, designed to be as "automatic" as possible to apply in all of the places where it is applicable, throws a wrench into this: because we don't make a distinction about how large the actual operand is, and many layers of the ISLE deal only in register-related operand types (
RegMem,GprMemorXmmMem), we can get an implicit load of the entire register width when we had started with a narrowly-typed value. This is a miscompile that can become serious if paired with a user of Cranelift (e.g. Wasmtime) that relies on certain characteristics of out-of-bound accesses to maintain a sandbox.In this PR, we take the approach discussed in #12477: we add newtypes for all specific bit-widths of operand, to make the distinction solely at metacopmilation time (i.e., when the ISLE DSL itself is typechecked) in the lowering patterns. The expected widths flow upward from the auto-generated instruction constructors in the assembler layer. We use specific types almost universally; in a few cases where helpers do a dynamic dispatch on type, we have specific generic-to-specific-width converters that also take the
tyand assert that it has the correct width. Then, finally and most importantly, the auto-conversions for load sinking will only work (and will release-assert otherwise) if the type-width of the correspondingValuematches.As a result of doing this large refactor, three separate bugs in sunk-load width were found; see the new filetests. They are:
bitselecton scalar f32/f64 args could sink a load, and would use a SIMD instruction that could fold a load and do a 128-bit load. (Wasm lowering only usesbitselectwith 128-bit args.)
swiden_lowanduwiden_lowcould fold a 128-bit load and do a 64-bit load (i.e., could narrow the memory op) because the SSE4.1 instruction takes a 64-bit source operand. (Wasm lowering inserts bitcasts that prevent load-sinking in this case.)
fcvt_from_sintcould likewise turn a 128-bit load into a 64-bit load. (Wasm lowering likewise foils the bug with bitcasts.)I verified that none of them are reachable from Wasm, fortunately, so we don't have a repeat of the above CVE.
[advisory]:
https://github.com/bytecodealliance/wasmtime/security/advisories/GHSA-vc8c-j3xm-xj73<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
cfallin requested alexcrichton for a review on PR #13184.
cfallin requested wasmtime-compiler-reviewers for a review on PR #13184.
cfallin updated PR #13184.
cfallin edited PR #13184:
This change addresses the kind of lowering mismatch that we saw in CVE-2026-24116 ([advisory]): a case where an instruction lowering reasons about a narrower value (e.g. a scalar
f64) in a wider register (e.g. a 128-bit XMM), and implicit conversions cause load-sinking to happen in a way that leads to a larger-than-expected load from memory.Background: in Cranelift in general, we have an "upper bits undefined" invariant. That is, a 32/64-bit FP value can live in a 128-bit XMM register, or 8/16/32-bit integer can live in a 64-bit GPR, with no problem. The instruction lowering sequences generally deal fine with this: e.g., an
addinstruction doesn't care what is in the upper bits (garbage in, garbage out, and carry only flows from lower to upper, not the other way).However, load-sinking, designed to be as "automatic" as possible to apply in all of the places where it is applicable, throws a wrench into this: because we don't make a distinction about how large the actual operand is, and many layers of the ISLE deal only in register-related operand types (
RegMem,GprMemorXmmMem), we can get an implicit load of the entire register width when we had started with a narrowly-typed value. This is a miscompile that can become serious if paired with a user of Cranelift (e.g. Wasmtime) that relies on certain characteristics of out-of-bound accesses to maintain a sandbox.In this PR, we take the approach discussed in #12477: we add newtypes for all specific bit-widths of operand, to make the distinction solely at metacopmilation time (i.e., when the ISLE DSL itself is typechecked) in the lowering patterns. The expected widths flow upward from the auto-generated instruction constructors in the assembler layer. We use specific types almost universally; in a few cases where helpers do a dynamic dispatch on type, we have specific generic-to-specific-width converters that also take the
tyand assert that it has the correct width. Then, finally and most importantly, the auto-conversions for load sinking will only work (and will release-assert otherwise) if the type-width of the correspondingValuematches.As a result of doing this large refactor, three separate bugs in sunk-load width were found; see the new filetests. They are:
bitselecton scalar f32/f64 args could sink a load, and would use a SIMD instruction that could fold a load and do a 128-bit load. (Wasm lowering only usesbitselectwith 128-bit args.)
swiden_lowanduwiden_lowcould fold a 128-bit load and do a 64-bit load (i.e., could narrow the memory op) because the SSE4.1 instruction takes a 64-bit source operand. (Wasm lowering inserts bitcasts that prevent load-sinking in this case.)
fcvt_from_sintcould likewise turn a 128-bit load into a 64-bit load. (Wasm lowering likewise foils the bug with bitcasts.)I verified that none of them are reachable from Wasm, fortunately, so we don't have a repeat of the above CVE.
Fixes #12477.
[advisory]:
https://github.com/bytecodealliance/wasmtime/security/advisories/GHSA-vc8c-j3xm-xj73<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
github-actions[bot] added the label cranelift on PR #13184.
github-actions[bot] added the label cranelift:area:x64 on PR #13184.
github-actions[bot] added the label cranelift:meta on PR #13184.
github-actions[bot] added the label isle on PR #13184.
github-actions[bot] commented on PR #13184:
Subscribe to Label Action
cc @cfallin, @fitzgen
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:x64", "cranelift:meta", "isle"Thus the following users have been cc'd because of the following labels:
- cfallin: isle
- fitzgen: isle
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
fitzgen submitted PR review:
Did not read every line of isle that changed, but everything I sampled looked good. Focused more on the newtype definitions, which also look good.
fitzgen added PR #13184 x64: Add bitwidth-specific newtypes for registers. to the merge queue
github-merge-queue[bot] removed PR #13184 x64: Add bitwidth-specific newtypes for registers. from the merge queue
cfallin commented on PR #13184:
Merge queue bounced with network error in
onnx-download; retrying...
cfallin added PR #13184 x64: Add bitwidth-specific newtypes for registers. to the merge queue
cfallin merged PR #13184.
cfallin removed PR #13184 x64: Add bitwidth-specific newtypes for registers. from the merge queue
Last updated: May 03 2026 at 22:13 UTC