Re: [PATCH v3] s390/cpacf: Unpoison instruction results

On 8/14/26 14:27, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] Unconditional KMSAN unpoisoning masks partial completions and information leaks.
> 
> Pre-existing issues:
> - [Critical] Silent ignoring of PRNO and TRNG partial completions causes critical information leaks.
> - [High] Silent ignoring of KMA operation failures causes cryptographic protocol violations.
> --
> 
> --- Patch [1]: [PATCH v3] s390/cpacf: Unpoison instruction results ---
> commit c65c87d6cf548eaef4bd0b06821cb0e68caaa4c3
> Author: Ilya Leoshkevich <[email protected]>
> 
> s390/cpacf: Unpoison instruction results
> 
> This commit suppresses KMSAN uninitialized memory warnings for CPACF
> outputs by adding kmsan_unpoison_memory() calls to multiple wrappers.
> It intentionally avoids modifying variable-length parameter blocks and
> the cpacf_kma() wrapper.
> 
>> Also do not touch cpacf_kma(): this wrapper does not handle cc 1 and
>> cc2 at the moment and has to be reworked.
> 
> [Severity: High]
> This is a pre-existing issue, but does silently falling through on Condition
> Code 1 and 2 in cpacf_kma() cause callers (like gcm_aes_crypt) to accept
> cryptographic errors?
> 
> By returning void when the KMA instruction encounters an error (such as
> incorrect AAD/plaintext lengths or a hardware abort), callers have no way
> of knowing the operation failed. This could result in partially written or
> completely uninitialized ciphertext, plaintext, and tags being processed as
> valid data.

Yes. As stated in the commit message and discussed in v2, this will be
addressed separately.

>> diff --git a/arch/s390/include/asm/cpacf.h b/arch/s390/include/asm/cpacf.h
>> index a83683169d98d..6174552d856dc 100644
>> --- a/arch/s390/include/asm/cpacf.h
>> +++ b/arch/s390/include/asm/cpacf.h
> 
> [ ... ]
> 
>> @@ -619,6 +624,7 @@ static inline void cpacf_prno(unsigned long func, void *param,
>>   		: [fc] "d" (func), [pba] "d" ((unsigned long)param),
>>   		  [seed] "d" (s.pair), [opc] "i" (CPACF_PRNO)
>>   		: "cc", "memory", "0", "1");
>> +	kmsan_unpoison_memory(dest, dest_len);
> 
> [Severity: High]
> Does unpoisoning the requested length (dest_len) instead of the written length
> (dest_len - d.odd) mask partial completions?
> 
> When cpacf_prno() terminates early (e.g., returning Condition Code 1 due to
> entropy exhaustion), the hardware only partially fills the destination buffer.
> Unconditionally unpoisoning the requested length blinds KMSAN to the
> uninitialized tail of the buffer, potentially hiding active kernel memory
> information leaks.
> 
> It appears the pre-existing cpacf_trng() wrapper suffers from the same
> issue, unpoisoning ucbuf_len and cbuf_len unconditionally.

PRNO cannot return CC 1. The only documented values are 0 (normal
completion) and 3 (partial completion).

> [Severity: Critical]
> This is a pre-existing issue, but does the silent exit on Condition Code 1
> in cpacf_trng() cause uninitialized kernel memory to be copied to userspace?
> 
> Looking at cpacf_trng() in arch/s390/include/asm/cpacf.h, the assembly only
> loops on Condition Code 3:
> 
> 	asm volatile (
> 		"	lghi	0,%[fc]\n"
> 		"0:	.insn	rre,%[opc] << 16,%[ucbuf],%[cbuf]\n"
> 		"	brc	1,0b"	  /* handle partial completion */
> 		: [ucbuf] "+&d" (u.pair), [cbuf] "+&d" (c.pair)
> 		: [fc] "K" (CPACF_PRNO_TRNG), [opc] "i" (CPACF_PRNO)
> 		: "cc", "memory", "0");
> 
> If hardware TRNG entropy is exhausted, it returns CC 1 and falls through,
> discarding the unwritten byte counters (c.odd) and returning void.
> 
> Because the wrapper returns void, trng_read() in
> drivers/char/hw_random/s390-trng.c falsely assumes the entire buffer allocated
> with kmalloc(..., GFP_KERNEL) was filled:
> 
> 	cpacf_trng(NULL, 0, p, n);
> 	atomic64_add(n, &trng_dev_counter);
> 	if (copy_to_user(ubuf, p, n)) {
> 
> Can this sequence leak uninitialized kernel heap memory to any unprivileged
> local user reading from /dev/trng until entropy runs out?

Same as above.