commit 771c47055f2b9db6762ab32dab36092952990573
author Minos Galanakis <minos.galanakis@arm.com> Thu Oct 27 12:22:22 2022 +0100
committer Minos Galanakis <minos.galanakis@arm.com> Thu Oct 27 12:36:24 2022 +0100
tree ebfe6449a6af757c7f1bdf61a8fd902ad068db45
parent dd365a526fc32cd688558e0c4aa9642f4670c97f

bignum_mod: Style changes

This patch addresses review comments with regards to style of
`mbedtls_mpi_mod_modulus_setup/free()`.

It also removes a test check which was triggering a use-after-free.

Signed-off-by: Minos Galanakis <minos.galanakis@arm.com>

library/bignum_mod.c

diff --git a/library/bignum_mod.c b/library/bignum_mod.c
index 92c011c..705b2db 100644
--- a/library/bignum_mod.c
+++ b/library/bignum_mod.c
@@ -81,7 +81,8 @@
                                       m->limbs );
             mbedtls_free( (mbedtls_mpi_uint *)m->rep.mont.rr );
             m->rep.mont.rr = NULL;
-            m->rep.mont.mm = 0; break;
+            m->rep.mont.mm = 0;
+            break;
         case MBEDTLS_MPI_MOD_REP_OPT_RED:
             mbedtls_free( m->rep.ored );
             break;
@@ -110,11 +111,11 @@
     if ( A == NULL || limbs == 0 || limbs >= ( MBEDTLS_MPI_MAX_LIMBS / 2 ) - 2 )
         goto cleanup;
 
-    if ( !mbedtls_mpi_grow( &N,  limbs ))
-        memcpy( N.p, A, sizeof(mbedtls_mpi_uint) *  limbs );
-    else
+    if ( mbedtls_mpi_grow( &N,  limbs ))
         goto cleanup;
 
+    memcpy( N.p, A, sizeof(mbedtls_mpi_uint) *  limbs );
+
     mbedtls_mpi_core_get_mont_r2_unsafe(&RR, &N);
 
     *X = RR.p;

tests/suites/test_suite_bignum_mod.function

diff --git a/tests/suites/test_suite_bignum_mod.function b/tests/suites/test_suite_bignum_mod.function
index 8716f67..2e0377a 100644
--- a/tests/suites/test_suite_bignum_mod.function
+++ b/tests/suites/test_suite_bignum_mod.function
@@ -18,7 +18,6 @@
     mbedtls_mpi_uint mp[MLIMBS];
     mbedtls_mpi_mod_modulus m;
     const size_t mp_size = sizeof(mbedtls_mpi_uint);
-    mbedtls_mpi_uint * rr;
     int ret;
 
     memset( mp, 0xFF, mp_size );
@@ -34,8 +33,6 @@
         TEST_ASSERT( m.rep.mont.rr != NULL );
         TEST_ASSERT( m.rep.mont.mm != 0 );
 
-        /* Keep a copy of the memory location used to store Montgomery const */
-        rr = (mbedtls_mpi_uint *)m.rep.mont.rr;
     }
 
     /* Address sanitiser should catch if we try to free mp */
@@ -54,10 +51,6 @@
         TEST_ASSERT( m.rep.mont.rr == NULL );
         TEST_ASSERT( m.rep.mont.mm == 0 );
 
-        /* mbedtls_mpi_mod_modulus_free() has set the
-         * (mbedtls_mpi_uint *)m.rep.mont.rr -> NULL.
-         * Verify that the actual data have been zeroed */
-        ASSERT_COMPARE( rr, mp_size, &mp, mp_size );
     }
 exit:
     /* It should be safe to call an mbedtls free several times */