I'm seeing some anomalous results with the "by8" AVX CTR optimization in
3.18.
In particular, crypto_aead_encrypt appears to produce different
ciphertext from the same plaintext depending on whether or not the
optimization is enabled.
See the attached patch to tcrypt that demonstrates the discrepancy.
James
On 23/09/2014 14:31, Mathias Krause wrote:
The "by8" implementation introduced in commit 22cddcc7df8f ("crypto: aes
- AES CTR x86_64 "by8" AVX optimization") is failing crypto tests as it
handles counter block overflows differently. It only accounts the right
most 32 bit as a counter -- not the whole block as all other
implementations do. This makes it fail the cryptomgr test #4 that
specifically tests this corner case.
As we're quite late in the release cycle, just disable the "by8" variant
for now.
Reported-by: Romain Francoise <romain@xxxxxxxxxxxxx>
Signed-off-by: Mathias Krause <minipli@xxxxxxxxxxxxxx>
Cc: Chandramouli Narayanan <mouli@xxxxxxxxxxxxxxx>
---
I'll try to create a real fix next week but I can't promise much. If
Linus releases v3.17 early, as he has mentioned in his -rc6
announcement, we should hot fix this by just disabling the "by8"
variant. The real fix would add the necessary counter block handling to
the asm code and revert this commit afterwards. Reverting the whole code
is not necessary, imho.
Would that be okay for you, Herbert?
---
arch/x86/crypto/aesni-intel_glue.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c
index 888950f29fd9..a7ccd57f19e4 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -481,7 +481,7 @@ static void ctr_crypt_final(struct crypto_aes_ctx *ctx,
crypto_inc(ctrblk, AES_BLOCK_SIZE);
}
-#ifdef CONFIG_AS_AVX
+#if 0 /* temporary disabled due to failing crypto tests */
static void aesni_ctr_enc_avx_tfm(struct crypto_aes_ctx *ctx, u8 *out,
const u8 *in, unsigned int len, u8 *iv)
{
@@ -1522,7 +1522,7 @@ static int __init aesni_init(void)
aesni_gcm_dec_tfm = aesni_gcm_dec;
}
aesni_ctr_enc_tfm = aesni_ctr_enc;
-#ifdef CONFIG_AS_AVX
+#if 0 /* temporary disabled due to failing crypto tests */
if (cpu_has_avx) {
/* optimize performance of ctr mode encryption transform */
aesni_ctr_enc_tfm = aesni_ctr_enc_avx_tfm;
This is a standard 3.18 kernel with "by8" AVX CTR optimization in place.
Processor is Intel(R) Xeon(R) CPU E3-1220 V2 @ 3.10GHz.
Run tcrypt mode=600 using attached patch to tcrypt. The input plaintext
is 128 bytes of 0xFF.
[ 6.859579] test_aead_encrypt_consistency alg=gcm(aes) succeeded, hash=0x52fc2dd3
[ 6.860682] Key:
[ 6.860914] 00000000: 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55
[ 6.861671] Initial IV:
[ 6.861961] 00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 6.862725] Final IV:
[ 6.863000] 00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0a
[ 6.863827] AD:
[ 6.864077] 00000000: ad ad ad ad ad ad ad ad ad ad ad ad ad ad ad ad
[ 6.864831] Ciphertext:
[ 6.865116] 00000000: 9d 49 0e af 65 17 a3 2a 1f ef 05 27 d9 af 5e 6e
[ 6.865871] 00000010: 9d 2b fc fa be 66 14 35 f4 b5 82 9d ee c2 be a8
[ 6.866695] 00000020: 6e 8f af e0 f5 26 79 f9 6f ed 91 15 c3 26 30 06
[ 6.867463] 00000030: b3 b1 cc 70 0a b7 73 6e f3 8c 96 f0 26 ab 13 ca
[ 6.868268] 00000040: a9 4a 5f e6 1f a8 fa e5 71 f7 a6 5b 73 93 40 94
[ 6.869040] 00000050: f1 82 5e 08 5c 85 02 02 8c 6f 4b 93 f8 10 1a f1
[ 6.869810] 00000060: c9 5e 23 0c bc ad 0f 33 6a e7 da f3 71 b7 be 12
[ 6.870575] 00000070: b1 a0 83 94 60 8d 70 ca 43 ff d0 e9 61 17 56 6e
[ 6.871386] Auth Tag:
[ 6.871659] 00000000: aa fe 4e ce 3b 12 59 1d 06 93 fb 37 26 1a bb bd
Next, remove the optimization code: "rmmod aesni_intel".
Run tcrypt again and the results are different:
[ 7.173145] test_aead_encrypt_consistency alg=gcm(aes) succeeded, hash=0xad4487f8
[ 7.174026] Key:
[ 7.174252] 00000000: 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55 55
[ 7.175068] Initial IV:
[ 7.175380] 00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 7.176237] Final IV:
[ 7.176520] 00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0a
[ 7.177360] AD:
[ 7.177586] 00000000: ad ad ad ad ad ad ad ad ad ad ad ad ad ad ad ad
[ 7.178405] Ciphertext:
[ 7.178721] 00000000: 16 3d ce aa 94 c5 41 ef 4f f8 38 98 b3 ec 0b 68
[ 7.179526] 00000010: 29 c1 b6 12 10 46 3a f8 77 22 d4 df da fd 95 fc
[ 7.180396] 00000020: 3a 15 b5 e3 01 e6 d9 9f ea 26 ae ed 98 63 6e 62
[ 7.181189] 00000030: 0c ca dc 5b 65 98 f5 29 f5 e4 d8 3a 2e ea 6c 39
[ 7.181984] 00000040: 7d df 67 66 ce 69 6d 74 f4 e0 e3 df ff 93 1a 9a
[ 7.182768] 00000050: 5a a0 cb af 7b dd e9 bb dd 6a df a5 57 b9 1d 56
[ 7.183604] 00000060: f6 21 cf 45 7d 82 bb ec a4 59 42 4b 8a 46 34 1d
[ 7.184613] 00000070: 18 85 77 09 b7 81 4f e2 92 fc 84 95 6e 3f 94 75
[ 7.185407] Auth Tag:
[ 7.185695] 00000000: de 7a 34 56 34 5a 02 19 2a 97 e7 bb 70 d0 20 89
Which version is correct? The latter version
(without the optimization) matches the results
produced by previous kernel versions and matches
up with PolarSSL as well.
diff --git a/tcrypt.c b/tcrypt.c
index 890449e..9dd7bc9 100644
--- a/tcrypt.c
+++ b/tcrypt.c
@@ -33,6 +33,7 @@
#include <linux/jiffies.h>
#include <linux/timex.h>
#include <linux/interrupt.h>
+#include <linux/jhash.h>
#include "tcrypt.h"
#include "internal.h"
@@ -265,6 +266,109 @@ static void sg_init_aead(struct scatterlist *sg, char *xbuf[XBUFSIZE],
}
}
+static void test_aead_encrypt_consistency(const char *algo, unsigned int secs, unsigned int keylen)
+{
+ const unsigned int data_size = 128;
+ const unsigned int ad_size = 16;
+ const unsigned int auth_tag_size = 16;
+ const unsigned int data_frags = 1;
+
+ const unsigned int data_sg_base = 1;
+ const unsigned int nfrags = data_frags + 2;
+
+ unsigned char *data = NULL;
+ struct scatterlist *sg = NULL;
+ struct crypto_aead *tfm = NULL;
+ struct aead_request *req = NULL;
+ unsigned char key[keylen];
+ unsigned char auth_tag[auth_tag_size];
+ unsigned char ad[ad_size];
+ unsigned char orig_iv[16];
+ unsigned char iv[16];
+ unsigned int hash;
+ int err;
+
+ data = kmalloc(data_size, GFP_KERNEL);
+ sg = kzalloc(sizeof(struct scatterlist) * nfrags, GFP_KERNEL);
+ if (!data || !sg) {
+ printk("kmalloc failed\n");
+ goto done;
+ }
+ sg_init_table(sg, nfrags);
+ sg_set_buf(sg, ad, ad_size);
+ sg_set_buf(&sg[data_frags + 1], auth_tag, auth_tag_size);
+
+ sg_set_buf(&sg[data_sg_base+0], data, data_size);
+
+ tfm = crypto_alloc_aead(algo, 0, 0);
+ if (IS_ERR(tfm)) {
+ printk("crypto_alloc_aead failed, err=%ld\n", PTR_ERR(tfm));
+ tfm = NULL;
+ goto done;
+ }
+
+ memset(key, 0x55, keylen);
+ err = crypto_aead_setkey(tfm, key, keylen);
+ if (err < 0) {
+ printk("crypto_aead_setkey failed, err=%d\n", err);
+ goto done;
+ }
+
+ err = crypto_aead_setauthsize(tfm, auth_tag_size);
+ if (err < 0) {
+ printk("crypto_aead_setauthsize failed, err=%d\n", err);
+ goto done;
+ }
+
+ req = aead_request_alloc(tfm, GFP_KERNEL);
+ if (!req) {
+ printk("aead_request_alloc failed\n");
+ goto done;
+ }
+
+ memset(ad, 0xAD, ad_size);
+ memset(data, 0xFF, data_size);
+ memset(iv, 0, sizeof(iv));
+ memcpy(orig_iv, iv, sizeof(orig_iv));
+ memset(auth_tag, 0, auth_tag_size);
+
+ /* encrypt */
+ aead_request_set_crypt(req, &sg[data_sg_base], &sg[data_sg_base],
+ data_size,
+ iv);
+ aead_request_set_assoc(req, sg, ad_size);
+ err = crypto_aead_encrypt(req);
+ if (err < 0) {
+ printk("crypto_aead_encrypt failed err=%d\n", err);
+ goto done;
+ }
+
+ /* hash the ciphertext */
+ hash = jhash(data, data_size, 0);
+
+ printk("test_aead_encrypt_consistency alg=%s succeeded, hash=0x%x\n", algo, hash);
+ printk("Key:\n");
+ print_hex_dump(KERN_INFO, " ", DUMP_PREFIX_OFFSET, 16, 1, key, keylen, 0);
+ printk("Initial IV:\n");
+ print_hex_dump(KERN_INFO, " ", DUMP_PREFIX_OFFSET, 16, 1, orig_iv, sizeof(orig_iv), 0);
+ printk("Final IV:\n");
+ print_hex_dump(KERN_INFO, " ", DUMP_PREFIX_OFFSET, 16, 1, iv, sizeof(iv), 0);
+ printk("AD:\n");
+ print_hex_dump(KERN_INFO, " ", DUMP_PREFIX_OFFSET, 16, 1, ad, ad_size, 0);
+ printk("Ciphertext:\n");
+ print_hex_dump(KERN_INFO, " ", DUMP_PREFIX_OFFSET, 16, 1, data, data_size, 0);
+ printk("Auth Tag:\n");
+ print_hex_dump(KERN_INFO, " ", DUMP_PREFIX_OFFSET, 16, 1, auth_tag, auth_tag_size, 0);
+
+done:
+ if (req)
+ aead_request_free(req);
+ if (tfm)
+ crypto_free_aead(tfm);
+ kfree(data);
+ kfree(sg);
+}
+
static void test_aead_speed(const char *algo, int enc, unsigned int secs,
struct aead_speed_template *template,
unsigned int tcount, u8 authsize,
@@ -2119,6 +2223,10 @@ static int do_test(int m)
speed_template_8_32);
break;
+ case 600:
+ test_aead_encrypt_consistency("gcm(aes)", sec, 16);
+ break;
+
case 1000:
test_available();
break;
