Re: [PATCH v2 2/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] kzalloc() does not guarantee the 4K-byte page alignment strictly required by the s390x STSI hardware instruction, leading to a host kernel panic.
--

commit 1bfe659dfe76809762877435521d74d061f7364a
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>
KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer

This commit replaces get_zeroed_page() with kzalloc() when allocating the
STSI response buffer. The intention is to use a cleaner API that does not
require casting and does not need the size when freeing the memory.

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/arch/s390/kvm/s390/priv.c b/arch/s390/kvm/s390/priv.c
> index b3cb2c2c3aa71..856939acf012f 100644
> --- a/arch/s390/kvm/s390/priv.c
> +++ b/arch/s390/kvm/s390/priv.c

[ ... ]

> @@ -911,19 +912,19 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
>  	switch (fc) {
>  	case 1: /* same handling for 1 and 2 */
>  	case 2:
> -		mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
> +		mem = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);

[Severity: Critical]
In handle_stsi(), does kzalloc() guarantee the 4K-byte page alignment that
the s390x stsi hardware instruction requires? The previous get_zeroed_page()
guaranteed page alignment, but kzalloc() might not if kernel SLUB debugging
(e.g., CONFIG_SLUB_DEBUG with redzoning) is enabled.

>  		if (!mem)
>  			goto out_no_data;
> -		if (stsi((void *) mem, fc, sel1, sel2))
> +		if (stsi(mem, fc, sel1, sel2))

[Severity: Critical]
If the buffer is unaligned, will executing the native stsi instruction here
trigger a host specification exception? Since the stsi() inline assembly in
arch/s390/include/asm/sysinfo.h lacks exception table handling, it seems an
unprivileged guest could reliably trigger a host kernel panic by invoking the
stsi instruction.

>  			goto out_no_data;
>  		break;
>  	case 3:
>  		if (sel1 != 2 || sel2 != 2)
>  			goto out_no_data;
> -		mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
> +		mem = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);

[Severity: Critical]
Does this second allocation in handle_stsi() introduce the same alignment
issue when the buffer is passed to handle_stsi_3_2_2() and used in the
stsi() execution there?

>  		if (!mem)
>  			goto out_no_data;
> -		handle_stsi_3_2_2(vcpu, (void *) mem);
> +		handle_stsi_3_2_2(vcpu, mem);
>  		break;
>  	case 15: /* fc 15 is fully handled in userspace */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-s390-kvm-v2-0-2cf6434e6646@kernel.org?part=2