Re: [PATCH v4 26/27] KVM: s390: arm64: Implement basic page fault handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Guest-triggerable WARN_ONCE via KVM_BUG in kvm_handle_pic leading to host DoS
- [Low] Unused parameters in user_mem_abort function signature
- [Low] Type mismatch: Assigning macro FOLL_WRITE to a boolean variable
--

commit d14fe466e599e9b62bd9a5838ffa35d7c0a359d1
Author: Steffen Eiden <seiden@linux.ibm.com>
Subject: KVM: s390: arm64: Implement basic page fault handler

This patch adds host functionality to page in guest memory and aborts
guest execution if the guest does something unexpected or illegal.

> diff --git a/arch/s390/kvm/arm64/mmu.c b/arch/s390/kvm/arm64/mmu.c
> new file mode 100644
> index 0000000000000..d5acabd762ba4
> --- /dev/null
> +++ b/arch/s390/kvm/arm64/mmu.c
> @@ -0,0 +1,178 @@

[ ... ]

> +static int user_mem_abort(struct kvm_vcpu *vcpu, gpa_t fault_ipa,
> +			  struct kvm_memory_slot *slot, hva_t hva)
> +{

[Severity: Low]
Are the slot and hva parameters needed here? They appear to be unused in
the function body.

> +	struct guest_fault f = { };
> +	int ret;
> +
> +	if (kvm_s390_cur_gmap_fault_is_write(vcpu))
> +		f.write_attempt = FOLL_WRITE;

[Severity: Low]
Is it intentional to assign the FOLL_WRITE integer macro directly to the
write_attempt boolean field? 

Would it be clearer to assign this to true instead?

> +	f.gfn = gpa_to_gfn(fault_ipa);

[ ... ]

> +static int kvm_handle_pic(struct kvm_vcpu *vcpu, bool *translation)
> +{
> +	switch (kvm_vcpu_fault_pic(vcpu)) {
> +	/* expected cases: */
> +	case PGM_ASCE_TYPE:

[ ... ]

> +	case PGM_PROTECTION:
> +		break;
> +	/* unexpected cases: */
> +	case 0:
> +		KVM_BUG(1, vcpu->kvm, "On MMU fault path but no fault occurred");
> +		return -EFAULT;
> +	default:
> +		KVM_BUG(1, vcpu->kvm, "Unexpected program interrupt 0x%x, TEID 0x%016lx",
> +			vcpu->arch.sae_block.hai.pic, vcpu->arch.sae_block.hai.teid.val);
> +		send_sig(SIGSEGV, current, 0);
> +		return -EFAULT;
> +	}
> +
> +	return 0;
> +}

[Severity: High]
Does this allow a guest to trigger a host WARN_ONCE and potentially a kernel
panic?

For faults like Alignment Faults or Synchronous External Aborts, S390 hardware
might not report a DAT exception, meaning hai.pic could be 0 or an unrelated
code.

Can an unprivileged guest performing an unaligned memory access hit these
KVM_BUG() paths and crash the host if panic_on_warn is enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706085229.979525-1-seiden@linux.ibm.com?part=26