crypto: ecc - Fix carry overflow in vli multiplication

The carry flag calculation fails when r01.m_high is saturated
(0xFFFFFFFFFFFFFFFF) and addition of lower bits overflows.

The condition (r01.m_high < product.m_high) doesn't handle the case
where r01.m_high == product.m_high and an additional carry exists
from lower-bit overflow.

When commit 3c4b23901a0c ("crypto: ecdh - Add ECDH software support")
introduced crypto/ecc.c, it split the muladd() function in the
micro-ecc library into separate mul_64_64() and add_128_128() helpers.
It seems the check got lost in translation.

Add proper handling for this boundary by accounting for the carry
from the lower addition.

Fixes: 3c4b23901a0c ("crypto: ecdh - Add ECDH software support")
Signed-off-by: Anastasia Tishchenko <sv3iry@gmail.com>
Cc: stable@vger.kernel.org # v4.8+
Reviewed-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/ecc.c b/crypto/ecc.c
index 43b0def3a225cc15be1c82aff77713f0b0e13558..6eb4d97a5f0d3794b23d96d1c79c9530dd0ec76e 100644 (file)
--- a/crypto/ecc.c
+++ b/crypto/ecc.c
@@ -393,14 +393,26 @@ static uint128_t mul_64_64(u64 left, u64 right)
        return result;
 }
 
-static uint128_t add_128_128(uint128_t a, uint128_t b)
+/* Calculate addition with overflow checking. Returns true on wrap-around,
+ * false otherwise.
+ */
+static bool check_add_128_128_overflow(uint128_t *result, uint128_t a,
+                                      uint128_t b)
 {
-       uint128_t result;
+       bool carry;
 
-       result.m_low = a.m_low + b.m_low;
-       result.m_high = a.m_high + b.m_high + (result.m_low < a.m_low);
+       result->m_low = a.m_low + b.m_low;
+       carry = (result->m_low < a.m_low);
 
-       return result;
+       result->m_high = a.m_high + b.m_high + carry;
+
+       /* Using constant-time bitwise arithmetic to prevent timing
+        * side-channels.
+        */
+       carry = (result->m_high < a.m_high) |
+               ((result->m_high == a.m_high) & carry);
+
+       return carry;
 }
 
 static void vli_mult(u64 *result, const u64 *left, const u64 *right,
@@ -425,9 +437,7 @@ static void vli_mult(u64 *result, const u64 *left, const u64 *right,
                        uint128_t product;
 
                        product = mul_64_64(left[i], right[k - i]);
-
-                       r01 = add_128_128(r01, product);
-                       r2 += (r01.m_high < product.m_high);
+                       r2 += check_add_128_128_overflow(&r01, r01, product);
                }
 
                result[k] = r01.m_low;
@@ -450,7 +460,7 @@ static void vli_umult(u64 *result, const u64 *left, u32 right,
                uint128_t product;
 
                product = mul_64_64(left[k], right);
-               r01 = add_128_128(r01, product);
+               check_add_128_128_overflow(&r01, r01, product);
                /* no carry */
                result[k] = r01.m_low;
                r01.m_low = r01.m_high;
@@ -487,8 +497,7 @@ static void vli_square(u64 *result, const u64 *left, unsigned int ndigits)
                                product.m_low <<= 1;
                        }
 
-                       r01 = add_128_128(r01, product);
-                       r2 += (r01.m_high < product.m_high);
+                       r2 += check_add_128_128_overflow(&r01, r01, product);
                }
 
                result[k] = r01.m_low;