Hi Herbert,
On 11/7/22 17:48, Herbert Xu wrote:
> On Sun, Nov 06, 2022 at 02:36:24PM +0000, Taehee Yoo wrote:
>>
>> struct aria_ctx {
>> u32 enc_key[ARIA_MAX_RD_KEYS][ARIA_RD_KEY_WORDS];
>> u32 dec_key[ARIA_MAX_RD_KEYS][ARIA_RD_KEY_WORDS];
>> int rounds;
>> int key_length;
>> +#if defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64) || \
>> + defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64_MODULE)
>> + u8 keystream[ARIA_KEYSTREAM_SIZE];
>> +#endif
>> };
>
> The tfm ctx is shared between all users of the tfm. You need
> something that is private to the request so this needs to be
> moved into the reqctx.
>
> Cheers,
I have encountered kernel panic(stack-out-of-bounds) while using the
reqctx instead of the tfm.
cryptd is used when simd drivers are used.
cryptd_skcipher_encrypt() internally doesn't allocate a request ctx of a
child, instead, it uses stack memory with SYNC_SKCIPHER_REQUEST_ON_STACK.
It retains only 384 bytes for child request ctx even if a child set a
large reqsize value with crypto_skcipher_set_reqsize().
aria-avx2 needs 512 bytes and aria-avx512 needs 1024 bytes.
So, stack-out-of-bounds occurs.
What do you think about using the per-cpu for the keystream array?
SIMD functions are called under fpu context.
So, per-cpu keystream will not be used concurrently, it's safe.
It also can avoid unnecessary allocation for large temporary memory.
I tested the per-cpu for keystream, and it works well.
If you are okay, I would like to submit v4 patch using per-cpu.
Thanks,
Taehee Yoo
