On 6 February 2018 at 20:41, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 6 February 2018 at 19:15, Peter Maydell <peter.mayd...@linaro.org> wrote:
>> On 6 February 2018 at 19:06, Peter Maydell <peter.mayd...@linaro.org> 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.
