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 •
edited
Loading

Copy link
Copy Markdown
Member

Closes #579

@newpavlov
newpavlov requested a review from tarcieri August 27, 2026 15:25
newpavlov
newpavlov commented Aug 27, 2026
View reviewed changes
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 •
edited
Loading

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 •
edited
Loading

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 mentioned this pull request Aug 27, 2026
Merged
@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 newpavlov mentioned this pull request Aug 27, 2026
Merged
newpavlov added a commit that referenced this pull request Aug 27, 2026
@newpavlov
Release chacha20 v0.10.2 (#581)
6b236b7 
### Fixed
- Use of SSE4.1 intrinsic in SSE2 backend of RNG and legacy (64-bit
counter) variants ([#580])

[#580]: #580
@newpavlov

newpavlov commented Aug 27, 2026

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

tarcieri commented Aug 27, 2026

Copy link
Copy Markdown
Member

sure

@newpavlov newpavlov mentioned this pull request Aug 27, 2026
Merged
newpavlov added a commit that referenced this pull request Aug 27, 2026
@newpavlov
chacha20: mark v0.10.0 and v0.10.1 as yanked (#583)
dc302ec 
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

tarcieri commented Aug 27, 2026

Copy link
Copy Markdown
Member

That much is definitely cool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tarcieri tarcieri Awaiting requested review from tarcieri

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

miri fails on chacha20 with incorrectly gated Intel intrinsic

3 participants

@newpavlov @tarcieri @elichai