Re: [PATCH v4 1/3] s390/crypto: Fix skcipher_walk return code handling in aes_s390

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Using a mutex (`ctrblk_lock`) in `ctr_aes_crypt`, which can be executed in interrupt/softirq context.
--

--- Patch [1]: [PATCH v4 1/3] s390/crypto: Fix skcipher_walk return code handling in aes_s390 ---
commit 17298a26fe3f9b4cf4ce956d4ad5d1b5f20ca7ef
Author: Harald Freudenberger <[email protected]>
s390/crypto: Fix skcipher_walk return code handling in aes_s390

This patch adds proper return code checking from skcipher_walk_virt() in
the processing loops of several symmetric cipher modes to prevent operating
on an uninitialized walk structure if the initialization fails.

> diff --git a/arch/s390/crypto/aes_s390.c b/arch/s390/crypto/aes_s390.c
> index 62edc66d54788..366ce22d36237 100644
> --- a/arch/s390/crypto/aes_s390.c
> +++ b/arch/s390/crypto/aes_s390.c

[ ... ]

> @@ -577,7 +577,7 @@ static int ctr_aes_crypt(struct skcipher_request *req)
>  	locked = mutex_trylock(&ctrblk_lock);

[Severity: High]
This isn't a bug introduced by this patch, but can using a mutex here
corrupt the interrupted task's lock state?

Since ctr_aes_crypt() can be executed in softirq context (such as during
IPsec ESP receive processing via NET_RX_SOFTIRQ), calling mutex_trylock()
and later mutex_unlock() assigns ownership to the arbitrarily interrupted
task. Mutexes enforce strict task ownership and are explicitly forbidden
in interrupt context, even when using trylock.

Could this trigger lockdep splats, incorrect optimistic spinning behavior,
or hard panics on PREEMPT_RT kernels where mutexes are strictly task-bound?

>  
>  	ret = skcipher_walk_virt(&walk, req, false);
> -	while ((nbytes = walk.nbytes) >= AES_BLOCK_SIZE) {
> +	while (!ret && ((nbytes = walk.nbytes) >= AES_BLOCK_SIZE)) {
>  		n = AES_BLOCK_SIZE;
>  
>  		if (nbytes >= 2*AES_BLOCK_SIZE && locked)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1