Re: [PATCH] KVM: riscv: Avoid redundant interrupt check for pending timers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Masked timer interrupts cause an infinite loop of spurious WFI wakeups, leading to 100% CPU usage for idle guests.
- [High] The hrtimer cancellation during vCPU teardown can be bypassed, leading to a Use-After-Free if the timer callback executes concurrently.
- [High] Restoring KVM_REG_RISCV_TIMER_REG(state) via the ONE_REG interface overwrites the timer's compare value with the state flag (1).
--

--- Patch [1]: [PATCH] KVM: riscv: Avoid redundant interrupt check for pending timers ---
commit 91f0cac16b524c91a61fc14d9cc9aaf56d289c69
Author: Jinyu Tang <jinyu.tang@linux.dev>

KVM: riscv: Avoid redundant interrupt check for pending timers
    
kvm_vcpu_check_block() calls kvm_arch_vcpu_runnable() before
checking whether a timer is pending. On RISC-V, kvm_arch_vcpu_runnable()
already checks all pending guest interrupts through
kvm_riscv_vcpu_has_interrupts(vcpu, -1UL), including IRQ_VS_TIMER.

Drop the repeated IRQ_VS_TIMER pending-interrupt check from
kvm_riscv_vcpu_timer_pending() and keep the helper focused on the timer
deadline state. If the timer becomes pending after the runnable check,
the timer path still injects IRQ_VS_TIMER and kicks the vCPU through the
normal wakeup path.

> diff --git a/arch/riscv/kvm/vcpu_timer.c b/arch/riscv/kvm/vcpu_timer.c
> index 85a7262115e13..f6919048a584b 100644
> --- a/arch/riscv/kvm/vcpu_timer.c
> +++ b/arch/riscv/kvm/vcpu_timer.c
> @@ -130,11 +130,7 @@ bool kvm_riscv_vcpu_timer_pending(struct kvm_vcpu *vcpu)
>  	struct kvm_vcpu_timer *t = &vcpu->arch.timer;
>  	struct kvm_guest_timer *gt = &vcpu->kvm->arch.timer;
>  
> -	if (!kvm_riscv_delta_cycles2ns(t->next_cycles, gt, t) ||
> -	    kvm_riscv_vcpu_has_interrupts(vcpu, 1UL << IRQ_VS_TIMER))
> -		return true;
> -	else
> -		return false;
> +	return !kvm_riscv_delta_cycles2ns(t->next_cycles, gt, t);

[Severity: High]
This is a pre-existing issue, but this function returns true when the timer
has expired without checking if the timer interrupt is actually unmasked by
the guest.

Can this cause an infinite loop of spurious WFI wakeups? If a guest OS masks
the timer interrupt (vsie.STIE = 0), leaves the timer comparator in the past,
and executes the WFI instruction, kvm_arch_vcpu_runnable() correctly returns
false.

However, kvm_cpu_has_pending_timer() might use this helper and return true,
ignoring the mask. KVM would then break out of kvm_vcpu_block() and resume
the guest. The guest would re-execute WFI, immediately trapping again.

Could this lead to 100% host CPU usage for idle guests?

>  }
>  
>  static void kvm_riscv_vcpu_timer_blocking(struct kvm_vcpu *vcpu)

[Severity: High]
This isn't a bug introduced by this patch, but can hrtimer cancellation
during vCPU teardown be bypassed, leading to a Use-After-Free?

Looking at kvm_riscv_vcpu_hrtimer_expired() in arch/riscv/kvm/vcpu_timer.c:

	t->next_set = false;
	kvm_riscv_vcpu_set_interrupt(vcpu, IRQ_VS_TIMER);

And in kvm_riscv_vcpu_timer_cancel():

	if (!t->init_done || !t->next_set)
		return -EINVAL;

	hrtimer_cancel(&t->hrt);

Since t->next_set is cleared locklessly in kvm_riscv_vcpu_hrtimer_expired()
before it finishes accessing the vcpu structure, what happens if VM teardown
races with this callback?

If kvm_riscv_vcpu_timer_deinit() calls kvm_riscv_vcpu_timer_cancel() and sees
t->next_set as false, it skips hrtimer_cancel(). Does this allow teardown to
free the vcpu while the callback is still actively using it?

[Severity: High]
This is a pre-existing issue, but is the timer's compare value overwritten
when restoring KVM_REG_RISCV_TIMER_REG(state) via the ONE_REG interface?

In arch/riscv/kvm/vcpu_timer.c:kvm_riscv_vcpu_set_reg_timer():

	case KVM_REG_RISCV_TIMER_REG(state):
		if (reg_val == KVM_RISCV_TIMER_STATE_ON)
			ret = kvm_riscv_vcpu_timer_next_event(vcpu, reg_val);
		else
			ret = kvm_riscv_vcpu_timer_cancel(t);
		break;

When userspace restores the VM state during live migration, restoring state
with KVM_RISCV_TIMER_STATE_ON (1) incorrectly passes reg_val (1) as the target
cycles deadline to kvm_riscv_vcpu_timer_next_event().

Would this overwrite t->next_cycles with 1, permanently losing the actual
restored timer comparator value and causing immediate spurious interrupts?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814123541.593613-1-jinyu.tang@linux.dev?part=1