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

Backport 2.7: Fix the i386 MPI multiply helper assembly code #1849

Merged
merged 3 commits into from
Jul 20, 2018

Conversation

redtangent
Copy link
Contributor

Description

This is a backport of #1778 to the mbedtls-2.7 branch, to fix GitHub issue #1550.

The ebx register was used by the assembly code but not listed in the clobber list, so when the compiler chose to also use it, ebx was getting corrupted. I'm surprised this wasn't spotted sooner.

The fix is very simply to add the ebx register to the clobber list for the i386 inline assembly
for the multiply helper function.

This fixes both i386 and its SSE2 variant which was also broken.

Status

READY

Todos

  • Tests
  • Documentation
  • Changelog updated
  • Backported

This fix adds the ebx register to the clobber list for the i386 inline assembly
for the multiply helper function.

ebx was used but not listed, so when the compiler chose to also use it, ebx was
getting corrupted. I'm surprised this wasn't spotted sooner.

Fixes Github issues Mbed-TLS#1550.
We don't compile in the assembly code if compiler optimisations are disabled as
the number of registers used in the assembly code doesn't work with the -O0
option. Also anyone select -O0 probably doesn't want to compile in the assembly
code anyway.
@simonbutcher simonbutcher requested review from mpg and Patater July 10, 2018 22:29
@simonbutcher simonbutcher added bug CLA not applicable component-crypto Crypto primitives and low-level interfaces needs-review Every commit must be reviewed by at least two team members, labels Jul 10, 2018
@simonbutcher simonbutcher added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Jul 16, 2018
@simonbutcher simonbutcher merged commit d064b5c into Mbed-TLS:mbedtls-2.7 Jul 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports bug component-crypto Crypto primitives and low-level interfaces
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants