Re: [PATCH] crypto: aesni-intel - fix unaligned cbc decrypt for x86-32
On Thu, May 31, 2012 at 7:27 AM, Herbert Xu herb...@gondor.apana.org.au wrote: On Wed, May 30, 2012 at 01:43:08AM +0200, Mathias Krause wrote: The 32 bit variant of cbc(aes) decrypt is using instructions requiring 128 bit aligned memory locations but fails to ensure this constraint in the code. Fix this by loading the data into intermediate registers with load unaligned instructions. This fixes reported general protection faults related to aesni. References: https://bugzilla.kernel.org/show_bug.cgi?id=43223 Reported-by: Daniel gark...@mailueberfall.de Cc: sta...@kernel.org [v2.6.39+] Signed-off-by: Mathias Krause mini...@googlemail.com Have measured this against increasing alignmask to 15? No, but the latter will likely be much slower as it would need to memmove the data if it's not aligned, right? My patch essentially just breaks the combined XOR a memory operand with a register operation into two -- load memory into register, then XOR with registers. It shouldn't be much slower compared to the current version. But it fixes a bug the current version exposes when working on unaligned data. That said, I did some micro benchmark on pxor (%edx), %xmm0 vs. movups (%edx), %xmm1; pxor %xmm1, %xmm0 and observed the latter might be even slightly faster! But changing the code to perform better is out of scope for this patch as it should just fix the bug in the code. We can increase performance in a follow up patch. Mathias -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: aesni-intel - fix unaligned cbc decrypt for x86-32
On Thu, May 31, 2012 at 08:45:58AM +0200, Mathias Krause wrote: No, but the latter will likely be much slower as it would need to memmove the data if it's not aligned, right? Most crypto users should already be providing aligned data. After all, padlock-aes requires 16-byte alignment and it works quite well. That said, I did some micro benchmark on pxor (%edx), %xmm0 vs. movups (%edx), %xmm1; pxor %xmm1, %xmm0 and observed the latter might be even slightly faster! But changing the code to perform better is out of scope for this patch as it should just fix the bug in the code. We can increase performance in a follow up patch. OK we can do that. Thanks, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: aesni-intel - fix unaligned cbc decrypt for x86-32
On Wed, May 30, 2012 at 01:43:08AM +0200, Mathias Krause wrote: The 32 bit variant of cbc(aes) decrypt is using instructions requiring 128 bit aligned memory locations but fails to ensure this constraint in the code. Fix this by loading the data into intermediate registers with load unaligned instructions. This fixes reported general protection faults related to aesni. References: https://bugzilla.kernel.org/show_bug.cgi?id=43223 Reported-by: Daniel gark...@mailueberfall.de Cc: sta...@kernel.org [v2.6.39+] Signed-off-by: Mathias Krause mini...@googlemail.com Patch applied to crypto. Thanks. -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: aesni-intel - fix unaligned cbc decrypt for x86-32
On Wed, May 30, 2012 at 01:43:08AM +0200, Mathias Krause wrote: The 32 bit variant of cbc(aes) decrypt is using instructions requiring 128 bit aligned memory locations but fails to ensure this constraint in the code. Fix this by loading the data into intermediate registers with load unaligned instructions. This fixes reported general protection faults related to aesni. References: https://bugzilla.kernel.org/show_bug.cgi?id=43223 Reported-by: Daniel gark...@mailueberfall.de Cc: sta...@kernel.org [v2.6.39+] Signed-off-by: Mathias Krause mini...@googlemail.com Have measured this against increasing alignmask to 15? Thanks, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] crypto: aesni-intel - fix unaligned cbc decrypt for x86-32
The 32 bit variant of cbc(aes) decrypt is using instructions requiring 128 bit aligned memory locations but fails to ensure this constraint in the code. Fix this by loading the data into intermediate registers with load unaligned instructions. This fixes reported general protection faults related to aesni. References: https://bugzilla.kernel.org/show_bug.cgi?id=43223 Reported-by: Daniel gark...@mailueberfall.de Cc: sta...@kernel.org [v2.6.39+] Signed-off-by: Mathias Krause mini...@googlemail.com --- arch/x86/crypto/aesni-intel_asm.S |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/x86/crypto/aesni-intel_asm.S b/arch/x86/crypto/aesni-intel_asm.S index be6d9e3..3470624 100644 --- a/arch/x86/crypto/aesni-intel_asm.S +++ b/arch/x86/crypto/aesni-intel_asm.S @@ -2460,10 +2460,12 @@ ENTRY(aesni_cbc_dec) pxor IN3, STATE4 movaps IN4, IV #else - pxor (INP), STATE2 - pxor 0x10(INP), STATE3 pxor IN1, STATE4 movaps IN2, IV + movups (INP), IN1 + pxor IN1, STATE2 + movups 0x10(INP), IN2 + pxor IN2, STATE3 #endif movups STATE1, (OUTP) movups STATE2, 0x10(OUTP) -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
