* [PATCH] crypto: aesni - fix "by8" variant for 128 bit keys
@ 2014-12-30 21:50 Mathias Krause
2015-01-01 17:08 ` James Yonan
0 siblings, 1 reply; 3+ messages in thread
From: Mathias Krause @ 2014-12-30 21:50 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, James Yonan
Cc: linux-crypto, Mathias Krause, Romain Francoise,
Chandramouli Narayanan
The "by8" counter mode optimization is broken for 128 bit keys with
input data longer than 128 bytes. It uses the wrong key material for
en- and decryption.
The key registers xkey0, xkey4, xkey8 and xkey12 need to be preserved
in case we're handling more than 128 bytes of input data -- they won't
get reloaded after the initial load. They must therefore be (a) loaded
on the first iteration and (b) be preserved for the latter ones. The
implementation for 128 bit keys does not comply with (a) nor (b).
Fix this by bringing the implementation back to its original source
and correctly load the key registers and preserve their values by
*not* re-using the registers for other purposes.
Kudos to James for reporting the issue and providing a test case
showing the discrepancies.
Reported-by: James Yonan <james@openvpn.net>
Cc: Chandramouli Narayanan <mouli@linux.intel.com>
Cc: <stable@vger.kernel.org> # v3.18
Signed-off-by: Mathias Krause <minipli@googlemail.com>
---
James, this should fix the issue for you, too -- it did for me, using
your test case. Feel free to add your 'Tested-by' if it does.
This patch should go on top of crypto-2.6.git#master as it's a fix
that needs to go to stable as well.
arch/x86/crypto/aes_ctrby8_avx-x86_64.S | 46 +++++++++++++++++++++++--------
1 file changed, 35 insertions(+), 11 deletions(-)
diff --git a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
index 2df2a0298f5a..a916c4a61165 100644
--- a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
+++ b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
@@ -208,7 +208,7 @@ ddq_add_8:
.if (klen == KEY_128)
.if (load_keys)
- vmovdqa 3*16(p_keys), xkeyA
+ vmovdqa 3*16(p_keys), xkey4
.endif
.else
vmovdqa 3*16(p_keys), xkeyA
@@ -224,7 +224,7 @@ ddq_add_8:
add $(16*by), p_in
.if (klen == KEY_128)
- vmovdqa 4*16(p_keys), xkey4
+ vmovdqa 4*16(p_keys), xkeyB
.else
.if (load_keys)
vmovdqa 4*16(p_keys), xkey4
@@ -234,7 +234,12 @@ ddq_add_8:
.set i, 0
.rept by
club XDATA, i
- vaesenc xkeyA, var_xdata, var_xdata /* key 3 */
+ /* key 3 */
+ .if (klen == KEY_128)
+ vaesenc xkey4, var_xdata, var_xdata
+ .else
+ vaesenc xkeyA, var_xdata, var_xdata
+ .endif
.set i, (i +1)
.endr
@@ -243,13 +248,18 @@ ddq_add_8:
.set i, 0
.rept by
club XDATA, i
- vaesenc xkey4, var_xdata, var_xdata /* key 4 */
+ /* key 4 */
+ .if (klen == KEY_128)
+ vaesenc xkeyB, var_xdata, var_xdata
+ .else
+ vaesenc xkey4, var_xdata, var_xdata
+ .endif
.set i, (i +1)
.endr
.if (klen == KEY_128)
.if (load_keys)
- vmovdqa 6*16(p_keys), xkeyB
+ vmovdqa 6*16(p_keys), xkey8
.endif
.else
vmovdqa 6*16(p_keys), xkeyB
@@ -267,12 +277,17 @@ ddq_add_8:
.set i, 0
.rept by
club XDATA, i
- vaesenc xkeyB, var_xdata, var_xdata /* key 6 */
+ /* key 6 */
+ .if (klen == KEY_128)
+ vaesenc xkey8, var_xdata, var_xdata
+ .else
+ vaesenc xkeyB, var_xdata, var_xdata
+ .endif
.set i, (i +1)
.endr
.if (klen == KEY_128)
- vmovdqa 8*16(p_keys), xkey8
+ vmovdqa 8*16(p_keys), xkeyB
.else
.if (load_keys)
vmovdqa 8*16(p_keys), xkey8
@@ -288,7 +303,7 @@ ddq_add_8:
.if (klen == KEY_128)
.if (load_keys)
- vmovdqa 9*16(p_keys), xkeyA
+ vmovdqa 9*16(p_keys), xkey12
.endif
.else
vmovdqa 9*16(p_keys), xkeyA
@@ -297,7 +312,12 @@ ddq_add_8:
.set i, 0
.rept by
club XDATA, i
- vaesenc xkey8, var_xdata, var_xdata /* key 8 */
+ /* key 8 */
+ .if (klen == KEY_128)
+ vaesenc xkeyB, var_xdata, var_xdata
+ .else
+ vaesenc xkey8, var_xdata, var_xdata
+ .endif
.set i, (i +1)
.endr
@@ -306,7 +326,12 @@ ddq_add_8:
.set i, 0
.rept by
club XDATA, i
- vaesenc xkeyA, var_xdata, var_xdata /* key 9 */
+ /* key 9 */
+ .if (klen == KEY_128)
+ vaesenc xkey12, var_xdata, var_xdata
+ .else
+ vaesenc xkeyA, var_xdata, var_xdata
+ .endif
.set i, (i +1)
.endr
@@ -412,7 +437,6 @@ ddq_add_8:
/* main body of aes ctr load */
.macro do_aes_ctrmain key_len
-
cmp $16, num_bytes
jb .Ldo_return2\key_len
--
1.7.10.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] crypto: aesni - fix "by8" variant for 128 bit keys
2014-12-30 21:50 [PATCH] crypto: aesni - fix "by8" variant for 128 bit keys Mathias Krause
@ 2015-01-01 17:08 ` James Yonan
2015-01-05 10:36 ` Herbert Xu
0 siblings, 1 reply; 3+ messages in thread
From: James Yonan @ 2015-01-01 17:08 UTC (permalink / raw)
To: Mathias Krause, Herbert Xu, David S. Miller
Cc: linux-crypto, Romain Francoise, Chandramouli Narayanan
On 30/12/2014 14:50, Mathias Krause wrote:
> The "by8" counter mode optimization is broken for 128 bit keys with
> input data longer than 128 bytes. It uses the wrong key material for
> en- and decryption.
>
> The key registers xkey0, xkey4, xkey8 and xkey12 need to be preserved
> in case we're handling more than 128 bytes of input data -- they won't
> get reloaded after the initial load. They must therefore be (a) loaded
> on the first iteration and (b) be preserved for the latter ones. The
> implementation for 128 bit keys does not comply with (a) nor (b).
>
> Fix this by bringing the implementation back to its original source
> and correctly load the key registers and preserve their values by
> *not* re-using the registers for other purposes.
>
> Kudos to James for reporting the issue and providing a test case
> showing the discrepancies.
>
> Reported-by: James Yonan <james@openvpn.net>
> Cc: Chandramouli Narayanan <mouli@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v3.18
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
This looks great, fixes the issue on 3.18.1 for all of our use cases.
Thanks to Mathias for putting this together.
James
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] crypto: aesni - fix "by8" variant for 128 bit keys
2015-01-01 17:08 ` James Yonan
@ 2015-01-05 10:36 ` Herbert Xu
0 siblings, 0 replies; 3+ messages in thread
From: Herbert Xu @ 2015-01-05 10:36 UTC (permalink / raw)
To: James Yonan
Cc: Mathias Krause, David S. Miller, linux-crypto, Romain Francoise,
Chandramouli Narayanan
On Thu, Jan 01, 2015 at 10:08:18AM -0700, James Yonan wrote:
> On 30/12/2014 14:50, Mathias Krause wrote:
> >The "by8" counter mode optimization is broken for 128 bit keys with
> >input data longer than 128 bytes. It uses the wrong key material for
> >en- and decryption.
> >
> >The key registers xkey0, xkey4, xkey8 and xkey12 need to be preserved
> >in case we're handling more than 128 bytes of input data -- they won't
> >get reloaded after the initial load. They must therefore be (a) loaded
> >on the first iteration and (b) be preserved for the latter ones. The
> >implementation for 128 bit keys does not comply with (a) nor (b).
> >
> >Fix this by bringing the implementation back to its original source
> >and correctly load the key registers and preserve their values by
> >*not* re-using the registers for other purposes.
> >
> >Kudos to James for reporting the issue and providing a test case
> >showing the discrepancies.
> >
> >Reported-by: James Yonan <james@openvpn.net>
> >Cc: Chandramouli Narayanan <mouli@linux.intel.com>
> >Cc: <stable@vger.kernel.org> # v3.18
> >Signed-off-by: Mathias Krause <minipli@googlemail.com>
>
> This looks great, fixes the issue on 3.18.1 for all of our use cases.
>
> Thanks to Mathias for putting this together.
Patch applied to crypto. Thanks a lot!
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-01-05 10:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-30 21:50 [PATCH] crypto: aesni - fix "by8" variant for 128 bit keys Mathias Krause
2015-01-01 17:08 ` James Yonan
2015-01-05 10:36 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).