Skip to content

Commit 2aa5670

Browse files
rananta468Sasha Levin
authored andcommitted
KVM: arm64: Get rid of userspace_irqchip_in_use
commit 38d7aac upstream. Improper use of userspace_irqchip_in_use led to syzbot hitting the following WARN_ON() in kvm_timer_update_irq(): WARNING: CPU: 0 PID: 3281 at arch/arm64/kvm/arch_timer.c:459 kvm_timer_update_irq+0x21c/0x394 Call trace: kvm_timer_update_irq+0x21c/0x394 arch/arm64/kvm/arch_timer.c:459 kvm_timer_vcpu_reset+0x158/0x684 arch/arm64/kvm/arch_timer.c:968 kvm_reset_vcpu+0x3b4/0x560 arch/arm64/kvm/reset.c:264 kvm_vcpu_set_target arch/arm64/kvm/arm.c:1553 [inline] kvm_arch_vcpu_ioctl_vcpu_init arch/arm64/kvm/arm.c:1573 [inline] kvm_arch_vcpu_ioctl+0x112c/0x1b3c arch/arm64/kvm/arm.c:1695 kvm_vcpu_ioctl+0x4ec/0xf74 virt/kvm/kvm_main.c:4658 vfs_ioctl fs/ioctl.c:51 [inline] __do_sys_ioctl fs/ioctl.c:907 [inline] __se_sys_ioctl fs/ioctl.c:893 [inline] __arm64_sys_ioctl+0x108/0x184 fs/ioctl.c:893 __invoke_syscall arch/arm64/kernel/syscall.c:35 [inline] invoke_syscall+0x78/0x1b8 arch/arm64/kernel/syscall.c:49 el0_svc_common+0xe8/0x1b0 arch/arm64/kernel/syscall.c:132 do_el0_svc+0x40/0x50 arch/arm64/kernel/syscall.c:151 el0_svc+0x54/0x14c arch/arm64/kernel/entry-common.c:712 el0t_64_sync_handler+0x84/0xfc arch/arm64/kernel/entry-common.c:730 el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:598 The following sequence led to the scenario: - Userspace creates a VM and a vCPU. - The vCPU is initialized with KVM_ARM_VCPU_PMU_V3 during KVM_ARM_VCPU_INIT. - Without any other setup, such as vGIC or vPMU, userspace issues KVM_RUN on the vCPU. Since the vPMU is requested, but not setup, kvm_arm_pmu_v3_enable() fails in kvm_arch_vcpu_run_pid_change(). As a result, KVM_RUN returns after enabling the timer, but before incrementing 'userspace_irqchip_in_use': kvm_arch_vcpu_run_pid_change() ret = kvm_arm_pmu_v3_enable() if (!vcpu->arch.pmu.created) return -EINVAL; if (ret) return ret; [...] if (!irqchip_in_kernel(kvm)) static_branch_inc(&userspace_irqchip_in_use); - Userspace ignores the error and issues KVM_ARM_VCPU_INIT again. Since the timer is already enabled, control moves through the following flow, ultimately hitting the WARN_ON(): kvm_timer_vcpu_reset() if (timer->enabled) kvm_timer_update_irq() if (!userspace_irqchip()) ret = kvm_vgic_inject_irq() ret = vgic_lazy_init() if (unlikely(!vgic_initialized(kvm))) if (kvm->arch.vgic.vgic_model != KVM_DEV_TYPE_ARM_VGIC_V2) return -EBUSY; WARN_ON(ret); Theoretically, since userspace_irqchip_in_use's functionality can be simply replaced by '!irqchip_in_kernel()', get rid of the static key to avoid the mismanagement, which also helps with the syzbot issue. Cc: <[email protected]> Reported-by: syzbot <[email protected]> Suggested-by: Marc Zyngier <[email protected]> Signed-off-by: Raghavendra Rao Ananta <[email protected]> Signed-off-by: Oliver Upton <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 58689ad commit 2aa5670

