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

MPI inline assembly doesn't compile for PIC on i386 with older gcc #1910

Closed
simonbutcher opened this issue Aug 2, 2018 · 7 comments · Fixed by #1986
Closed

MPI inline assembly doesn't compile for PIC on i386 with older gcc #1910

simonbutcher opened this issue Aug 2, 2018 · 7 comments · Fixed by #1986
Labels
bug component-crypto Crypto primitives and low-level interfaces

Comments

@simonbutcher
Copy link
Contributor

Note: This is just a template, so feel free to use/remove the unnecessary things

Description

  • Type: Bug
  • Priority: Minor

Bug

mbed TLS build:
Version: mbedtls-2.12.0

With older versions of gcc, Mbed TLS is unable to compile in very specific circumstances:-

  • if optimisation is enabled (e.g. -O1 or -O2)
  • for an i386 target
  • with a version of gcc at least earlier than v6
  • when position independent code is enable for a shared library (e.g. -fPIC and -shared)

The problem is that the inline assembly for i386 specifies a lot of registers to clobber, and older versions of gcc can't compile with so few registers.

To reproduce the fault:-

  • use an older version of Debian i386. I used Jessue 8.5 (which at time of writing is old but still supported). It supplies gcc version 4.9.2.
  • download the Mbed TLS source code and execute the command:
gcc -O2 -fPIC -Iinclude -shared library/bignum.c

This will result in the following error:

In file included from library/bignum.c:47:0:
library/bignum.c: In function ‘mpi_mul_hlp’:
include/mbedtls/bn_mul.h:46:13: error: PIC register clobbered by ‘ebx’ in ‘asm’
 #define asm __asm
             ^
