Re: [PATCH 14/60] kvm: Make kvm_running_vcpus point to struct kvm_vcpu_common

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Unconditional dereference of a potentially NULL per-CPU variable (`common`) in KVM hardware interrupt handlers.
--

commit f697a135faef85bfacb6055235544bacfb1b5251
Author: Joerg Roedel <joerg.roedel@amd.com>

kvm: Make kvm_running_vcpus point to struct kvm_vcpu_common

This commit updates the kvm_running_vcpu per-CPU variable to point to
struct kvm_vcpu_common instead of kvm_vcpu, aiming to eliminate the need
for updates during plane switches.

> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index cbea4d9ee9552..b2c4f422414ef 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -204,7 +204,8 @@ static void soft_timer_cancel(struct hrtimer *hrt)
>  
>  static irqreturn_t kvm_arch_timer_handler(int irq, void *dev_id)
>  {
> -	struct kvm_vcpu *vcpu = *(struct kvm_vcpu **)dev_id;
> +	struct kvm_vcpu_common *common = *(struct kvm_vcpu_common **)dev_id;
> +	struct kvm_vcpu *vcpu = common->current_vcpu;

[Severity: Critical]
Can the per-CPU variable pointer be NULL here when the timer interrupt fires?

As noted in the existing comment just below this in kvm_arch_timer_handler(),
hardware interrupts can fire asynchronously after vcpu_put() has already
cleared the per-CPU pointer because the hardware interrupt signal may not
have retired from the interrupt controller yet.

If common is NULL, dereferencing it immediately to access current_vcpu
would cause a host kernel panic in interrupt context. Should common be
checked before it is dereferenced?

>  	struct arch_timer_context *ctx;
>  	struct timer_map map;

[ ... ]

> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index 933983bb20052..a12b89b423d5e 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -730,7 +730,8 @@ void kvm_vgic_cpu_down(void)
>  
>  static irqreturn_t vgic_maintenance_handler(int irq, void *data)
>  {
> -	struct kvm_vcpu *vcpu = *(struct kvm_vcpu **)data;
> +	struct kvm_vcpu_common *common = *(struct kvm_vcpu_common **)data;
> +	struct kvm_vcpu *vcpu = common->current_vcpu;

[Severity: Critical]
Could common also be NULL here during a maintenance interrupt?

Similar to the arch timer handler, if the vgic maintenance interrupt is
delivered asynchronously after the active vCPU has been put, dereferencing
common before checking it could result in a NULL pointer dereference.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608144252.351443-1-joro@8bytes.org?part=14