Re: [Qemu-devel] [PATCH v5 1/4] target/arm: implement SHA-512 instructions

[Ard Biesheuvel]

On 6 February 2018 at 20:41, Peter Maydell <address@hidden> wrote:
> On 6 February 2018 at 19:15, Peter Maydell <address@hidden> wrote:
>> On 6 February 2018 at 19:06, Peter Maydell <address@hidden> wrote:
>>> SM4EKEY, SM4E
>>
>> Sample SM4EKEY failure:
>> insn 0xce78cbdd (SM4EKEY V29.4S, V30.4S, V24.4S)
>>   V24   : 6ee7a2520059bd15bac75e4436b3a1bd
>>   V30   : a67d04e738f68da895ffd0c3e154e3e7
>>
>>   V29 actual:   a67d04e7b98aaef47bf01b8158da5407
>>   V29 expected: 8d492252b98aaef47bf01b8158da5407
>>
>> (top 32 bits are wrong)
>>
>> Sample SM4E failure:
>> insn 0xcec087dd (SM4E V29.4S, V30.4S)
>>   V30   : a67d04e738f68da895ffd0c3e154e3e7
>>   V29 actual  : e272e88588a781b7e77a90dd5641e34b
>>   V29 expected: a39884af88a781b7e77a90dd5641e34b
>>
>> (top 32 bits again)
>>
>> My test setup doesn't capture register values from
>> before the insn executes, which is awkward for SM4E since
>> it uses Vd as input as well as output. Probably the bug
>> is the same as SM4EKEY, though.
>
> ...and it's a pretty simple fix; we just weren't actually
> doing the 4th iteration of the algorithm:
>
> diff --git a/target/arm/crypto_helper.c b/target/arm/crypto_helper.c
> index b42c7e046b..2c3af64a52 100644
> --- a/target/arm/crypto_helper.c
> +++ b/target/arm/crypto_helper.c
> @@ -653,7 +653,7 @@ void HELPER(crypto_sm4e)(void *vd, void *vn)
>      union CRYPTO_STATE n = { .l = { rn[0], rn[1] } };
>      uint32_t t, i;
>
> -    for (i = 0; i < 3; i++) {
> +    for (i = 0; i < 4; i++) {
>          t = CR_ST_WORD(d, (i + 1) % 4) ^
>              CR_ST_WORD(d, (i + 2) % 4) ^
>              CR_ST_WORD(d, (i + 3) % 4) ^
> @@ -683,7 +683,7 @@ void HELPER(crypto_sm4ekey)(void *vd, void *vn, void* vm)
>      uint32_t t, i;
>
>      d = n;
> -    for (i = 0; i < 3; i++) {
> +    for (i = 0; i < 4; i++) {
>          t = CR_ST_WORD(d, (i + 1) % 4) ^
>              CR_ST_WORD(d, (i + 2) % 4) ^
>              CR_ST_WORD(d, (i + 3) % 4) ^
>
> That change is sufficient for SM4E and SM4EKEY to pass my tests.
>

Thanks a lot for debugging that. As I said, I don't have test vectors,
or I would have tested it myself, and most likely would have found
this as well.