Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <51ed3735-24f0-eef0-0ca6-908c4581d143@gmail.com>
Date:   Wed, 9 Nov 2022 22:16:58 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH v3 1/4] crypto: aria: add keystream array into struct
 aria_ctx
To:     Herbert Xu <herbert@gondor.apana.org.au>
Cc:     linux-crypto@vger.kernel.org, davem@davemloft.net,
        tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, hpa@zytor.com,
        kirill.shutemov@linux.intel.com, richard@nod.at,
        viro@zeniv.linux.org.uk,
        sathyanarayanan.kuppuswamy@linux.intel.com, jpoimboe@kernel.org,
        elliott@hpe.com, x86@kernel.org, jussi.kivilinna@iki.fi
References: <20221106143627.30920-1-ap420073@gmail.com>
 <20221106143627.30920-2-ap420073@gmail.com>
 <Y2jGTvgHnu4QZV+D@gondor.apana.org.au>
Content-Language: en-US
From:   Taehee Yoo <ap420073@gmail.com>
In-Reply-To: <Y2jGTvgHnu4QZV+D@gondor.apana.org.au>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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