File tree

3 files changed

+4
-19
lines changed

3 files changed

+4
-19
lines changed

arch/arm64/include/asm/kvm_host.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,6 @@ enum kvm_mode kvm_get_mode(void);
7272
static inline enum kvm_mode kvm_get_mode(void) { return KVM_MODE_NONE; };
7373
#endif
7474

75-
DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
76-
7775
extern unsigned int __ro_after_init kvm_sve_max_vl;
7876
int __init kvm_arm_init_sve(void);
7977

arch/arm64/kvm/arch_timer.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,7 @@ void get_timer_map(struct kvm_vcpu *vcpu, struct timer_map *map)
206206

207207
static inline bool userspace_irqchip(struct kvm *kvm)
208208
{
209-
return static_branch_unlikely(&userspace_irqchip_in_use) &&
210-
unlikely(!irqchip_in_kernel(kvm));
209+
return unlikely(!irqchip_in_kernel(kvm));
211210
}
212211

213212
static void soft_timer_start(struct hrtimer *hrt, u64 ns)

arch/arm64/kvm/arm.c

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ DECLARE_KVM_NVHE_PER_CPU(struct kvm_cpu_context, kvm_hyp_ctxt);
5757
static bool vgic_present, kvm_arm_initialised;
5858

5959
static DEFINE_PER_CPU(unsigned char, kvm_hyp_initialized);
60-
DEFINE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
6160

6261
bool is_kvm_arm_initialised(void)
6362
{
@@ -401,9 +400,6 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
401400

402401
void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
403402
{
404-
if (vcpu_has_run_once(vcpu) && unlikely(!irqchip_in_kernel(vcpu->kvm)))
405-
static_branch_dec(&userspace_irqchip_in_use);
406-
407403
kvm_mmu_free_memory_cache(&vcpu->arch.mmu_page_cache);
408404
kvm_timer_vcpu_terminate(vcpu);
409405
kvm_pmu_vcpu_destroy(vcpu);
@@ -627,14 +623,6 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
627623
return ret;
628624
}
629625

630-
if (!irqchip_in_kernel(kvm)) {
631-
/*
632-
* Tell the rest of the code that there are userspace irqchip
633-
* VMs in the wild.
634-
*/
635-
static_branch_inc(&userspace_irqchip_in_use);
636-
}
637-
638626
/*
639627
* Initialize traps for protected VMs.
640628
* NOTE: Move to run in EL2 directly, rather than via a hypercall, once
@@ -856,7 +844,7 @@ static bool kvm_vcpu_exit_request(struct kvm_vcpu *vcpu, int *ret)
856844
* state gets updated in kvm_timer_update_run and
857845
* kvm_pmu_update_run below).
858846
*/
859-
if (static_branch_unlikely(&userspace_irqchip_in_use)) {
847+
if (unlikely(!irqchip_in_kernel(vcpu->kvm))) {
860848
if (kvm_timer_should_notify_user(vcpu) ||
861849
kvm_pmu_should_notify_user(vcpu)) {
862850
*ret = -EINTR;
@@ -975,7 +963,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
975963
vcpu->mode = OUTSIDE_GUEST_MODE;
976964
isb(); /* Ensure work in x_flush_hwstate is committed */
977965
kvm_pmu_sync_hwstate(vcpu);
978-
if (static_branch_unlikely(&userspace_irqchip_in_use))
966+
if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
979967
kvm_timer_sync_user(vcpu);
980968
kvm_vgic_sync_hwstate(vcpu);
981969
local_irq_enable();
@@ -1021,7 +1009,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
10211009
* we don't want vtimer interrupts to race with syncing the
10221010
* timer virtual interrupt state.
10231011
*/
1024-
if (static_branch_unlikely(&userspace_irqchip_in_use))
1012+
if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
10251013
kvm_timer_sync_user(vcpu);
10261014

10271015
kvm_arch_vcpu_ctxsync_fp(vcpu);

0 commit comments

Comments
 (0)