Re: [PATCH v11 3/5] s390/zcrypt: Improve EP11 CPRB length and overflow checks

On 2026-08-03 10:50, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Incomplete Fix / Domain Isolation Bypass. The patch
> introduces proper ASN.1 parsing in `xcrb_msg_to_type6_ep11cprb_msgx()`
> to extract the function code, allowing shorter encodings of `func_val`
> (e.g., 1 byte instead of 4). However, it fails to update the
> subsequent send function `zcrypt_msgtype6_send_ep11_cprb()`, which
> still uses a fixed-offset C struct (`struct pld_hdr`) to overwrite the
> domain ID (`dom_val`) in the payload. Because the EP11 hardware
> natively parses the ASN.1 payload, an attacker can supply a shorter
> `func_val` to shift the actual `dom_val` field. The driver will
> validate the request but overwrite the wrong offset, leaving the
> attacker-supplied domain ID intact. The hardware will then process the
> payload using the attacker-controlled domain ID, bypassing
> cryptographic domain isolation.
> --
> 
> --- Patch [3]: [PATCH v11 3/5] s390/zcrypt: Improve EP11 CPRB length
> and overflow checks ---
> commit ded2aaf4bb041722a4604d826ab3b2f1e7bf5794
> Author: Harald Freudenberger <[email protected]>
> 
> s390/zcrypt: Improve EP11 CPRB length and overflow checks
> 
> This commit fixes missing minimum size validation and arithmetic 
> overflows
> in xcrb_msg_to_type6_ep11cprb_msgx(). It introduces proper size checks 
> and a
> simple ASN.1 parsing mechanism for the payload up to the function value
> field.
> 
>> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c 
>> b/drivers/s390/crypto/zcrypt_msgtype6.c
>> index 3e19e79d747cf..7e1f76c935ee1 100644
>> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
>> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c
>> @@ -491,26 +540,51 @@ static int xcrb_msg_to_type6_ep11cprb_msgx(bool 
>> userspace, struct ap_message *ap
> 
> [ ... ]
> 
>> +	/* function length */
>> +	if (offs >= pld_len)
>> +		return -EINVAL;
>> +	i = asn1_length_decode(pld + offs, pld_len - offs, &u);
>> +	if (i < 0)
>> +		return -EINVAL;
>> +	offs += i;
>> +	if (offs >= pld_len || u > pld_len - offs)
>> +		return -EINVAL;
>> +	/* function value */
>> +	i = asn1_int_decode(pld + offs, u, &u);
>> +	if (i < 0)
>> +		return -EINVAL;
>> +	offs += i;
>> +	*fcode = 0xFFFF & u;
> 
> [Severity: Critical]
> This is a pre-existing issue, but this updated parsing exposes a bypass 
> of
> the cryptographic domain isolation.
> 
> The new ASN.1 logic correctly parses a variable-length payload, 
> allowing a
> shorter encoding of func_val (e.g., 1 byte instead of 4). However, how 
> does
> the subsequent send function handle this?
> 
> Looking at zcrypt_msgtype6_send_ep11_cprb(), it appears to still use a
> fixed C-struct offset to overwrite the domain ID:
> 
> drivers/s390/crypto/zcrypt_msgtype6.c:zcrypt_msgtype6_send_ep11_cprb() 
> {
>         ...
>         payload_hdr = (struct pld_hdr *)((&msg->pld_lenfmt) + lfmt);
>         payload_hdr->dom_val = AP_QID_QUEUE(zq->queue->qid);
>         ...
> }
> 
> If an unprivileged user issues an EP11 CPRB ioctl with a valid ASN.1 
> payload
> containing a 1-byte function ID, wouldn't the fixed offset in
> zcrypt_msgtype6_send_ep11_cprb() miss the actual dom_val field?
> 
> It seems this would result in the driver validating the request but
> overwriting the wrong offset. This could leave an attacker-supplied 
> domain
> ID intact, allowing the EP11 hardware to process the payload using the
> wrong domain.

And exactly this is addressed with #4 of this patch series.