Re: [PATCH v8 4/5] s390/zcrypt: Improve EP11 CPRB domain handling with ASN.1 parsing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Undefined behavior due to unaligned memory access via direct pointer casting.
--

--- Patch [4]: [PATCH v8 4/5] s390/zcrypt: Improve EP11 CPRB domain handling with ASN.1 parsing ---
commit 78d69567226ac24ed3207a9812dbc20b238a13e1
Author: Harald Freudenberger <freude@linux.ibm.com>

s390/zcrypt: Improve EP11 CPRB domain handling with ASN.1 parsing

This commit replaces struct overlays with explicit ASN.1 parsing for
EP11 CPRB payload domain fields, adding validation and a new helper
function for safely encoding integer values.

> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
> index c4b5d9b85f7a9..59e182d1bddf9 100644
> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c
> @@ -1223,6 +1223,28 @@ int prep_ep11_ap_msg(bool userspace, struct ep11_urb *xcrb,
>  					       func_code, domain);
>  }
>  
> +/*
> + * Simple asn1 int writer/encoder helper function
> + * Returns number of bytes processed or < 0 on failure
> + * Only accepts int length values of 1, 2 or 4.
> + */
> +static inline int asn1_int_encode(u8 *buf, size_t intlen, u32 u)
> +{
> +	switch (intlen) {
> +	case 1:
> +		*((u8 *)buf) = (u8)u;
> +		return 1;
> +	case 2:
> +		*((u16 *)buf) = cpu_to_be16(u);

[Severity: Medium]
Could this lead to unaligned memory accesses?

When asn1_int_encode() is called from zcrypt_msgtype6_send_ep11_cprb()
using pld + offs, the offset is dynamically calculated from variable length
ASN.1 tags and lengths, which may leave the buf pointer unaligned.

While s390 hardware handles unaligned accesses, directly casting a potentially
unaligned u8 pointer to u16* could violate C standard alignment rules and
potentially lead to compiler misoptimization. Would it be safer to use
put_unaligned_be16() here?

> +		return 2;
> +	case 4:
> +		*((u32 *)buf) = cpu_to_be32(u);

[Severity: Medium]
Similarly, should this use put_unaligned_be32() instead of a direct cast
to u32* for the same unaligned pointer reasons?

> +		return 4;
> +	default:
> +		return -EINVAL;
> +	}
> +}

[ ... ]

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