Repository navigation
ppc64: regenerate SHA-256/SHA-512 crypto asm - #12009
yosuke-wolfssl wants to merge 1 commit into
Conversation
- ppc64-sha256-asm and ppc64-sha512-asm (.S and _c.c) byte-reverse each message word with vperm under __LITTLE_ENDIAN__ in the crypto transforms.
There was a problem hiding this comment.
🔵 Needs a closer look
The architecture-specific cryptographic assembly requires human confirmation of PPC64 known-answer tests for standalone and inline variants, as inline execution and the inaccessible generator PR could not be independently validated.
No open issues 🎉
What changed in this PR
Regenerates wolfSSL’s PPC64 SHA-256 and SHA-512 crypto assembly to correct message byte order on little-endian POWER8+ systems, while leaving big-endian behavior unchanged.
Changes:
- Builds byte-reversal masks and permutes loaded message vectors in both crypto transforms.
- Applies matching changes to standalone assembly and C inline-assembly implementations.
- Leaves the separate public Transform API fixes and PPC64LE CI coverage outside this PR.
| File | Description |
|---|---|
wolfcrypt/src/port/ppc64/ppc64-sha512-asm.S |
Reverses bytes within loaded SHA-512 doublewords on little-endian systems. |
wolfcrypt/src/port/ppc64/ppc64-sha512-asm_c.c |
Applies the SHA-512 correction to inline assembly. |
wolfcrypt/src/port/ppc64/ppc64-sha256-asm.S |
Reverses bytes within loaded SHA-256 words on little-endian systems. |
wolfcrypt/src/port/ppc64/ppc64-sha256-asm_c.c |
Applies the SHA-256 correction to inline assembly. |
Review effort: Balanced
|
|
@claude review |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #12009
Scan targets checked: wolfcrypt-port-bugs
Not reviewed (no scan target covers them): wolfcrypt/src/port/ppc64/ppc64-sha256-asm.S, wolfcrypt/src/port/ppc64/ppc64-sha512-asm.S
Coverage: 2 of 2 in-scope changed file(s) opened by the reviewer
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline finding, I checked the new lvsl/vperm mask setup in all four files and ruled out a problem there. r8 is set by li 8, 16 before the block loop and is not changed afterward, so lvsl yields the identity 0..15 permute. In the _c.c versions, "8", "v9" and "v10" are already in both clobber lists. The .S and _c.c sequences match.
Extended reasoning...
The change adds little-endian-only byte-swap permutes to the message loads of the ppc64 SHA-256/SHA-512 hardware-crypto transforms, in both the .S and inline-asm files. I ruled out the register-usage concern raised in the change summary: the lvsl base register holds 16 (16-byte aligned), and the scratch registers are declared clobbers.
| #ifdef __LITTLE_ENDIAN__ | ||
| lvsl 9, 0, 8 | ||
| vspltisb 10, 3 | ||
| vxor 9, 9, 10 | ||
| #endif /* __LITTLE_ENDIAN__ */ | ||
| lxvw4x 48, 0, 4 | ||
| #ifdef __LITTLE_ENDIAN__ | ||
| vperm 16, 16, 16, 9 |
There was a problem hiding this comment.
🔴 On ppc64le POWER8+ crypto builds with OPENSSL_EXTRA, wc_Sha256Transform and wc_Sha512Transform now return wrong digests; on the base branch they are correct. The new vperm makes the crypto transforms read a big-endian byte stream. The single-block wrappers still pass little-endian native words. Fix: on PPC64 little-endian, have both wrappers give the asm a big-endian stream (byte-reverse into a temp buffer in sha256.c, and skip the ByteReverseWords64 in sha512.c), which covers the 2 sites listed. Same pattern at 2 sites (sha256.c:2616, sha512.c:2916).
Why this was flagged
wc_Sha256Transform (wolfcrypt/src/sha256.c:2616) passes the caller's block straight to Transform_Sha256 -> SHA256_TRANSFORM_LEN -> Transform_Sha256_Len_crypto (sha256.c:1350). On little-endian its contract is native 32-bit words: test_wc_Sha256Transform feeds "\x80\x63\x62\x61..." and expects SHA-256("abc"). Before this PR, lxvw4x loaded those words natively, so POWER8+ gave the right answer. After this PR the vperm at ppc64-sha256-asm.S:3504 (and its _c.c twin) byte-swaps every word, so the result is wrong. wc_Sha512Transform (sha512.c:2916) byte-reverses data with ByteReverseWords64 into native words and then calls Transform_Sha512 -> Transform_Sha512_Len_crypto (sha512.c:2928/1892). Before, lxvd2x consumed those words correctly. Now the new vperm in ppc64-sha512-asm.S reverses them a second time. The result: SHA256_Transform/SHA512_Transform (src/ssl_crypto.c:489, 629, 706, 785) and test_wc_Sha256Transform/test_wc_Sha512Transform fail on POWER8+ ppc64le. The PR description calls the wrappers a separate fix, but there the crypto path worked and this PR newly breaks it.
Verification: wc_Sha256Transform (wolfcrypt/src/sha256.c:2616) passes the caller's block as-is to Transform_Sha256_Len_crypto. On base, lxvw4x gives W0 = 0x61626380, so the result is correct. After the PR, vperm 16, 16, 16, 9 byte-swaps every word, so W0 becomes 0x80636261 and the digest is wrong. wc_Sha512Transform (sha512.c:2911-2928) regresses the same way.
|
It's covered by #11999 |
Summary
On ppc64le built with
--enable-ppc64-asm=crypto, running on a CPU with vector crypto (POWER8+),Transform_Sha256_Len_cryptoloads the message words withlxvw4x. On little-endian that instruction loads native (little-endian) words, but every caller passes the transform a big-endian byte stream, which the scalarTransform_Sha256_Lenreads withlwbrx. So every SHA-224/256 digest is wrong.Fix (
wolfcrypt/src/port/ppc64/ppc64-sha256-asm.S)The four files are regenerated from wolfSSL/scripts
sha2/sha256.rbandsha2/sha512.rb(scripts PR: https://github.com/wolfSSL/scripts/pull/707). Under__LITTLE_ENDIAN__, both crypto transforms now:lvslgives bytes 0..15, then XOR with 3 (SHA-256) or 7 (SHA-512).vpermeach message vector right after itslxvw4x/lxvd2xload.Not in this PR:
wc_Sha256Transform(#11987) andwc_Sha512Transformon ppc64le are separate fixes. Adding ppc64le to the multi-arch CI is a follow-up.Verification
testwolfcrypt:--enable-ppc64-asm=cryptoSHA-256 test failed! error L=7557--enable-ppc64-asm=crypto