Patch for CVE-2021-32629 to stable-v0.26 for security point release.#2919
Merged
Conversation
Fixes bytecodealliance#2839. See also the issue description and comments in this commits for details of what the fix is about here.
Previously, the x64 backend's ABI code would generate a sign-extending load when loading a less-than-64-bit integer from a spillslot. This is incorrect: e.g., for i32s > 0x80000000, this would result in all high bits set. This interacts poorly with another optimization. Normally, the invariant is that the high bits of a register holding a value of a certain type, beyond that type's bits, are undefined. However, as an optimization, we recognize and use the fact that on x86-64, 32-bit instructions zero the upper 32 bits. This allows us to elide a 32-to-64-bit zero-extend op (turning it into just a move, which can then sometimes disappear entirely due to register coalescing). If a spill and reload happen between the production of a 32-bit value from an instruction known to zero the upper bits and its use, then we will rely on zero upper bits that might actually be set by a sign-extend. This will result in incorrect execution. As a fix, we stick to a simple invariant: we always spill and reload a full 64 bits when handling integer registers on x64. This ensures that no bits are mangled.
alexcrichton
approved these changes
May 21, 2021
Member
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
If you want, wanna fold the version bumps into this as well to double-check?
iximeow
approved these changes
May 21, 2021
Contributor
iximeow
left a comment
iximeow
left a comment
There was a problem hiding this comment.
thank you for shepherding these fixes along!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a cherrypick of the fix for the CVE (and related #2840) on top of the v0.26.0 release, using a new
stable-v0.26branch. Once this merges, I will version-bump and release v0.26.1 (and Cranelift v0.73.1) off of this branch.