include/mbedtls/bn_mul.h:62:5: note: in expansion of macro ‘asm’
     asm(                                    \
     ^
library/bignum.c:1142:9: note: in expansion of macro ‘MULADDC_INIT’
         MULADDC_INIT
         ^
include/mbedtls/bn_mul.h:46:13: error: PIC register clobbered by ‘ebx’ in ‘asm’
 #define asm __asm
             ^

Workarounds

There’s a couple of ways around this:

  • the easiest option is probably going to be to update the compiler. The code will work with gcc 7 and gcc 6 with no issue. There's also no issue with clang, so swapping to clang may fix this.

  • you can specify to turn off inline assembly. This can be done by commenting out the line ’#define MBEDTLS_HAVE_ASM in the Mbed TLS configuration file, found in include/mbedlts/config.h. That slows down some of the asymmetric crypto operations slower, but at least it builds.

@simonbutcher simonbutcher added bug tracking component-crypto Crypto primitives and low-level interfaces labels Aug 2, 2018
@ciarmcom
Copy link

ciarmcom commented Aug 2, 2018

ARM Internal Ref: IOTSSL-2461

@wyattoday
Copy link

wyattoday commented Aug 2, 2018

We encountered this bug and worked around this problem by modifying include/mbedtls/bn_mul.h to not use the i386 "big num" assembly when -fPIC is used during the compilation. So, modifying this...

#if defined(__i386__) && defined(__OPTIMIZE__)

#define MULADDC_INIT                        \
    asm(                                    \
....

To this:

#if defined(__i386__) && defined(__OPTIMIZE__) && !defined(__PIC__)

#define MULADDC_INIT                        \
    asm(                                    \
....

Note that __PIC__ is defined if -fPIC is used during compilation.

This solution for us was preferable to the other options (using a newer compiler, dropping support for x86, not using position independent code, disabling ASM throughout mbedTLS, etc.)

jacmet added a commit to jacmet/mbedtls that referenced this issue Aug 27, 2018
Fixes Mbed-TLS#1910

With ebx added to the MULADDC_STOP clobber list to fix Mbed-TLS#1550, the inline
assembly fails to build with GCC<5 in PIC mode with the following error:

bn_mul.h:46:13: error: PIC register clobbered by ‘ebx’ in ‘asm’

This is because older GCC versions treated the x86 ebx register (which is
used for the GOT) as a fixed reserved register when building as PIC.

This is fixed by an improved register allocator in GCC 5+.  From the release
notes:

Register allocation improvements: Reuse of the PIC hard register, instead of
using a fixed register, was implemented on x86/x86-64 targets.  This
improves generated PIC code performance as more hard registers can be used.

https://www.gnu.org/software/gcc/gcc-5/changes.html

As a workaround, detect this situation and disable the inline assembly,
similar to the MULADDC_CANNOT_USE_R7 logic.

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
jacmet added a commit to jacmet/mbedtls that referenced this issue Aug 27, 2018
Fixes Mbed-TLS#1910

With ebx added to the MULADDC_STOP clobber list to fix Mbed-TLS#1550, the inline
assembly fails to build with GCC < 5 in PIC mode with the following error:

include/mbedtls/bn_mul.h:46:13: error: PIC register clobbered by ‘ebx’ in ‘asm’

This is because older GCC versions treated the x86 ebx register (which is
used for the GOT) as a fixed reserved register when building as PIC.

This is fixed by an improved register allocator in GCC 5+.  From the release
notes:

Register allocation improvements: Reuse of the PIC hard register, instead of
using a fixed register, was implemented on x86/x86-64 targets.  This
improves generated PIC code performance as more hard registers can be used.

https://www.gnu.org/software/gcc/gcc-5/changes.html

As a workaround, detect this situation and disable the inline assembly,
similar to the MULADDC_CANNOT_USE_R7 logic.

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
SSE4 added a commit to bincrafters/conan-mbedtls that referenced this issue Oct 9, 2018
Signed-off-by: SSE4 <tomskside@gmail.com>
@mckaygerhard
Copy link

we need #1986 merged please

@gilles-peskine-arm
Copy link
Contributor

@mckaygerhard Can you please make a rebased version of it for the three supported branches (development, development_2.x, mbedtls-2.16)?

@mckaygerhard
Copy link

i can help with that.. for the weekend if you guys wait for me! thanks to accept the proposal

@mckaygerhard
Copy link

mckaygerhard commented May 2, 2022

I forgot about this, I was too busy, this patch must be apply .. yet many devices still use gcc 4.X for example embedded... this patch should still be supported because linux kernels like 4.X are still in use... for devices that can't use 5.X as an example, i cannot rebase, cos i used older devices (have a lot of those with large life and i will not change cos represent a cost)

i will try again for, next weekend if there's no response

@tom-cosgrove-arm
Copy link
Contributor

Note that the supported branches are now development and mbedtls-2.28

tom-cosgrove-arm pushed a commit to tom-cosgrove-arm/mbedtls that referenced this issue Jul 19, 2022
Fixes Mbed-TLS#1910

With ebx added to the MULADDC_STOP clobber list to fix Mbed-TLS#1550, the inline
assembly fails to build with GCC < 5 in PIC mode with the following error:

include/mbedtls/bn_mul.h:46:13: error: PIC register clobbered by ‘ebx’ in ‘asm’

This is because older GCC versions treated the x86 ebx register (which is
used for the GOT) as a fixed reserved register when building as PIC.

This is fixed by an improved register allocator in GCC 5+.  From the release
notes:

Register allocation improvements: Reuse of the PIC hard register, instead of
using a fixed register, was implemented on x86/x86-64 targets.  This
improves generated PIC code performance as more hard registers can be used.

https://www.gnu.org/software/gcc/gcc-5/changes.html

As a workaround, detect this situation and disable the inline assembly,
similar to the MULADDC_CANNOT_USE_R7 logic.

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
daverodgman pushed a commit that referenced this issue Jul 21, 2022
Fixes #1910

With ebx added to the MULADDC_STOP clobber list to fix #1550, the inline
assembly fails to build with GCC < 5 in PIC mode with the following error:

include/mbedtls/bn_mul.h:46:13: error: PIC register clobbered by ‘ebx’ in ‘asm’

This is because older GCC versions treated the x86 ebx register (which is
used for the GOT) as a fixed reserved register when building as PIC.

This is fixed by an improved register allocator in GCC 5+.  From the release
notes:

Register allocation improvements: Reuse of the PIC hard register, instead of
using a fixed register, was implemented on x86/x86-64 targets.  This
improves generated PIC code performance as more hard registers can be used.

https://www.gnu.org/software/gcc/gcc-5/changes.html

As a workaround, detect this situation and disable the inline assembly,
similar to the MULADDC_CANNOT_USE_R7 logic.

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-crypto Crypto primitives and low-level interfaces
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants