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 <[email protected]>
Cc: [email protected] [v2.6.39+]
Signed-off-by: Mathias Krause <[email protected]>
---
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
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 <[email protected]>
> Cc: [email protected] [v2.6.39+]
> Signed-off-by: Mathias Krause <[email protected]>
Have measured this against increasing alignmask to 15?
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, May 31, 2012 at 7:27 AM, Herbert Xu <[email protected]> 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 <[email protected]>
>> Cc: [email protected] [v2.6.39+]
>> Signed-off-by: Mathias Krause <[email protected]>
>
> 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
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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
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 <[email protected]>
> Cc: [email protected] [v2.6.39+]
> Signed-off-by: Mathias Krause <[email protected]>
Patch applied to crypto. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt