Re: [PATCH v2 1/2] s390/zcrypt: Guard domain index uses against speculative bypass

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] s390/zcrypt: speculative bypass of array_index_nospec() in domain checks
--

commit 2f58ddfe66cdbe9c54595e16465810bc53b3d205
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/zcrypt: Guard domain index uses against speculative bypass

The previous array_index_nospec() placement only covered the admin
permission check, leaving ap_test_config_usage_domain() and
ap_test_config_ctrl_domain() in the CCA path exposed to speculative
execution. This commit moves the sanitization to before all permission
and config checks in both the CCA and EP11 paths.

> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
> index ec6a4c2f9f04a..d2ff7ab54c8d5 100644
> --- a/drivers/s390/crypto/zcrypt_api.c
> +++ b/drivers/s390/crypto/zcrypt_api.c
> @@ -877,9 +877,17 @@ static long _zcrypt_send_cprb(u32 xflags, struct ap_perms *perms,
>  	print_hex_dump_debug("ccareq: ", DUMP_PREFIX_ADDRESS, 16, 1,
>  			     ap_msg.msg, ap_msg.len, false);
>  
> +	/* Make sure domain is either AUTOSEL_DOM or in range 0...AP_DOMAIN-1 */
> +	if (domain != AUTOSEL_DOM) {
> +		if (domain >= AP_DOMAINS) {
> +			rc = -EINVAL;
> +			goto out;
> +		}
> +		domain = array_index_nospec(domain, AP_DOMAINS);
> +	}

[Severity: High]
Could placing array_index_nospec() inside this conditional block lead to a
speculative execution bypass?

If the CPU's branch predictor is mistrained to assume domain == AUTOSEL_DOM,
the speculative execution could skip this block entirely. This would leave an
out-of-bounds domain completely unclamped.

> +
>  	if (perms != &ap_perms && domain < AP_DOMAINS) {
>  		if (ap_msg.flags & AP_MSG_FLAG_ADMIN) {
> -			domain = array_index_nospec(domain, AP_DOMAINS);
>  			if (!test_bit_inv(domain, perms->adm)) {

[Severity: High]
Since the nospec clamping could be bypassed speculatively above, could this
block be entered with an out-of-bounds domain?

If the CPU then mispredicts the if (domain < AP_DOMAINS) check as true, does
this result in a speculative out-of-bounds read when test_bit_inv() is called
with the unclamped domain?

>  				rc = -ENODEV;
>  				goto out;

[ ... ]

> @@ -1078,9 +1086,17 @@ static long _zcrypt_send_ep11_cprb(u32 xflags, struct ap_perms *perms,
>  	print_hex_dump_debug("ep11req: ", DUMP_PREFIX_ADDRESS, 16, 1,
>  			     ap_msg.msg, ap_msg.len, false);
>  
> +	/* Make sure domain is either AUTOSEL_DOM or in range 0...AP_DOMAIN-1 */
> +	if (domain != AUTOSEL_DOM) {
> +		if (domain >= AP_DOMAINS) {
> +			rc = -EINVAL;
> +			goto out;
> +		}
> +		domain = array_index_nospec(domain, AP_DOMAINS);
> +	}

[Severity: High]
Does this identical pattern in the EP11 path also expose the domain variable
to the same speculative execution bypass?

> +
>  	if (perms != &ap_perms && domain < AP_DOMAINS) {
>  		if (ap_msg.flags & AP_MSG_FLAG_ADMIN) {
> -			domain = array_index_nospec(domain, AP_DOMAINS);
>  			if (!test_bit_inv(domain, perms->adm)) {

[Severity: High]
Does this result in the same speculative out-of-bounds read during the EP11
admin check, similar to the CCA path above?

>  				rc = -ENODEV;
>  				goto out;

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