Re: [PATCH v3 2/2] 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:

Pre-existing issues:
- [High] Authorization bypass for administrative CPRB requests via domain auto-selection.
- [Medium] Incomplete Spectre v1 mitigation for the user-controlled domain value.
--

--- Patch [2]: [PATCH v3 2/2] s390/zcrypt: Close speculative mem read possibility ---
commit 231c807d810ac3d48184d28be474dabc8e020aa9
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/zcrypt: Close speculative mem read possibility

This commit attempts to fix a speculative execution vulnerability in the
s390 zcrypt driver. It adds an array_index_nospec() barrier for the domain
value during administrative CPRB processing to prevent potential
side-channel attacks.

> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
> --- a/drivers/s390/crypto/zcrypt_api.c
> +++ b/drivers/s390/crypto/zcrypt_api.c
> @@ -879,12 +879,13 @@ 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 check allow an authorization bypass
for administrative requests if the user specifies AUTOSEL_DOM (0xFFFF) as the
domain?

If an administrative CPRB request uses AUTOSEL_DOM, the condition
`domain < AP_DOMAINS` evaluates to false, causing the code to skip the
`test_bit_inv(domain, perms->adm)` administrative permission validation
entirely.

Later in _zcrypt_send_cprb(), the queue selection logic autoselects a queue and
only validates the user's usage permissions via zcrypt_check_queue(). Does this
allow unprivileged users with access to a restricted custom device node to
execute administrative commands on a queue where they only have usage rights?

>  		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;
>  			}
>  		} else if ((ap_msg.flags & AP_MSG_FLAG_USAGE) == 0) {
>  			rc = -EOPNOTSUPP;
>  			goto out;
>  		}
>  	}
>  	/*
>  	 * If a valid target domain is set and this domain is NOT a usage
>  	 * domain but a control only domain, autoselect target domain.
>  	 */
>  	if (domain < AP_DOMAINS &&
>  	    !ap_test_config_usage_domain(domain) &&

[Severity: Medium]
This isn't a bug introduced by this patch, but does the un-sanitized domain
value remain vulnerable to speculative out-of-bounds reads here?

The patch adds array_index_nospec() inside the AP_MSG_FLAG_ADMIN check, but
for non-administrative requests, the domain variable is used unmitigated in
ap_test_config_usage_domain() and ap_test_config_ctrl_domain().

Both functions call into ap_test_config(), which uses the domain as an array
index. Since the `domain < AP_DOMAINS` bounds check can be speculatively
bypassed, does this leave a Prime+Probe side-channel attack gadget open for
non-administrative requests?

>  	    ap_test_config_ctrl_domain(domain))
>  		domain = AUTOSEL_DOM;

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