Skip to contentSkip to conversation

crypto: ecdh - avoid unaligned accesses in ecdh_set_secret()

PATCH In review Ard Biesheuvel 2 participants 2 messages Nov 24, 2020 – Dec 4, 2020 Crypto API

lore
Share & tools
lore

Ard Biesheuvel

author
#0 lore raw

PATCH 1/1 crypto: ecdh - avoid unaligned accesses in ecdh_set_secret()

ecdh_set_secret() casts a void* pointer to a const u64* in order to feed it into ecc_is_key_valid(). This is not generally permitted by the C standard, and leads to actual misalignment faults on ARMv6 cores. In some cases, these are fixed up in software, but this still leads to performance hits that are entirely avoidable.

So let's copy the key into the ctx buffer first, which we will do anyway in the common case, and which guarantees correct alignment.

Cc <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
 crypto/ecdh.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
crypto/ecdh.c +5 -4
index b0232d6ab4ce..d56b8603dec9 100644
@@ -53,12 +53,13 @@ static int ecdh_set_secret(struct crypto_kpp *tfm, const void *buf,
 		return ecc_gen_privkey(ctx->curve_id, ctx->ndigits,
 				       ctx->private_key);
 
-	if (ecc_is_key_valid(ctx->curve_id, ctx->ndigits,
-			     (const u64 *)params.key, params.key_size) < 0)
-		return -EINVAL;
-
 	memcpy(ctx->private_key, params.key, params.key_size);
 
+	if (ecc_is_key_valid(ctx->curve_id, ctx->ndigits,
+			     ctx->private_key, params.key_size) < 0) {
+		memzero_explicit(ctx->private_key, params.key_size);
+		return -EINVAL;
+	}
 	return 0;
 }
Signature · 1 line
2.17.1
maintainer
#1 lore raw
Quoted 6 lines · Ard Biesheuvel

ecdh_set_secret() casts a void* pointer to a const u64* in order to feed it into ecc_is_key_valid(). This is not generally permitted by the C standard, and leads to actual misalignment faults on ARMv6 cores. In some cases, these are fixed up in software, but this still leads to performance hits that are entirely avoidable.

Quoted 8 lines

So let's copy the key into the ctx buffer first, which we will do anyway in the common case, and which guarantees correct alignment.

Cc <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
 crypto/ecdh.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Patch applied. Thanks.

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt