AddressSanitizer: SEGV mbedtls/library/bignum.c:1165 mpi_mul_hlp #1550

LaszloLango · 2018-04-03T07:50:45Z

Description

Type: Bug
Priority: Major

Bug

OS
linux

mbed TLS build:
Version: 2.8.0 (8be0e6d)
OS version: Ubuntu 16.04.9
Configuration: default configuration
Compiler:gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
Options: CFLAGS="-fsanitize=address -m32 -g"

Expected behavior
Returns 0

Actual behavior

ASAN:SIGSEGV
=================================================================
==28637==ERROR: AddressSanitizer: SEGV on unknown address 0x03f8371a (pc 0x080633a9 bp 0xffa40068 sp 0xffa3ff90 T0)
    #0 0x80633a8 in mpi_mul_hlp mbedtls/library/bignum.c:1165
    #1 0x8063a96 in mbedtls_mpi_mul_mpi mbedtls/library/bignum.c:1205
    #2 0x8063d1a in mbedtls_mpi_mul_int mbedtls/library/bignum.c:1229
    #3 0x80648f2 in mbedtls_mpi_div_mpi mbedtls/library/bignum.c:1397
    #4 0x8065298 in mbedtls_mpi_mod_mpi mbedtls/library/bignum.c:1469
    #5 0x809150e in mbedtls_rsa_deduce_crt mbedtls/library/rsa_internal.c:465
    #6 0x80509f0 in mbedtls_rsa_complete mbedtls/library/rsa.c:319
    #7 0x804e251 in pk_parse_key_pkcs1_der mbedtls/library/pkparse.c:758
    #8 0x804f525 in mbedtls_pk_parse_key mbedtls/library/pkparse.c:1172
    #9 0x8048e3b in main mbedtls-asan-error.c:36
    #10 0xf6fbd636 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x18636)
    #11 0x8048f0f  (mbedtls-asan-error+0x8048f0f)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV mbedtls/library/bignum.c:1165 mpi_mul_hlp

Steps to reproduce

compile mbedtls libraries:
cd $MBEDTLS_HOME
make clean
CFLAGS="-fsanitize=address -m32 -g" make lib
compile sample (see the code block below):
cd $HOME
gcc mbedtls-asan-error.c -o mbedtls-asan-error -O2 -fsanitize=address -m32 -g -I$MBEDTLS_HOME/include -L$MBEDTLS_HOME/library -lmbedcrypto

Code:

#include "mbedtls/ssl.h"

