Skip to content

chacha20: fix use of SSE4.1 intrinsic in SSE2 backend - #580

Merged
newpavlov merged 4 commits into
masterfrom
chacha20/fix_sse2
Aug 27, 2026
Merged

chacha20: fix use of SSE4.1 intrinsic in SSE2 backend#580
newpavlov merged 4 commits into
masterfrom
chacha20/fix_sse2

Conversation

@newpavlov

@newpavlov newpavlov commented Aug 27, 2026

Copy link
Copy Markdown
Member

Closes #579

@newpavlov
newpavlov requested a review from tarcieri August 27, 2026 15:25
Comment thread chacha20/src/backends/sse2.rs Outdated
if size_of::<V::Counter>() == 8 {
state[13] = _mm_extract_epi32(backend.v[3], 1) as u32;
}
_mm_storeu_si128(state_ptr.add(3), backend.v[3]);

@newpavlov newpavlov Aug 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unnecessarily writes 64-96 bits which stay unmodified, but it results in a simpler code (especially considering that _mm_cvtsi128_si64 is not available on i686 targets) and it even may be miniscully more efficient since we directly write XMM register data.

@newpavlov

newpavlov commented Aug 27, 2026

Copy link
Copy Markdown
Member Author

@tarcieri
It seems that we don't have sufficient testing for the legacy cipher variants and RNG. Writing only 32 counter bits for them does not get caught by current tests, so it's probably worth to fix in a future PR.

@newpavlov
newpavlov merged commit 075ccbd into master Aug 27, 2026
32 checks passed
@newpavlov
newpavlov deleted the chacha20/fix_sse2 branch August 27, 2026 17:41
newpavlov added a commit that referenced this pull request Aug 27, 2026
### Fixed
- Use of SSE4.1 intrinsic in SSE2 backend of RNG and legacy (64-bit
counter) variants ([#580])

[#580]: #580
@newpavlov

Copy link
Copy Markdown
Member Author

@tarcieri
I think it's probably worth to yank v0.10.0 and v0.10.1 as releases containing UB. WDYT?

@tarcieri

Copy link
Copy Markdown
Member

sure

newpavlov added a commit that referenced this pull request Aug 27, 2026
The yanked versions contain UB in SSE2 backend, see #580.
@elichai

elichai commented Aug 27, 2026

Copy link
Copy Markdown

Glad to see that RustCrypto/utils#1513 helped catch UB :)

@tarcieri

Copy link
Copy Markdown
Member

That much is definitely cool!

MarcusKainth added a commit to spate-etl/spate that referenced this pull request Aug 27, 2026
`chacha20` 0.10.0 and 0.10.1 were yanked from crates.io. The SSE2 backend of
the RNG and legacy 64-bit-counter variants used `_mm_extract_epi32`, an SSE4.1
intrinsic, so a CPU with SSE2 and without SSE4.1 met an illegal instruction
instead of a working fallback. 0.10.2 replaces it with `_mm_storeu_si128`,
which is SSE2 (RustCrypto/stream-ciphers#580).

Nothing about ciphertext, keys or nonces changes. The fault is portability, and
the affected path is reachable only on x86_64 hardware predating SSE4.1.

`deny.toml` sets `yanked = "deny"`, so the yank fails `make deny` on every
branch until the lockfile moves. The crate reaches the shipped tree through
`apache-avro` and `async-nats`, both by way of `rand` 0.10.2.

THIRD-PARTY.md carries the version, so it is regenerated here.

Changelog: none
IceTDrinker added a commit to zama-ai/tfhe-rs that referenced this pull request Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

miri fails on chacha20 with incorrectly gated Intel intrinsic

3 participants