Skip to content

Commit 38d7aac

Browse files
rananta468oupton
authored andcommitted
KVM: arm64: Get rid of userspace_irqchip_in_use
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: <stable@vger.kernel.org> Reported-by: syzbot <syzkaller@googlegroups.com> Suggested-by: Marc Zyngier <maz@kernel.org> Signed-off-by: Raghavendra Rao Ananta <rananta@google.com> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
1 parent b56680d commit 38d7aac

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
@@ -73,8 +73,6 @@ enum kvm_mode kvm_get_mode(void);
7373
static inline enum kvm_mode kvm_get_mode(void) { return KVM_MODE_NONE; };
7474
#endif
7575

76-
DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
77-
7876
extern unsigned int __ro_after_init kvm_sve_max_vl;
7977
extern unsigned int __ro_after_init kvm_host_sve_max_vl;
8078
int __init kvm_arm_init_sve(void);

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
@@ -69,7 +69,6 @@ DECLARE_KVM_NVHE_PER_CPU(struct kvm_cpu_context, kvm_hyp_ctxt);
6969
static bool vgic_present, kvm_arm_initialised;
7070

7171
static DEFINE_PER_CPU(unsigned char, kvm_hyp_initialized);
72-
DEFINE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
7372

7473
bool is_kvm_arm_initialised(void)
7574
{
@@ -503,9 +502,6 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
503502

504503
void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
505504
{
506-
if (vcpu_has_run_once(vcpu) && unlikely(!irqchip_in_kernel(vcpu->kvm)))
507-
static_branch_dec(&userspace_irqchip_in_use);
508-
509505
kvm_mmu_free_memory_cache(&vcpu->arch.mmu_page_cache);
510506
kvm_timer_vcpu_terminate(vcpu);
511507
kvm_pmu_vcpu_destroy(vcpu);
@@ -848,14 +844,6 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
848844
return ret;
849845
}
850846

851-
if (!irqchip_in_kernel(kvm)) {
852-
/*
853-
* Tell the rest of the code that there are userspace irqchip
854-
* VMs in the wild.
855-
*/
856-
static_branch_inc(&userspace_irqchip_in_use);
857-
}
858-
859847
mutex_lock(&kvm->arch.config_lock);
860848
set_bit(KVM_ARCH_FLAG_HAS_RAN_ONCE, &kvm->arch.flags);
861849
mutex_unlock(&kvm->arch.config_lock);
@@ -1064,7 +1052,7 @@ static bool kvm_vcpu_exit_request(struct kvm_vcpu *vcpu, int *ret)
10641052
* state gets updated in kvm_timer_update_run and
10651053
* kvm_pmu_update_run below).
10661054
*/
1067-
if (static_branch_unlikely(&userspace_irqchip_in_use)) {
1055+
if (unlikely(!irqchip_in_kernel(vcpu->kvm))) {
10681056
if (kvm_timer_should_notify_user(vcpu) ||
10691057
kvm_pmu_should_notify_user(vcpu)) {
10701058
*ret = -EINTR;
@@ -1186,7 +1174,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
11861174
vcpu->mode = OUTSIDE_GUEST_MODE;
11871175
isb(); /* Ensure work in x_flush_hwstate is committed */
11881176
kvm_pmu_sync_hwstate(vcpu);
1189-
if (static_branch_unlikely(&userspace_irqchip_in_use))
1177+
if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
11901178
kvm_timer_sync_user(vcpu);
11911179
kvm_vgic_sync_hwstate(vcpu);
11921180
local_irq_enable();
@@ -1232,7 +1220,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
12321220
* we don't want vtimer interrupts to race with syncing the
12331221
* timer virtual interrupt state.
12341222
*/
1235-
if (static_branch_unlikely(&userspace_irqchip_in_use))
1223+
if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
12361224
kvm_timer_sync_user(vcpu);
12371225

12381226
kvm_arch_vcpu_ctxsync_fp(vcpu);

0 commit comments

Comments
 (0)