Re: [PATCH v5 3/8] s390/crypto: Fix use of mutex in atomic context in PAES

[email protected]
Newsgroups org.kernel.vger.linux-s390
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch correctly replaces an unsafe `mutex_trylock()` with a binary semaphore in the `paes_s390` PAES CTR implementation, because mutexes cannot be safely locked in atomic or interrupt context. However, it fails to apply the exact same fix to the closely related standard AES CTR implementation in `aes_s390.c`. The function `ctr_aes_crypt()` still uses `mutex_trylock(&ctrblk_lock)` on its fast path, which is unsafe when called from softirq/atomic context.
--

--- Patch [3]: [PATCH v5 3/8] s390/crypto: Fix use of mutex in atomic context in PAES ---
commit a2afbae94320906011d9357378185a56c441bdd2
Author: Harald Freudenberger <[email protected]>

s390/crypto: Fix use of mutex in atomic context in PAES

The PAES CTR implementation used a mutex to lock one page of exclusive
memory for fast CTR processing. Unfortunately a mutex is not save to
use in atomic or interrupt context. So use a binary semaphore instead
which is save to use in such environments.

> diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
> index d075b0241f1f5..e8bbfc376ebb1 100644
> --- a/arch/s390/crypto/paes_s390.c
> +++ b/arch/s390/crypto/paes_s390.c
[ ... ]
> @@ -971,7 +972,7 @@ static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
>  	if (rc)
>  		goto out;
>  
> -	locked = mutex_trylock(&ctrblk_lock);
> +	locked = down_trylock(&ctrblk_sem) == 0;

[Severity: High]
This is a pre-existing issue, but I noticed the standard AES CTR
implementation in arch/s390/crypto/aes_s390.c seems to suffer from the
exact same problem.

Looking at ctr_aes_crypt() in aes_s390.c, it still uses mutex_trylock()
on its fast path:

arch/s390/crypto/aes_s390.c:ctr_aes_crypt() {
    ...
	if (unlikely(!sctx->fc))
		return fallback_skcipher_crypt(sctx, req, 0);

	locked = mutex_trylock(&ctrblk_lock);

	ret = skcipher_walk_virt(&walk, req, false);
    ...
}

Since mutexes cannot be safely locked in atomic or interrupt context, and
network IPsec/MACsec could execute packet decryption in softirq context,
does the standard AES implementation need this identical fix as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.