const char SSL_PKEY[] =
"-----BEGIN RSA PRIVATE KEY-----\n"
"MIIEpAIBAAKCAQEAyPlnutdMskKWVd+Rf2Z8Bgp+oJgciT16izkuWglVZk2ttQ1Q\n"
"vvhj1wqRkqlrOWv5ZbsEOkx3SbcMEoZ2SSKs5DINhc8UlNB8H4TBrX+jBbZgW6U+\n"
"sgPkCW5jWucveN32AcptPSgK2fD3UAloQQu9tsAkE3u+1wLlpr/f1SrjPOZnR5EO\n"
"A9NwRWKY9x+ivTfrmKozuUI/Tv8X2vtiIJMoIj9ZVzxpGolclQmW61qVAuTx5xi8\n"
"RbRO7zgUe32+gM4KmKwIvnX4mOyo1438XqWlGkSM0npEbWvmqtthldUeWUgocRne\n"
"qbIk2n2Yg/O8ghLEFY+vSvB8XdYrEvZKY/ZSIQIDAQABAoIBAAO6nPIajJHCKoft\n"
"XgW/IQ37o93W9LCXM27h4LpK8gqz5kU1ugnREgkraQgPnK20EyyQC8QJJy12+AJf\n"
"/FHfEtYpF5ckAH3CYIs1K2LMu3mfqKoKpt8ms1R5d95366mSdL4Tw3MfCxSAJBaY\n"
"Fhce4KZDamfyI9hZdWlipgSOhyjaFSH6SdiS8u8P9SV9ySHNfRX4UPQ7a5HUg2FG\n"
"sXCObFvSdAkkpAzNl+1gzAO9cy6hOjS4epHa3P/B0uGG6/FHJbT1zFKBJxGi2WFn\n"
"lf0jURU1q8026gaoh3sgrK3yX00Q9M/hIkVUaUYXzdMv2sPYP+fxECtAOkSUDipb\n"
"qZsdAAECgYEA8Hsy+7uFNpyo6Q07sZ3fbsnk+uAI9Blw6Ezz4nsGEAHQ7bXqQUOh\n"
"wyWItyL57KwwVSaTlVCZwp2V8tU4ybiyhf1FyWJgSwFoj2XJMOlR2EmqNUjXN91X\n"
"mV62hWJIJhtRaQKEGT9JZsGDLfPuen9gFUjXmFbe37Tf5A/NpUVZGGECgYEA1fGK\n"
"vXgjYagd2o5TY7xEgtEAZV1bBQ0d7lYDjvZsDzug+gfqU1MwtXEJAIMEQL4ywI1t\n"
"uP4QryK5J65PmBuq474HqWa4txwqM38J+Z8oYplJSXTaybSYD+Vf2R+KLkFHMchH\n"
"2EPzJE6lYYMJDrSW8iC5wCaC+fbJ6QfTmZZ8kcECgYEAoUBrImNOYx1PId6OvX33\n"
"+YkFsreRKWT50brv+li16vvcxdiquJKKIJnFf8/DOFEJo79XTNMcF2SlzIvvJUxk\n"
"4PXA2tXNbd4G58i/zL1W9SoIKOyr67jO6XeZ+fy6FltRDpHyVB+cr3to4+Jicd+B\n"
"ZSRP9MWjcuwNCRcTtRO4N2ECgYAEM40c8WoIdeu4KglbMQxLYV1XoEC0VbCbyJaj\n"
"TRWMKwibQGKKplyTg5fAqdIAj3uhqmVYN60OM2ldbR/lBc4SUN4HppvEBMqTXlBM\n"
"1aJOZWI6DhBp26EM1t1N/z+Qbvm98YfvqE3zDZRT2OXpowQ/1wKu0lLKI92NNPkj\n"
"z/+8QQKBgQDPCwtgT2m8SGcRM0MtYK2FSXDrQIXDISQ/c2/7Xrhzx4C7anj2z0Bp\n"
"BjT9ufv86DkZUr31ObP6B8SW9J9EPlQRhHOD6h2XIPs+stxEbm1LvQGVU1sFA/C9\n"
"Un0H1iDxSOnLjK1StjWwYfIy6SHFRLYOAmX0BPglJ+5p6Eq/dc96PQ==\n"
"-----END RSA PRIVATE KEY-----\n"
;

int main(void) {
  mbedtls_pk_context pkey;
  mbedtls_pk_init(&pkey);
  mbedtls_pk_parse_key(&pkey, (const unsigned char *) SSL_PKEY, sizeof(SSL_PKEY), NULL, 0);
  mbedtls_pk_free(&pkey);
  return 0;
}

Additional information:

The error happens with every generated key (both openssl and mbedtls), e.g:
openssl genrsa -out ryans-key.pem 2048
or
programs/pkey/gen_key type=rsa rsa_keysize=4096 filename=our_key.key

The text was updated successfully, but these errors were encountered:

RonEld · 2018-04-03T08:21:20Z

@LaszloLango Thank you for raising this issue!
We will look into this.
Note: This happens only with the -m32 flag defined

RonEld · 2018-04-03T08:28:36Z

@LaszloLango I believe #814 fixes this issue

hanno-becker · 2018-04-03T08:37:01Z

@RonEld It just disables the relevant assembly, though, not identifying the underlying problem.

RonEld · 2018-04-03T08:44:39Z

@hanno-arm yes, it's a workaround.
But investigating the full assembly code \ implementing assembly code for 32 bit will probably take long time, which I don't see the added value in it

hanno-becker · 2018-04-03T08:49:04Z

@RonEld There is definitely added value -- it's not acceptable to have assembly which is broken for unknown reasons in our library.

RonEld · 2018-04-03T08:52:59Z

@hanno-arm let me rephrase.
This assembly code is still used for more than i386.
Now, this assembly will not be used for i386 and it will use the c code.
I meant I don't see added value at the moment on investing resources on 32 bit linux platforms

hanno-becker · 2018-04-03T08:54:22Z

@RonEld Is it? The workaround adds a #if 0 around the respective assembly.

RonEld · 2018-04-03T08:58:08Z

@hanno-arm correct, it was supposed to be for i386 , it's a copy\ paste error from same implementation for sparc

RonEld · 2018-04-03T09:55:41Z

@hanno-arm I fixed the workaround, to avoid i386 only until it will be resolved, if we decide to invest resources in it

ciarmcom · 2018-04-03T11:51:27Z

