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

On 2026-07-29 16:50, [email protected] wrote:
> 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 <[email protected]>
> 
> 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;
>> +	}
>> +}
> 
> [ ... ]

These both unaligned issues will be fixed with v9 by using 
put_unaligned_be16/32()