From 5408f72027f5f797dffe35451650e50349e12b6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 9 Apr 2020 10:11:17 +0200 Subject: [PATCH 1/2] Fix integer overflow in benchmark program MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When building with MBEDTLS_MEMORY_DEBUG enabled, and running the ecdh part, the benchmark program would start writing a very large number of space characters on stdout, and would have to be killed because it never seemed to terminate. This was due to an integer overflow in computing how many space to leave after the title in order to get memory measurements aligned, which resulted in up to SIZE_MAX spaces being printed. This commit just fixes the overflow, the next commit is going to fix the magic number (12). Signed-off-by: Manuel Pégourié-Gonnard --- programs/test/benchmark.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/programs/test/benchmark.c b/programs/test/benchmark.c index a63a30ec8..18447105b 100644 --- a/programs/test/benchmark.c +++ b/programs/test/benchmark.c @@ -141,7 +141,8 @@ do { \ #define MEMORY_MEASURE_PRINT( title_len ) \ mbedtls_memory_buffer_alloc_max_get( &max_used, &max_blocks ); \ - for( ii = 12 - title_len; ii != 0; ii-- ) mbedtls_printf( " " ); \ + for( ii = 12 > (title_len) ? 12 - (title_len) : 1; ii !=0; ii--) \ + mbedtls_printf( " " ); \ max_used -= prv_used; \ max_blocks -= prv_blocks; \ max_bytes = max_used + MEM_BLOCK_OVERHEAD * max_blocks; \ From 49e7a951da6cab3e2e534f55745a6c61a21a8ebb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 9 Apr 2020 10:40:03 +0200 Subject: [PATCH 2/2] Get rid of a magic value in benchmark.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also update its value while at it. Signed-off-by: Manuel Pégourié-Gonnard --- programs/test/benchmark.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/programs/test/benchmark.c b/programs/test/benchmark.c index 18447105b..9300dabcd 100644 --- a/programs/test/benchmark.c +++ b/programs/test/benchmark.c @@ -133,6 +133,16 @@ do { \ #if defined(MBEDTLS_MEMORY_BUFFER_ALLOC_C) && defined(MBEDTLS_MEMORY_DEBUG) +/* How much space to reserve for the title when printing heap usage results. + * Updated manually as the output of the following command: + * + * sed -n 's/.*[T]IME_PUBLIC.*"\(.*\)",/\1/p' programs/test/benchmark.c | + * awk '{print length+2}' | sort -rn | head -n1 + * + * This computes the maximum length of a title +2 (because we appends "/s"). + * (If the value is too small, the only consequence is poor alignement.) */ +#define TITLE_SPACE 11 + #define MEMORY_MEASURE_INIT \ size_t max_used, max_blocks, max_bytes; \ size_t prv_used, prv_blocks; \ @@ -141,8 +151,8 @@ do { \ #define MEMORY_MEASURE_PRINT( title_len ) \ mbedtls_memory_buffer_alloc_max_get( &max_used, &max_blocks ); \ - for( ii = 12 > (title_len) ? 12 - (title_len) : 1; ii !=0; ii--) \ - mbedtls_printf( " " ); \ + ii = TITLE_SPACE > (title_len) ? TITLE_SPACE - (title_len) : 1; \ + while( ii-- ) mbedtls_printf( " " ); \ max_used -= prv_used; \ max_blocks -= prv_blocks; \ max_bytes = max_used + MEM_BLOCK_OVERHEAD * max_blocks; \