ARM Internal Ref: IOTSSL-2206

RonEld · 2018-04-09T11:05:33Z

ARM Internal Ref: IOTSSL-1139 , as 2206 is a duplicate

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.

redtangent · 2018-06-24T12:26:38Z

I've just posted a fix as PR #1778 .

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.

simonbutcher · 2018-07-20T19:25:39Z

The PR's for this issue (#1778, #1849 and #1850) are now merged, so this issue can be closed.

LaszloLango · 2018-07-23T06:35:40Z

Will this fix be in the next release?

RonEld · 2018-07-23T07:14:00Z

@LaszloLango the fix has been merged and backported, so it should be in the next release

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>

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>

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>

RonEld added bug tracking labels Apr 3, 2018

RonEld added the fix available label Apr 3, 2018

RonEld mentioned this issue Apr 3, 2018

Workaround for 32 bit linux platform failure #814

Closed

LaszloLango mentioned this issue Apr 3, 2018

Use TLS module in HTTPS jerryscript-project/iotjs#1560

Merged

ciarmcom added the mirrored label Apr 3, 2018

simonbutcher added the component-crypto Crypto primitives and low-level interfaces label Jun 13, 2018

redtangent mentioned this issue Jun 24, 2018

Fix the i386 MPI multiply helper assembly code #1778

Merged

4 tasks

redtangent added a commit to redtangent/mbedtls that referenced this issue Jun 24, 2018

Add fix for Mbed-TLS#1550 and credit to the ChangeLog

6c34442

simonbutcher mentioned this issue Jun 28, 2018

Expand i386 all.sh tests to include full configuration ASan builds #1810

Merged

4 tasks

redtangent added a commit to redtangent/mbedtls that referenced this issue Jul 10, 2018

Add fix for Mbed-TLS#1550 and credit to the ChangeLog

c6bf5b4

redtangent added a commit to redtangent/mbedtls that referenced this issue Jul 10, 2018

Add fix for Mbed-TLS#1550 and credit to the ChangeLog

54cf322

This was referenced Jul 10, 2018

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

Merged

Backport 2.1: Fix the i386 MPI multiply helper assembly code #1850

Merged

simonbutcher closed this as completed Jul 20, 2018

ciarmcom added the closed_in_jira label Jul 20, 2018

This was referenced Jul 20, 2018

Backport 2.7: Expand i386 all.sh tests to include full configuration ASan builds #1872

Merged

Backport 2.1: Expand i386 all.sh tests to include full configuration ASan builds #1873

Merged

jacmet mentioned this issue Aug 27, 2018

bn_mul.h: fix x86 PIC inline ASM compilation with GCC < 5 #1986

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AddressSanitizer: SEGV mbedtls/library/bignum.c:1165 mpi_mul_hlp #1550

AddressSanitizer: SEGV mbedtls/library/bignum.c:1165 mpi_mul_hlp #1550

LaszloLango commented Apr 3, 2018

RonEld commented Apr 3, 2018

RonEld commented Apr 3, 2018

hanno-becker commented Apr 3, 2018

RonEld commented Apr 3, 2018

hanno-becker commented Apr 3, 2018

RonEld commented Apr 3, 2018

hanno-becker commented Apr 3, 2018

RonEld commented Apr 3, 2018

RonEld commented Apr 3, 2018

ciarmcom commented Apr 3, 2018

RonEld commented Apr 9, 2018

redtangent commented Jun 24, 2018

simonbutcher commented Jul 20, 2018

LaszloLango commented Jul 23, 2018

RonEld commented Jul 23, 2018

AddressSanitizer: SEGV mbedtls/library/bignum.c:1165 mpi_mul_hlp #1550

AddressSanitizer: SEGV mbedtls/library/bignum.c:1165 mpi_mul_hlp #1550

Comments

LaszloLango commented Apr 3, 2018

Description

Bug

RonEld commented Apr 3, 2018

RonEld commented Apr 3, 2018

hanno-becker commented Apr 3, 2018

RonEld commented Apr 3, 2018

hanno-becker commented Apr 3, 2018

RonEld commented Apr 3, 2018

hanno-becker commented Apr 3, 2018

RonEld commented Apr 3, 2018

RonEld commented Apr 3, 2018

ciarmcom commented Apr 3, 2018

RonEld commented Apr 9, 2018

redtangent commented Jun 24, 2018

simonbutcher commented Jul 20, 2018

LaszloLango commented Jul 23, 2018

RonEld commented Jul 23, 2018