Re: [PATCH v3 1/2] s390/zcrypt: Fix wrong domain value verification with EP11 CPRBs

[sashiko-bot@kernel.org]

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

New issues:
- [High] Missing `array_index_nospec()` mitigation leading to a Spectre v1 vulnerability.

Pre-existing issues:
- [Critical] Privilege Escalation / Access Control Bypass in administrative CPRBs.
--

--- Patch [1]: [PATCH v3 1/2] s390/zcrypt: Fix wrong domain value verification with EP11 CPRBs ---
commit 9a314209ce4cb4581d3716a2f6e1c11a24aab46e
Author: Harald Freudenberger <freude@linux.ibm.com>
s390/zcrypt: Fix wrong domain value verification with EP11 CPRBs

This commit fixes an upper limit check for the domain value when an EP11
CPRB is processed for sending to a crypto card. It adjusts the check to use
AP_DOMAINS to prevent heap memory access beyond the permissions array.

> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
> index f57189c2b839e..81eefdeae2485 100644
> --- a/drivers/s390/crypto/zcrypt_api.c
> +++ b/drivers/s390/crypto/zcrypt_api.c
> @@ -1077,7 +1077,7 @@ 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);
>  
> -	if (perms != &ap_perms && domain < AUTOSEL_DOM) {
> +	if (perms != &ap_perms && domain < AP_DOMAINS) {

[Severity: Critical]
This is a pre-existing issue, but does this logic completely bypass the
administrative permission checks if domain is greater than or equal to
AP_DOMAINS?

If a user provides a domain value like AUTOSEL_DOM (0xFFFF), this condition
evaluates to false, skipping the permission check block entirely. This
appears to allow a user with only usage access to bypass the perms->adm
check.

Since the CCA equivalent _zcrypt_send_cprb() has the same logic, and the
domain can be overwritten later with the dynamically selected queue's domain
before submission:

drivers/s390/crypto/zcrypt_msgtype6.c:zcrypt_msgtype6_send_cprb() {
    ...
    /* update domain field within the CPRB struct */
    msg->cprbx.domain = AP_QID_QUEUE(zq->queue->qid);
    ...
}

Can this lead to regressions by allowing arbitrary administrative commands to
be executed on the queue?

>  		if (ap_msg.flags & AP_MSG_FLAG_ADMIN) {

[Severity: High]
Does this code leave the domain variable vulnerable to speculative execution
attacks?

Since domain is extracted from untrusted user-provided data, the bounds
check could be mispredicted. If the CPU speculatively executes the true
path for an out-of-bounds domain, test_bit_inv() performs a bitwise load
from memory offset by domain.

Would it be appropriate to sanitize the domain variable with
array_index_nospec() before it is used as an index into perms->adm to
prevent potential kernel memory leaks via cache side channels?

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

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