On 11 April 2014 18:03, gre...@linuxfoundation.org
<gre...@linuxfoundation.org> wrote:
> On Fri, Apr 04, 2014 at 10:11:19AM +0200, Ard Biesheuvel wrote:
>> Greg,
>>
>> This pertains to commit 8ceee72808d1 (crypto: ghash-clmulni-intel -
>> use C implementation for setkey()) that has been pulled by Linus
>> during the current merge window.
>>
>> It is missing two things:
>> - a cc to stable annotation
>> - a fix for the sparse warning below (change cast from __be64 to __force
>> __be64)
>>
>> The reason for cc'ing stable on this patch is that it fixes a
>> potential data corruption issue where the ghash setkey() method uses
>> SSE registers without calling kernel_fpu_begin() first. This issue was
>> introduced by 0e1227d356e9b (crypto: ghash - Add PCLMULQDQ accelerated
>> implementation).
>>
>> So how would you like to proceed with this? Should I propose a new
>> patch somewhere?
>
> No problem, I'll apply this as-is. But it doesn't apply to the
> 3.4-stable tree cleanly, can you send me a backported version if it's
> still needed there as well?
>
Yes, the code was broken from the start. 3.4 version is attached, the
only difference is the missing ENDPROC() at the end of the asm file.
In the mean time, Herbert has submitted a fix for the sparse warning,
but we settled on a different fix than I had suggested before.
https://git.kernel.org/cgit/linux/kernel/git/herbert/cryptodev-2.6.git/commit/?id=0ea481466d1c
Note that this code has not been tested (not by me, at least), so I
wouldn't suggest you take it straight away, but if you care about the
sparse warning, we could add a cc stable to it as well, I suppose.
Regards,
Ard.
From db9c70e8f3291490fec0da56dce2bfa7837e99f2 Mon Sep 17 00:00:00 2001
From: Ard Biesheuvel <ard.biesheu...@linaro.org>
Date: Fri, 11 Apr 2014 20:37:37 +0200
Subject: [PATCH] crypto: ghash-clmulni-intel - use C implementation for
setkey()
commit 8ceee72808d1ae3fb191284afc2257a2be964725 upstream.
The GHASH setkey() function uses SSE registers but fails to call
kernel_fpu_begin()/kernel_fpu_end(). Instead of adding these calls, and
then having to deal with the restriction that they cannot be called from
interrupt context, move the setkey() implementation to the C domain.
Note that setkey() does not use any particular SSE features and is not
expected to become a performance bottleneck.
Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
Acked-by: H. Peter Anvin <h...@linux.intel.com>
Fixes: 0e1227d356e9b (crypto: ghash - Add PCLMULQDQ accelerated implementation)
Signed-off-by: Herbert Xu <herb...@gondor.apana.org.au>
---
arch/x86/crypto/ghash-clmulni-intel_asm.S | 28 ----------------------------
arch/x86/crypto/ghash-clmulni-intel_glue.c | 14 +++++++++++---
2 files changed, 11 insertions(+), 31 deletions(-)
diff --git a/arch/x86/crypto/ghash-clmulni-intel_asm.S b/arch/x86/crypto/ghash-clmulni-intel_asm.S
index 1eb7f90cb7b9..eb4d2a254b35 100644
--- a/arch/x86/crypto/ghash-clmulni-intel_asm.S
+++ b/arch/x86/crypto/ghash-clmulni-intel_asm.S
@@ -24,10 +24,6 @@
.align 16
.Lbswap_mask:
.octa 0x000102030405060708090a0b0c0d0e0f
-.Lpoly:
- .octa 0xc2000000000000000000000000000001
-.Ltwo_one:
- .octa 0x00000001000000000000000000000001
#define DATA %xmm0
#define SHASH %xmm1
@@ -131,27 +127,3 @@ ENTRY(clmul_ghash_update)
movups DATA, (%rdi)
.Lupdate_just_ret:
ret
-
-/*
- * void clmul_ghash_setkey(be128 *shash, const u8 *key);
- *
- * Calculate hash_key << 1 mod poly
- */
-ENTRY(clmul_ghash_setkey)
- movaps .Lbswap_mask, BSWAP
- movups (%rsi), %xmm0
- PSHUFB_XMM BSWAP %xmm0
- movaps %xmm0, %xmm1
- psllq $1, %xmm0
- psrlq $63, %xmm1
- movaps %xmm1, %xmm2
- pslldq $8, %xmm1
- psrldq $8, %xmm2
- por %xmm1, %xmm0
- # reduction
- pshufd $0b00100100, %xmm2, %xmm1
- pcmpeqd .Ltwo_one, %xmm1
- pand .Lpoly, %xmm1
- pxor %xmm1, %xmm0
- movups %xmm0, (%rdi)
- ret
diff --git a/arch/x86/crypto/ghash-clmulni-intel_glue.c b/arch/x86/crypto/ghash-clmulni-intel_glue.c
index b4bf0a63b520..c07446d17463 100644
--- a/arch/x86/crypto/ghash-clmulni-intel_glue.c
+++ b/arch/x86/crypto/ghash-clmulni-intel_glue.c
@@ -30,8 +30,6 @@ void clmul_ghash_mul(char *dst, const be128 *shash);
void clmul_ghash_update(char *dst, const char *src, unsigned int srclen,
const be128 *shash);
-void clmul_ghash_setkey(be128 *shash, const u8 *key);
-
struct ghash_async_ctx {
struct cryptd_ahash *cryptd_tfm;
};
@@ -58,13 +56,23 @@ static int ghash_setkey(struct crypto_shash *tfm,
const u8 *key, unsigned int keylen)
{
struct ghash_ctx *ctx = crypto_shash_ctx(tfm);
+ be128 *x = (be128 *)key;
+ u64 a, b;
if (keylen != GHASH_BLOCK_SIZE) {
crypto_shash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
return -EINVAL;
}
- clmul_ghash_setkey(&ctx->shash, key);
+ /* perform multiplication by 'x' in GF(2^128) */
+ a = be64_to_cpu(x->a);
+ b = be64_to_cpu(x->b);
+
+ ctx->shash.a = (__be64)((b << 1) | (a >> 63));
+ ctx->shash.b = (__be64)((a << 1) | (b >> 63));
+
+ if (a >> 63)
+ ctx->shash.b ^= cpu_to_be64(0xc2);
return 0;
}
--
1.8.3.2
