Skip to content

Commit

Permalink
mbedtls_mpi_sub_abs: check the range of the result when it happens
Browse files Browse the repository at this point in the history
The function mbedtls_mpi_sub_abs first checked that A >= B and then
performed the subtraction, relying on the fact that A >= B to
guarantee that the carry propagation would stop, and not taking
advantage of the fact that the carry when subtracting two numbers can
only be 0 or 1. This made the carry propagation code a little hard to
follow.

Write an ad hoc loop for the carry propagation, checking the size of
the result. This makes termination obvious.

The initial check that A >= B is no longer needed, since the function
now checks that the carry propagation terminates, which is equivalent.
This is a slight performance gain.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
  • Loading branch information
gilles-peskine-arm committed Jun 8, 2020
1 parent 221626f commit 0e5faf6
Showing 1 changed file with 15 additions and 9 deletions.
24 changes: 15 additions & 9 deletions library/bignum.c
Original file line number Diff line number Diff line change
Expand Up @@ -1374,20 +1374,20 @@ static mbedtls_mpi_uint mpi_sub_hlp( size_t n,
}

/*
* Unsigned subtraction: X = |A| - |B| (HAC 14.9)
* Unsigned subtraction: X = |A| - |B| (HAC 14.9, 14.10)
*/
int mbedtls_mpi_sub_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi *B )
{
mbedtls_mpi TB;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
size_t n;
mbedtls_mpi_uint c, z;
mbedtls_mpi_uint carry;
MPI_VALIDATE_RET( X != NULL );
MPI_VALIDATE_RET( A != NULL );
MPI_VALIDATE_RET( B != NULL );

if( mbedtls_mpi_cmp_abs( A, B ) < 0 )
return( MBEDTLS_ERR_MPI_NEGATIVE_VALUE );
/* if( mbedtls_mpi_cmp_abs( A, B ) < 0 ) */
/* return( MBEDTLS_ERR_MPI_NEGATIVE_VALUE ); */

mbedtls_mpi_init( &TB );

Expand All @@ -1411,11 +1411,17 @@ int mbedtls_mpi_sub_abs( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi
if( B->p[n - 1] != 0 )
break;

c = mpi_sub_hlp( n, X->p, B->p );
while( c != 0 )
{
z = ( X->p[n] < c ); X->p[n] -= c;
c = z; n++;
carry = mpi_sub_hlp( n, X->p, B->p );
if( carry != 0 )
{
/* Propagate the carry to the first nonzero limb of X. */
for( ; n < X->n && X->p[n] == 0; n++ )
--X->p[n];
/* If we ran out of space for the carry, it means that the result
* is negative. */
if( n == X->n )
return( MBEDTLS_ERR_MPI_NEGATIVE_VALUE );
--X->p[n];
}

cleanup:
Expand Down

0 comments on commit 0e5faf6

Please sign in to comment.