Re: [PATCH v3 25/27] KVM: s390: arm64: Implement vCPU IOCTLs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Userspace-reachable WARN_ONCE creates a host-side DoS vulnerability via hardware validity intercepts.
- [High] Calling `local_irq_enable()` inside an RCU extended quiescent state breaks context tracking and introduces a race condition.
- [High] The vCPU run loop fails to set `vcpu->mode` to `IN_GUEST_MODE`, leading to silently dropped IPI kicks.
- [High] Crucial KVM requests (such as KVM_REQ_VM_DEAD and KVM_REQ_SLEEP) are ignored, allowing execution in an invalid or fatal state.
- [High] The vCPU run loop checks `need_resched()` and `signal_pending()` with interrupts enabled, causing a TOCTOU race with host interrupts.
- [Medium] `kvm_arm_vcpu_finalize` unconditionally returns success for unsupported features.
--

commit 9e53079e1aa1b7ab592dcb9275360b129725a075
Author: Steffen Eiden <seiden@linux.ibm.com>
Subject: KVM: s390: arm64: Implement vCPU IOCTLs

This patch implements the core vCPU ioctls for the s390 arm64 KVM port. It
adds support for initializing and running the guest, as well as handling
registers and handling exceptions through the SAE block.

diff --git a/arch/s390/kvm/arm64/arm.c b/arch/s390/kvm/arm64/arm.c
--- a/arch/s390/kvm/arm64/arm.c
+++ b/arch/s390/kvm/arm64/arm.c
[ ... ]
@@ -244,12 +268,12 @@
> +static int check_vcpu_requests(struct kvm_vcpu *vcpu)
> +{
> +	if (kvm_request_pending(vcpu)) {
> +		if (kvm_check_request(KVM_REQ_VCPU_RESET, vcpu))
> +			kvm_reset_vcpu(vcpu);
> +		/*
> +		 * Clear IRQ_PENDING requests that were made to guarantee
> +		 * that a VCPU sees new virtual interrupts.
> +		 */
> +		kvm_check_request(KVM_REQ_IRQ_PENDING, vcpu);
> +	}
> +
> +	return 1;
> +}

[Severity: High]
Does this code ignore crucial KVM requests like KVM_REQ_VM_DEAD or
KVM_REQ_SLEEP? It looks like it only processes KVM_REQ_VCPU_RESET and
KVM_REQ_IRQ_PENDING. If a fatal error occurs and KVM_REQ_VM_DEAD is pending,
would returning 1 here cause the vCPU to immediately resume guest execution
and override the KVM failsafe?

[ ... ]
@@ -348,15 +373,15 @@
> +static void arm_vcpu_run(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_sae_block *sae_block = &vcpu->arch.sae_block;
> +
> +	adjust_pc(vcpu);
> +
> +	local_irq_disable();
> +	guest_enter_irqoff();
> +	local_irq_enable();

[Severity: High]
Does calling local_irq_enable() immediately after guest_enter_irqoff() break
context tracking? guest_enter_irqoff() signals to RCU that the CPU is entering
an extended quiescent state. If a host interrupt fires after local_irq_enable()
but before the actual guest entry, would the interrupt handler execute while
RCU considers the CPU quiescent, leading to lockdep/RCU splats or potential
use-after-free bugs?

> +
> +	sae_block->icptr = 0;
> +
> +	sae64a(sae_block);
> +
> +	local_irq_disable();
> +	guest_exit_irqoff();
> +	local_irq_enable();
> +}
[ ... ]
@@ -395,20 +420,20 @@
> +	ret = 1;
> +	do {
> +		if (signal_pending(current)) {
> +			kvm_run->exit_reason = KVM_EXIT_INTR;
> +			ret = -EINTR;
> +			continue;
> +		}
> +
> +		if (need_resched())
> +			schedule();

[Severity: High]
Can this sequence cause a time-of-check to time-of-use race with host
interrupts? The checks for signal_pending() and need_resched() in
kvm_arch_vcpu_ioctl_run() are performed with local interrupts enabled. If a
host interrupt arrives immediately after these checks and sets a reschedule or
signal flag, would the vCPU blindly enter the guest via arm_vcpu_run() and
ignore the pending request for a long time?

> +
> +		if (ret > 0)
> +			ret = check_vcpu_requests(vcpu);
> +
> +		vcpu->arch.sae_block.icptr = 0;
> +
> +		arm_vcpu_run(vcpu);

[Severity: High]
Are we missing the transition of vcpu->mode to IN_GUEST_MODE before calling
arm_vcpu_run()? KVM's generic kvm_vcpu_kick() mechanism relies on checking if
vcpu->mode == IN_GUEST_MODE to send an IPI. If the state remains
OUTSIDE_GUEST_MODE, would concurrent kicks for virtual interrupt injections or
halt requests silently drop the IPI and leave the vCPU trapped in guest
execution?

> +
> +		icptr = vcpu->arch.sae_block.icptr;
> +		switch (icptr) {
> +		case SAE_ICPTR_SPURIOUS:
> +			break;
> +		case SAE_ICPTR_VALIDITY:
> +			WARN_ONCE(true, "SAE: validity intercept. vir: 0x%04x",
> +				  vcpu->arch.sae_block.vir);
> +			ret = -EINVAL;
> +			break;

[Severity: High]
Is it possible for malicious userspace to trigger this WARN_ONCE? If an
unprivileged host userspace provides invalid vCPU register states via
KVM_SET_ONE_REG prior to running the vCPU, it could force a validity intercept.
KVM subsystem guidelines forbid guest- or host-userspace-reachable WARN_ON
assertions since they can crash the host if panic_on_warn is enabled.

> +		case SAE_ICPTR_SYNCHRONOUS_EXCEPTION:
> +			ret = handle_trap_exceptions(vcpu);
> +			break;
[ ... ]
diff --git a/arch/s390/kvm/arm64/reset.c b/arch/s390/kvm/arm64/reset.c
new file mode 100644
--- /dev/null
+++ b/arch/s390/kvm/arm64/reset.c
[ ... ]
@@ -48,10 +48,10 @@
> +
> +int kvm_arm_vcpu_finalize(struct kvm_vcpu *vcpu, int feature)
> +{
> +	return 0;
> +}

[Severity: Medium]
Should this function validate the requested feature instead of unconditionally
returning success? By ignoring the feature argument and returning 0, would
userspace incorrectly believe an unsupported feature is successfully
configured, leading to unpredictable failures later?

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