From 78d564a84156974580895e5a1aa6c24b8dd2ac64 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?=
 <manuel.pegourie-gonnard@arm.com>
Date: Tue, 14 Mar 2017 11:48:38 +0100
Subject: [PATCH] Add check for changing arguments

In case of argument change, freeing everything is not the most efficient
(wastes one free()+calloc()) but makes the code simpler, which is probably
more important here
---
 library/ecp.c                        | 24 +++++++++++++++++++++---
 tests/suites/test_suite_ecp.function |  7 +++++++
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/library/ecp.c b/library/ecp.c
index 60aa0a3cd..0a0239cc3 100644
--- a/library/ecp.c
+++ b/library/ecp.c
@@ -105,6 +105,8 @@ void mbedtls_ecp_set_max_ops( unsigned max_ops )
  */
 struct mbedtls_ecp_restart {
     unsigned char fake_it;  /* for tests: should we fake early return? */
+    mbedtls_mpi m;          /* saved argument: scalar                       */
+    mbedtls_ecp_point P;    /* saved argument: point                        */
 };
 
 /*
@@ -122,6 +124,9 @@ static void ecp_restart_free( mbedtls_ecp_restart_ctx *ctx )
 {
     if( ctx == NULL )
         return;
+
+    mbedtls_mpi_free( &ctx->m );
+    mbedtls_ecp_point_free( &ctx->P );
 }
 #endif /* MBEDTLS_ECP_EARLY_RETURN */
 
@@ -1514,22 +1519,35 @@ static int ecp_mul_comb( mbedtls_ecp_group *grp, mbedtls_ecp_point *R,
     size_t d;
     mbedtls_ecp_point *T = NULL;
 
-    /* set up restart context if needed */
 #if defined(MBEDTLS_ECP_EARLY_RETURN)
+    /* check for restart with new arguments */
+    if( grp->rs != NULL &&
+        ( mbedtls_mpi_cmp_mpi( m, &grp->rs->m ) != 0 ||
+          mbedtls_mpi_cmp_mpi( &P->X, &grp->rs->P.X ) != 0 ||
+          mbedtls_mpi_cmp_mpi( &P->Y, &grp->rs->P.Y ) != 0 ) )
+    {
+        ecp_restart_free( grp->rs );
+        mbedtls_free( grp->rs );
+        grp->rs = NULL;
+    }
+
+    /* set up restart context if needed */
     if( ecp_max_ops != 0 && grp->rs == NULL )
     {
         grp->rs = mbedtls_calloc( 1, sizeof( mbedtls_ecp_restart_ctx ) );
         if( grp->rs == NULL )
             return( MBEDTLS_ERR_ECP_ALLOC_FAILED );
+
         ecp_restart_init( grp->rs );
 
-        grp->rs->fake_it = 1;
+        MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &grp->rs->m, m ) );
+        MBEDTLS_MPI_CHK( mbedtls_ecp_copy( &grp->rs->P, P ) );
     }
 #endif
 
     /* XXX: temporary */
 #if defined(MBEDTLS_ECP_EARLY_RETURN)
-    if( grp->rs && grp->rs->fake_it++ != 0 )
+    if( grp->rs && ++grp->rs->fake_it != 0 )
         return( MBEDTLS_ERR_ECP_IN_PROGRESS );
 #endif
 
diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function
index 74e23875d..caf983e72 100644
--- a/tests/suites/test_suite_ecp.function
+++ b/tests/suites/test_suite_ecp.function
@@ -121,6 +121,13 @@ void ecp_test_vect_restart( int id,
     }
     while( ret != 0 );
 
+    /* Ok, now start an operation with some arguments, and drop it.
+     * We'll see if the result of the next operation, with different args,
+     * are correct regardless (do we discard old context on new args?).
+     * This also tests that we don't write to R prematurely */
+    ret = mbedtls_ecp_mul( &grp, &R, &dA, &grp.G, NULL, NULL );
+    TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_ECP_IN_PROGRESS );
+
     /* Non-base point case */
     cnt_restarts = 0;
     do {