On 28 February 2015 at 22:30, Milan Broz <gmazyland@xxxxxxxxx> wrote:
> On 02/26/2015 08:22 AM, Ard Biesheuvel wrote:
>> This updates the bit sliced AES module to the latest version in the
>> upstream OpenSSL repository (e620e5ae37bc). This is needed to fix a
>> bug in the XTS decryption path, where data chunked in a certain way
>> could trigger the ciphertext stealing code, which is not supposed to
>> be active in the kernel build (The kernel implementation of XTS only
>> supports round multiples of the AES block size of 16 bytes, whereas
>> the conformant OpenSSL implementation of XTS supports inputs of
>> arbitrary size by applying ciphertext stealing). This is fixed in
>> the upstream version by adding the missing #ifndef XTS_CHAIN_TWEAK
>> around the offending instructions.
>>
>> The upstream code also contains the change applied by Russell to
>> build the code unconditionally, i.e., even if __LINUX_ARM_ARCH__ < 7,
>> but implemented slightly differently.
>>
>> Fixes: e4e7f10bfc40 ("ARM: add support for bit sliced AES using NEON instructions")
>> Reported-by: Adrian Kotelba <adrian.kotelba@xxxxxxxxx>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>> ---
>>
>> This was found using the tcrypt test code, to which I recently added additional
>> chunking modes. However, XTS typically operates on pages or at least on sectors,
>> so this bug is unlikely to affect anyone in real life.
>>
>> Still, please add cc stable when applying,
>
> Please apply it for stable.
>
> For me, this patch fixed bug reported here
> http://article.gmane.org/gmane.linux.kernel.device-mapper.dm-crypt/7966
> http://www.mail-archive.com/linux-crypto@xxxxxxxxxxxxxxx/msg13102.html
>
> Cryptsetup uses AF_ALG userspace crypto SKCIPHER interface and on
> Raspberry Pi2 with Neon AES implementation it fails to unlock LUKS device
> (if XTS and aes-arm-bs module is used).
>
> After applying this patch it works as expected.
>
> I did not have time to check it more in detail (we are always encrypting
> the whole sector so this should not happen... but apparently it does.)
>
> Tested-by: Milan Broz <gmazyland@xxxxxxxxx>
>
OK, thanks for pointing that out, and for confirming that this fixes the issue.
Actually, I did not consider the userspace use case, but it is not
surprising that the data ends up being pushed into the crypto layer in
odd size chunks.
@Herbert: are you ok to take this as a bug fix for stable?
Thanks,
Ard.
>> arch/arm/crypto/aesbs-core.S_shipped | 12 ++++++++----
>> arch/arm/crypto/bsaes-armv7.pl | 12 ++++++++----
>> 2 files changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/crypto/aesbs-core.S_shipped b/arch/arm/crypto/aesbs-core.S_shipped
>> index 71e5fc7cfb18..1d1800f71c5b 100644
>> --- a/arch/arm/crypto/aesbs-core.S_shipped
>> +++ b/arch/arm/crypto/aesbs-core.S_shipped
>> @@ -58,14 +58,18 @@
>> # define VFP_ABI_FRAME 0
>> # define BSAES_ASM_EXTENDED_KEY
>> # define XTS_CHAIN_TWEAK
>> -# define __ARM_ARCH__ 7
>> +# define __ARM_ARCH__ __LINUX_ARM_ARCH__
>> +# define __ARM_MAX_ARCH__ 7
>> #endif
>>
>> #ifdef __thumb__
>> # define adrl adr
>> #endif
>>
>> -#if __ARM_ARCH__>=7
>> +#if __ARM_MAX_ARCH__>=7
>> +.arch armv7-a
>> +.fpu neon
>> +
>> .text
>> .syntax unified @ ARMv7-capable assembler is expected to handle this
>> #ifdef __thumb2__
>> @@ -74,8 +78,6 @@
>> .code 32
>> #endif
>>
>> -.fpu neon
>> -
>> .type _bsaes_decrypt8,%function
>> .align 4
>> _bsaes_decrypt8:
>> @@ -2095,9 +2097,11 @@ bsaes_xts_decrypt:
>> vld1.8 {q8}, [r0] @ initial tweak
>> adr r2, .Lxts_magic
>>
>> +#ifndef XTS_CHAIN_TWEAK
>> tst r9, #0xf @ if not multiple of 16
>> it ne @ Thumb2 thing, sanity check in ARM
>> subne r9, #0x10 @ subtract another 16 bytes
>> +#endif
>> subs r9, #0x80
>>
>> blo .Lxts_dec_short
>> diff --git a/arch/arm/crypto/bsaes-armv7.pl b/arch/arm/crypto/bsaes-armv7.pl
>> index be068db960ee..a4d3856e7d24 100644
>> --- a/arch/arm/crypto/bsaes-armv7.pl
>> +++ b/arch/arm/crypto/bsaes-armv7.pl
>> @@ -701,14 +701,18 @@ $code.=<<___;
>> # define VFP_ABI_FRAME 0
>> # define BSAES_ASM_EXTENDED_KEY
>> # define XTS_CHAIN_TWEAK
>> -# define __ARM_ARCH__ 7
>> +# define __ARM_ARCH__ __LINUX_ARM_ARCH__
>> +# define __ARM_MAX_ARCH__ 7
>> #endif
>>
>> #ifdef __thumb__
>> # define adrl adr
>> #endif
>>
>> -#if __ARM_ARCH__>=7
>> +#if __ARM_MAX_ARCH__>=7
>> +.arch armv7-a
>> +.fpu neon
>> +
>> .text
>> .syntax unified @ ARMv7-capable assembler is expected to handle this
>> #ifdef __thumb2__
>> @@ -717,8 +721,6 @@ $code.=<<___;
>> .code 32
>> #endif
>>
>> -.fpu neon
>> -
>> .type _bsaes_decrypt8,%function
>> .align 4
>> _bsaes_decrypt8:
>> @@ -2076,9 +2078,11 @@ bsaes_xts_decrypt:
>> vld1.8 {@XMM[8]}, [r0] @ initial tweak
>> adr $magic, .Lxts_magic
>>
>> +#ifndef XTS_CHAIN_TWEAK
>> tst $len, #0xf @ if not multiple of 16
>> it ne @ Thumb2 thing, sanity check in ARM
>> subne $len, #0x10 @ subtract another 16 bytes
>> +#endif
>> subs $len, #0x80
>>
>> blo .Lxts_dec_short
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
