Re: [PATCH v4 2/7] s390/crypto: Fix missing scrub of temp buffers with PAES algorithm

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

Pre-existing issues:
- [High] Incomplete fix: stack buffers containing sensitive user plaintext/ciphertext are left unscrubbed in standard AES functions, identical to the pattern fixed in the PAES functions.
--

--- Patch [2]: [PATCH v4 2/7] s390/crypto: Fix missing scrub of temp buffers with PAES algorithm ---
commit 4d897eecbd2aa1ba90d2105c4cb807323e706e6b
Author: Harald Freudenberger <[email protected]>
s390/crypto: Fix missing scrub of temp buffers with PAES algorithm

This commit explicitly scrubs a stack buffer used to process remaining bytes
in ctr_paes_do_crypt(), and broadens a scrub in __xts_2keys_prep_param() to
clear the entire parameter block. This prevents potentially sensitive data
from being left exposed.

Cc: [email protected] # 6.16+

> diff --git a/arch/s390/crypto/paes_s390.c b/arch/s390/crypto/paes_s390.c
> index ccc7da106c3a0..d075b0241f1f5 100644
> --- a/arch/s390/crypto/paes_s390.c
> +++ b/arch/s390/crypto/paes_s390.c
> @@ -1044,6 +1044,7 @@ static int ctr_paes_do_crypt(struct s390_paes_ctx *ctx,
>  	}
>  
>  out:
> +	memzero_explicit(buf, sizeof(buf));

[Severity: High]
This isn't a bug introduced by this patch, but does this missing scrub pattern
also need to be fixed in the standard AES functions?

In arch/s390/crypto/aes_s390.c:ctr_aes_crypt(), the buf stack array
is used to process the final block data, but it doesn't appear to be
explicitly zeroed before the function returns:

arch/s390/crypto/aes_s390.c:ctr_aes_crypt() {
    ...
        ret = skcipher_walk_done(&walk, 0);
    }

    return ret;
}

Similarly, in arch/s390/crypto/aes_s390.c:gcm_aes_crypt(), the gw_in
and gw_out structs contain local buf arrays that store block data
and seem to remain on the stack without being scrubbed:

arch/s390/crypto/aes_s390.c:gcm_aes_crypt() {
    ...
    memzero_explicit(&param, sizeof(param));
    return ret;
}

Could cryptographic operations using ctr(aes) or gcm(aes) with data lengths
not aligned to block boundaries leave unscrubbed plaintext or ciphertext on
the stack, similar to the vulnerability fixed here for PAES?

>  	pr_debug("rc=%d\n", rc);
>  	return rc;
>  }

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