From patchwork Thu Dec 11 08:52:40 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Yonan X-Patchwork-Id: 5474791 X-Patchwork-Delegate: herbert@gondor.apana.org.au Return-Path: X-Original-To: patchwork-linux-crypto@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id C6F66BEEA8 for ; Thu, 11 Dec 2014 08:57:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id ABD9520172 for ; Thu, 11 Dec 2014 08:57:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5338120121 for ; Thu, 11 Dec 2014 08:57:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934157AbaLKI54 (ORCPT ); Thu, 11 Dec 2014 03:57:56 -0500 Received: from mail.yonan.net ([54.244.116.145]:49258 "EHLO mail.yonan.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932634AbaLKI5z (ORCPT ); Thu, 11 Dec 2014 03:57:55 -0500 X-Greylist: delayed 313 seconds by postgrey-1.27 at vger.kernel.org; Thu, 11 Dec 2014 03:57:55 EST Received: from Moab.local (c-67-166-32-18.hsd1.co.comcast.net [67.166.32.18]) (using TLSv1 with cipher ECDHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mail.yonan.net (Postfix) with ESMTPSA id 7CFDBC0F31; Thu, 11 Dec 2014 01:52:41 -0700 (MST) Message-ID: <54895B58.8030200@openvpn.net> Date: Thu, 11 Dec 2014 01:52:40 -0700 From: James Yonan Organization: OpenVPN Technologies, Inc. User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Mathias Krause , Herbert Xu , "David S. Miller" CC: Romain Francoise , linux-crypto@vger.kernel.org, Chandramouli Narayanan Subject: Re: [PATCH] crypto: aesni - disable "by8" AVX CTR optimization References: <20140917112911.GA2129@gondor.apana.org.au> <1411504267-10170-1-git-send-email-minipli@googlemail.com> In-Reply-To: <1411504267-10170-1-git-send-email-minipli@googlemail.com> Sender: linux-crypto-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 > Signed-off-by: Mathias Krause > Cc: Chandramouli Narayanan > > --- > 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 #include #include +#include #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;