From a5f8b420561b198a310600cfc9f8f0bd1c179f49 Mon Sep 17 00:00:00 2001 From: James Cowgill Date: Thu, 17 Dec 2015 01:40:26 +0000 Subject: [PATCH 1/7] Fix build errors on x32 by using the generic 'add' instruction On x32 systems, pointers are 4-bytes wide and are therefore stored in %e?x registers (instead of %r?x registers). These registers must be accessed using "addl" instead of "addq", however the GNU assembler will acccept the generic "add" instruction and determine the correct opcode based on the registers passed to it. --- library/aesni.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/aesni.c b/library/aesni.c index 83a5868bd7..1ca3c3ef5b 100644 --- a/library/aesni.c +++ b/library/aesni.c @@ -100,7 +100,7 @@ int mbedtls_aesni_crypt_ecb( mbedtls_aes_context *ctx, asm( "movdqu (%3), %%xmm0 \n\t" // load input "movdqu (%1), %%xmm1 \n\t" // load round key 0 "pxor %%xmm1, %%xmm0 \n\t" // round 0 - "addq $16, %1 \n\t" // point to next round key + "add $16, %1 \n\t" // point to next round key "subl $1, %0 \n\t" // normal rounds = nr - 1 "test %2, %2 \n\t" // mode? "jz 2f \n\t" // 0 = decrypt @@ -108,7 +108,7 @@ int mbedtls_aesni_crypt_ecb( mbedtls_aes_context *ctx, "1: \n\t" // encryption loop "movdqu (%1), %%xmm1 \n\t" // load round key AESENC xmm1_xmm0 "\n\t" // do round - "addq $16, %1 \n\t" // point to next round key + "add $16, %1 \n\t" // point to next round key "subl $1, %0 \n\t" // loop "jnz 1b \n\t" "movdqu (%1), %%xmm1 \n\t" // load round key @@ -118,7 +118,7 @@ int mbedtls_aesni_crypt_ecb( mbedtls_aes_context *ctx, "2: \n\t" // decryption loop "movdqu (%1), %%xmm1 \n\t" AESDEC xmm1_xmm0 "\n\t" // do round - "addq $16, %1 \n\t" + "add $16, %1 \n\t" "subl $1, %0 \n\t" "jnz 2b \n\t" "movdqu (%1), %%xmm1 \n\t" // load round key From ca20ced208f8edf1845e3a8eaca1a0401cd65252 Mon Sep 17 00:00:00 2001 From: James Cowgill Date: Thu, 17 Dec 2015 01:51:09 +0000 Subject: [PATCH 2/7] Fix segfault on x32 by using better register constraints in bn_mul.h On x32, pointers are only 4-bytes wide and need to be loaded using the "movl" instruction instead of "movq" to avoid loading garbage into the register. The MULADDC routines for x86-64 are adjusted to work on x32 as well by getting gcc to load all the registers for us in advance (and storing them later) by using better register constraints. The b, c, D and S constraints correspond to the rbx, rcx, rdi and rsi registers respectively. --- include/mbedtls/bn_mul.h | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/bn_mul.h b/include/mbedtls/bn_mul.h index 1fc7aa68db..cac3f14577 100644 --- a/include/mbedtls/bn_mul.h +++ b/include/mbedtls/bn_mul.h @@ -162,10 +162,6 @@ #define MULADDC_INIT \ asm( \ - "movq %3, %%rsi \n\t" \ - "movq %4, %%rdi \n\t" \ - "movq %5, %%rcx \n\t" \ - "movq %6, %%rbx \n\t" \ "xorq %%r8, %%r8 \n\t" #define MULADDC_CORE \ @@ -181,12 +177,9 @@ "addq $8, %%rdi \n\t" #define MULADDC_STOP \ - "movq %%rcx, %0 \n\t" \ - "movq %%rdi, %1 \n\t" \ - "movq %%rsi, %2 \n\t" \ - : "=m" (c), "=m" (d), "=m" (s) \ - : "m" (s), "m" (d), "m" (c), "m" (b) \ - : "rax", "rcx", "rdx", "rbx", "rsi", "rdi", "r8" \ + : "+c" (c), "+D" (d), "+S" (s) \ + : "b" (b) \ + : "rax", "rdx", "r8" \ ); #endif /* AMD64 */ From ce37ab7adab723dcf2f435c0698d3d4c3ca487ee Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Mon, 8 May 2017 11:15:49 +0100 Subject: [PATCH 3/7] Fix test_suite_pk.function to work on 64-bit ILP32 This change fixes a problem in the tests pk_rsa_alt() and pk_rsa_overflow() from test_suite_pk.function that would cause a segmentation fault. The problem is that these tests are only designed to run in computers where the sizeof(size_t) > sizeof(unsigned int). --- tests/suites/test_suite_pk.function | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 5fa8a693aa..6b695cbcfb 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -423,6 +423,9 @@ void pk_rsa_overflow( ) mbedtls_pk_context pk; size_t hash_len = (size_t)-1; + if( sizeof( size_t ) <= sizeof( unsigned int ) ) + return; + mbedtls_pk_init( &pk ); TEST_ASSERT( mbedtls_pk_setup( &pk, @@ -493,9 +496,12 @@ void pk_rsa_alt( ) TEST_ASSERT( mbedtls_pk_sign( &alt, MBEDTLS_MD_NONE, hash, sizeof hash, sig, &sig_len, rnd_std_rand, NULL ) == 0 ); #if defined(MBEDTLS_HAVE_INT64) - TEST_ASSERT( mbedtls_pk_sign( &alt, MBEDTLS_MD_NONE, hash, (size_t)-1, - NULL, NULL, rnd_std_rand, NULL ) == - MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + if( sizeof( size_t ) > sizeof( unsigned int ) ) + { + TEST_ASSERT( mbedtls_pk_sign( &alt, MBEDTLS_MD_NONE, hash, (size_t)-1, + NULL, NULL, rnd_std_rand, NULL ) == + MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + } #endif /* MBEDTLS_HAVE_INT64 */ TEST_ASSERT( sig_len == RSA_KEY_LEN ); TEST_ASSERT( mbedtls_pk_verify( &rsa, MBEDTLS_MD_NONE, From 401441b74d26c8bdc0bba56052d7770c8a845a02 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Mon, 8 May 2017 11:19:19 +0100 Subject: [PATCH 4/7] Add test command for 64-bit ILP32 in all.sh --- tests/scripts/all.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 8a4881af75..e333a13588 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -565,6 +565,16 @@ if uname -a | grep -F x86_64 >/dev/null; then msg "build: i386, make, gcc" # ~ 30s cleanup make CC=gcc CFLAGS='-Werror -m32' + + msg "test: i386, make, gcc" + make test + + msg "build: 64-bit ILP32, make, gcc" # ~ 30s + cleanup + make CC=gcc CFLAGS='-Werror -Wall -Wextra -mx32' + + msg "test: 64-bit ILP32, make, gcc" + make test fi # x86_64 msg "build: arm-none-eabi-gcc, make" # ~ 10s From 65915438b8dcb00e79e629328d6e8927387caa37 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Mon, 8 May 2017 13:18:39 +0100 Subject: [PATCH 5/7] Add ChangeLog entry for 64-bit ILP32 fixes --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index 51ca066068..61e3cabff0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -65,6 +65,9 @@ Bugfix * Fix bug in cipher decryption with MBEDTLS_PADDING_ONE_AND_ZEROS that sometimes accepted invalid padding. (Not used in TLS.) Found and fixed by Micha Kraus. + * Fix assembly sequences in bn_mul.h and aesni.c to avoid segmentation + faults and errors when building for the 64-bit ILP32 ABI. Found and fixed + by James Cowgill. Changes * Extend cert_write example program by options to set the CRT version From c2c343204059a92a448aede810066a79776392f1 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Fri, 9 Jun 2017 14:42:12 +0100 Subject: [PATCH 6/7] Improve test_suite_pk size_t vs unsigned int check --- tests/suites/test_suite_pk.function | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 6b695cbcfb..fbe522d692 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -5,8 +5,8 @@ #include "mbedtls/ecp.h" #include "mbedtls/rsa.h" -/* For detecting 64-bit compilation */ -#include "mbedtls/bignum.h" +#include +#include static int rnd_std_rand( void *rng_state, unsigned char *output, size_t len ); @@ -417,13 +417,13 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_RSA_C:MBEDTLS_HAVE_INT64 */ +/* BEGIN_CASE depends_on:MBEDTLS_RSA_C */ void pk_rsa_overflow( ) { mbedtls_pk_context pk; - size_t hash_len = (size_t)-1; + size_t hash_len = SIZE_MAX; - if( sizeof( size_t ) <= sizeof( unsigned int ) ) + if( SIZE_MAX <= UINT_MAX ) return; mbedtls_pk_init( &pk ); @@ -493,16 +493,13 @@ void pk_rsa_alt( ) TEST_ASSERT( strcmp( mbedtls_pk_get_name( &alt ), "RSA-alt" ) == 0 ); /* Test signature */ +#if SIZE_MAX > UINT_MAX + TEST_ASSERT( mbedtls_pk_sign( &alt, MBEDTLS_MD_NONE, hash, SIZE_MAX, + sig, &sig_len, rnd_std_rand, NULL ) == + MBEDTLS_ERR_PK_BAD_INPUT_DATA ); +#endif /* SIZE_MAX > UINT_MAX */ TEST_ASSERT( mbedtls_pk_sign( &alt, MBEDTLS_MD_NONE, hash, sizeof hash, - sig, &sig_len, rnd_std_rand, NULL ) == 0 ); -#if defined(MBEDTLS_HAVE_INT64) - if( sizeof( size_t ) > sizeof( unsigned int ) ) - { - TEST_ASSERT( mbedtls_pk_sign( &alt, MBEDTLS_MD_NONE, hash, (size_t)-1, - NULL, NULL, rnd_std_rand, NULL ) == - MBEDTLS_ERR_PK_BAD_INPUT_DATA ); - } -#endif /* MBEDTLS_HAVE_INT64 */ + sig, &sig_len, rnd_std_rand, NULL ) == 0 ); TEST_ASSERT( sig_len == RSA_KEY_LEN ); TEST_ASSERT( mbedtls_pk_verify( &rsa, MBEDTLS_MD_NONE, hash, sizeof hash, sig, sig_len ) == 0 ); From 36dde9e67acf82b355239ed9215bbe445102d8f6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 5 Dec 2017 14:42:23 +0100 Subject: [PATCH 7/7] Added ChangeLog entry for 64-bit ILP32 fix --- ChangeLog | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog b/ChangeLog index 61e3cabff0..f9153ac087 100644 --- a/ChangeLog +++ b/ChangeLog @@ -68,6 +68,7 @@ Bugfix * Fix assembly sequences in bn_mul.h and aesni.c to avoid segmentation faults and errors when building for the 64-bit ILP32 ABI. Found and fixed by James Cowgill. + * Fix test_suite_pk to work on 64-bit ILP32 systems. #849 Changes * Extend cert_write example program by options to set the CRT version