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

Update SHA proof for "moving AArch64/X86_64 dispatching to C" changes #155

Merged
merged 4 commits into from
Jul 29, 2024

Conversation

pennyannn
Copy link
Contributor

@pennyannn pennyannn commented Jun 14, 2024

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

AWS-LC PR: aws/aws-lc#1625
Ticket: V1417594860

The AWS-LC PR refactors existing SHA2 code to move AArch86/x86_64 dispatching code from within the assembly to within the C. In doing so, assembly function names are changed in the following way:

  1. For x86_64, the sha512_block_data_order routine is now called sha512_block_data_order_avx
  2. For AArch64, the sha512_block_data_order routine is now called sha512_block_data_order_nohw (for Graviton2) and sha512_block_armv8 routine is now called sha512_block_data_order_hw (for Graviton3)

While working on the fix, I solved a problem in the proof of EVP_DigestFinal. Previously, the proof fails for SHA512 because the LLVM optimization introduces pointer comparisons, which is an undefined behaviour in SAW (see GaloisInc/saw-script#1308). SAW produces the error:
"Comparison of pointers from different allocations"

In this fix, we use SAW feature enable_lax_pointer_ordering to allow pointer comparison introduced by LLVM optimization. In addition, this proof requires defining an override for CRYPTO_bswap8 for the version that uses __builtin_bswap64:

static inline uint64_t CRYPTO_bswap8(uint64_t x) {
  return __builtin_bswap64(x);
}

@pennyannn pennyannn changed the title Update SHA proof for moving AArch64/X86_64 dispatching to C changes Update SHA proof for "moving AArch64/X86_64 dispatching to C" changes Jun 14, 2024
@pennyannn pennyannn force-pushed the yppe/fix-1625 branch 8 times, most recently from edd1c3f to b7d1a16 Compare June 18, 2024 18:08
@pennyannn pennyannn marked this pull request as ready for review June 18, 2024 18:10
@pennyannn pennyannn requested a review from andrewhop as a code owner June 18, 2024 18:10
@pennyannn pennyannn force-pushed the yppe/fix-1625 branch 11 times, most recently from 0f2b2ba to 998d6ca Compare June 18, 2024 21:14
Copy link
Contributor

@robdockins robdockins left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me, modulo a couple of minor questions.

SAW/proof/SHA512/SHA512.saw Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@pennyannn pennyannn requested a review from darylmartin100 June 20, 2024 18:14
@pennyannn pennyannn force-pushed the yppe/fix-1625 branch 3 times, most recently from 14c122d to c4dacc9 Compare July 12, 2024 20:09
@pennyannn pennyannn mentioned this pull request Jul 22, 2024
@pennyannn pennyannn force-pushed the yppe/fix-1625 branch 2 times, most recently from c0d9b48 to 3cf2806 Compare July 22, 2024 23:05
@justsmth justsmth merged commit c8c1569 into awslabs:master Jul 29, 2024
5 checks passed
@pennyannn pennyannn deleted the yppe/fix-1625 branch July 29, 2024 20:54
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.

4 participants