Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

20250103-Sha512Final-no-scratch-digest #8329

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

douzzer
Copy link
Contributor

@douzzer douzzer commented Jan 3, 2025

wolfcrypt/src/sha512.c: in Sha512FinalRaw() and wc_Sha384FinalRaw(), refactor out the scratch digest -- ByteReverseWords64() is safe in-place, and the scratch digest caused a SEGV in the XMEMCPY() on AVX512-capable targets built with gcc -march=native -O2 unless XALIGN(64), due to gcc bug(s).

Note, the bug only manifests in ASAN builds, repro'd on gcc-13, gcc-14, and gcc-15. Regular -O2 build passes unit.test without this patch.

tested with wolfssl-multi-test.sh check-source-text sp-all-asm-sanitizer on an AVX512 target.

…refactor out the scratch digest -- ByteReverseWords64() is safe in-place, and the scratch digest caused a SEGV in the XMEMCPY() on AVX512-capable targets built with gcc -march=native unless XALIGN(64), due to gcc bug(s).
@douzzer douzzer requested a review from SparkiDev January 3, 2025 17:54
@douzzer douzzer assigned douzzer, SparkiDev and wolfSSL-Bot and unassigned douzzer Jan 3, 2025
if (sha512 == NULL || hash == NULL) {
return BAD_FUNC_ARG;
}

#ifdef LITTLE_ENDIAN_ORDER
ByteReverseWords64((word64*)digest, (word64*)sha512->digest,
ByteReverseWords64((word64*)sha512->digest, (word64*)sha512->digest,
Copy link
Contributor

Choose a reason for hiding this comment

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

ByteReverseWords64() doesn't handle unaligned access.

@SparkiDev SparkiDev assigned douzzer and unassigned SparkiDev Jan 5, 2025
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.

3 participants