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

On 2026-08-14 14:47, [email protected] wrote:
> 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)

Exactly this is addressed in patch #3 of this series.