Re: [PATCH v2 1/1] s390/zcrypt: Close speculative mem read possibility

[sashiko-bot@kernel.org]

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

New issues:
- [High] In `_zcrypt_send_ep11_cprb`, `array_index_nospec()` is used without a prior bounds check against `AP_DOMAINS`, silently aliasing out-of-bounds domains to domain 0 and bypassing administrative permission checks.

Pre-existing issues:
- [High] Administrative CPRBs with domain == AUTOSEL_DOM completely bypass the perms->adm permission check in `_zcrypt_send_cprb` and `_zcrypt_send_ep11_cprb`.
--

--- Patch [1]: [PATCH v2 1/1] s390/zcrypt: Close speculative mem read possibility ---
commit de8272723358b7baff06b29b44fe219b26350d5e
Author: Harald Freudenberger <freude@linux.ibm.com>
s390/zcrypt: Close speculative mem read possibility

This patch addresses a speculative execution out-of-bounds read by adding
array_index_nospec() to the domain value extracted from user-controlled
CPRB structs before it is used as an array index.

> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
> index f57189c2b839e..593aace5fc484 100644
> --- a/drivers/s390/crypto/zcrypt_api.c
> +++ b/drivers/s390/crypto/zcrypt_api.c
> @@ -879,6 +879,7 @@ static long _zcrypt_send_cprb(u32 xflags, struct ap_perms *perms,
>  
>  	if (perms != &ap_perms && domain < AP_DOMAINS) {

[Severity: High]
This is a pre-existing issue, but does this logic allow administrative
CPRBs with domain == AUTOSEL_DOM to bypass the perms->adm permission
check entirely?

If a user submits a CPRB request with an administrative command code and sets
the target domain to AUTOSEL_DOM, this condition evaluates to false.

The perms->adm evaluation is skipped, and the kernel later selects a queue
based solely on the user's usage permissions in zcrypt_check_queue().

>  		if (ap_msg.flags & AP_MSG_FLAG_ADMIN) {
> +			domain = array_index_nospec(domain, AP_DOMAINS);
>  			if (!test_bit_inv(domain, perms->adm)) {
>  				rc = -ENODEV;
>  				goto out;

[ ... ]

> @@ -1079,6 +1080,7 @@ static long _zcrypt_send_ep11_cprb(u32 xflags, struct ap_perms *perms,
>  
>  	if (perms != &ap_perms && domain < AUTOSEL_DOM) {

[Severity: High]
This is a pre-existing issue, but does this similarly skip the admin check
for EP11 CPRBs if domain == AUTOSEL_DOM?

>  		if (ap_msg.flags & AP_MSG_FLAG_ADMIN) {
> +			domain = array_index_nospec(domain, AP_DOMAINS);

[Severity: High]
Does this use of array_index_nospec() map out-of-bounds domains to domain 0?

The guard condition for this block in _zcrypt_send_ep11_cprb() checks
domain < AUTOSEL_DOM, but array_index_nospec() bounds against AP_DOMAINS.

If a user provides a domain value between AP_DOMAINS and AUTOSEL_DOM - 1, the
array_index_nospec() call will forcefully mask the out-of-bounds domain to 0. 

Will this result in checking perms->adm for domain 0 instead of the requested
domain, potentially allowing an authorization bypass if the user has admin
privileges for domain 0?

>  			if (!test_bit_inv(domain, perms->adm)) {
>  				rc = -ENODEV;
>  				goto out;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722152025.119391-2-freude@linux.ibm.com?part=1