| Message ID | 1568619752-3885-1-git-send-email-wanpengli@tencent.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] KVM: hyperv: Fix Direct Synthetic timers assert an interrupt w/o lapic_in_kernel | expand |
Wanpeng Li <kernellwp@gmail.com> writes: > From: Wanpeng Li <wanpengli@tencent.com> > > Reported by syzkaller: > > kasan: GPF could be caused by NULL-ptr deref or user memory access > general protection fault: 0000 [#1] PREEMPT SMP KASAN > RIP: 0010:__apic_accept_irq+0x46/0x740 arch/x86/kvm/lapic.c:1029 > Call Trace: > kvm_apic_set_irq+0xb4/0x140 arch/x86/kvm/lapic.c:558 > stimer_notify_direct arch/x86/kvm/hyperv.c:648 [inline] > stimer_expiration arch/x86/kvm/hyperv.c:659 [inline] > kvm_hv_process_stimers+0x594/0x1650 arch/x86/kvm/hyperv.c:686 > vcpu_enter_guest+0x2b2a/0x54b0 arch/x86/kvm/x86.c:7896 > vcpu_run+0x393/0xd40 arch/x86/kvm/x86.c:8152 > kvm_arch_vcpu_ioctl_run+0x636/0x900 arch/x86/kvm/x86.c:8360 > kvm_vcpu_ioctl+0x6cf/0xaf0 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2765 > > The testcase programs HV_X64_MSR_STIMERn_CONFIG/HV_X64_MSR_STIMERn_COUNT, > in addition, there is no lapic in the kernel, the counters value are small > enough in order that kvm_hv_process_stimers() inject this already-expired > timer interrupt into the guest through lapic in the kernel which triggers > the NULL deferencing. This patch fixes it by don't advertise direct mode > synthetic timers and discarding the inject when lapic is not in kernel. > > syzkaller source: https://syzkaller.appspot.com/x/repro.c?x=1752fe0a600000 > > Reported-by: syzbot+dff25ee91f0c7d5c1695@syzkaller.appspotmail.com > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Radim Krčmář <rkrcmar@redhat.com> > Cc: Vitaly Kuznetsov <vkuznets@redhat.com> > Signed-off-by: Wanpeng Li <wanpengli@tencent.com> > --- > v2 -> v3: > * add the link of syzkaller source > v1 -> v2: > * don't advertise direct mode synthetic timers when lapic is not in kernel > > arch/x86/kvm/hyperv.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c > index c10a8b1..069e655 100644 > --- a/arch/x86/kvm/hyperv.c > +++ b/arch/x86/kvm/hyperv.c > @@ -645,7 +645,9 @@ static int stimer_notify_direct(struct kvm_vcpu_hv_stimer *stimer) > .vector = stimer->config.apic_vector > }; > > - return !kvm_apic_set_irq(vcpu, &irq, NULL); > + if (lapic_in_kernel(vcpu)) > + return !kvm_apic_set_irq(vcpu, &irq, NULL); > + return 0; We can go even further and forbid to enable direct mode by adding lapic_in_kernel() check to stimer_set_config() but the guest (or userspace setting CPUIDs) is already misbehaving and we can't magically fix things in KVM. > } > > static void stimer_expiration(struct kvm_vcpu_hv_stimer *stimer) > @@ -1849,7 +1851,13 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid, > > ent->edx |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE; > ent->edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE; > - ent->edx |= HV_STIMER_DIRECT_MODE_AVAILABLE; > + > + /* > + * Direct Synthetic timers only make sense with in-kernel > + * LAPIC > + */ > + if (lapic_in_kernel(vcpu)) > + ent->edx |= HV_STIMER_DIRECT_MODE_AVAILABLE; > > break; Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
On 16/09/19 09:42, Wanpeng Li wrote: > From: Wanpeng Li <wanpengli@tencent.com> > > Reported by syzkaller: > > kasan: GPF could be caused by NULL-ptr deref or user memory access > general protection fault: 0000 [#1] PREEMPT SMP KASAN > RIP: 0010:__apic_accept_irq+0x46/0x740 arch/x86/kvm/lapic.c:1029 > Call Trace: > kvm_apic_set_irq+0xb4/0x140 arch/x86/kvm/lapic.c:558 > stimer_notify_direct arch/x86/kvm/hyperv.c:648 [inline] > stimer_expiration arch/x86/kvm/hyperv.c:659 [inline] > kvm_hv_process_stimers+0x594/0x1650 arch/x86/kvm/hyperv.c:686 > vcpu_enter_guest+0x2b2a/0x54b0 arch/x86/kvm/x86.c:7896 > vcpu_run+0x393/0xd40 arch/x86/kvm/x86.c:8152 > kvm_arch_vcpu_ioctl_run+0x636/0x900 arch/x86/kvm/x86.c:8360 > kvm_vcpu_ioctl+0x6cf/0xaf0 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2765 > > The testcase programs HV_X64_MSR_STIMERn_CONFIG/HV_X64_MSR_STIMERn_COUNT, > in addition, there is no lapic in the kernel, the counters value are small > enough in order that kvm_hv_process_stimers() inject this already-expired > timer interrupt into the guest through lapic in the kernel which triggers > the NULL deferencing. This patch fixes it by don't advertise direct mode > synthetic timers and discarding the inject when lapic is not in kernel. > > syzkaller source: https://syzkaller.appspot.com/x/repro.c?x=1752fe0a600000 > > Reported-by: syzbot+dff25ee91f0c7d5c1695@syzkaller.appspotmail.com > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Radim Krčmář <rkrcmar@redhat.com> > Cc: Vitaly Kuznetsov <vkuznets@redhat.com> > Signed-off-by: Wanpeng Li <wanpengli@tencent.com> > --- > v2 -> v3: > * add the link of syzkaller source > v1 -> v2: > * don't advertise direct mode synthetic timers when lapic is not in kernel > > arch/x86/kvm/hyperv.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c > index c10a8b1..069e655 100644 > --- a/arch/x86/kvm/hyperv.c > +++ b/arch/x86/kvm/hyperv.c > @@ -645,7 +645,9 @@ static int stimer_notify_direct(struct kvm_vcpu_hv_stimer *stimer) > .vector = stimer->config.apic_vector > }; > > - return !kvm_apic_set_irq(vcpu, &irq, NULL); > + if (lapic_in_kernel(vcpu)) > + return !kvm_apic_set_irq(vcpu, &irq, NULL); > + return 0; > } > > static void stimer_expiration(struct kvm_vcpu_hv_stimer *stimer) > @@ -1849,7 +1851,13 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid, > > ent->edx |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE; > ent->edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE; > - ent->edx |= HV_STIMER_DIRECT_MODE_AVAILABLE; > + > + /* > + * Direct Synthetic timers only make sense with in-kernel > + * LAPIC > + */ > + if (lapic_in_kernel(vcpu)) > + ent->edx |= HV_STIMER_DIRECT_MODE_AVAILABLE; > > break; > > Queued, thanks. Paolo
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c index c10a8b1..069e655 100644 --- a/arch/x86/kvm/hyperv.c +++ b/arch/x86/kvm/hyperv.c @@ -645,7 +645,9 @@ static int stimer_notify_direct(struct kvm_vcpu_hv_stimer *stimer) .vector = stimer->config.apic_vector }; - return !kvm_apic_set_irq(vcpu, &irq, NULL); + if (lapic_in_kernel(vcpu)) + return !kvm_apic_set_irq(vcpu, &irq, NULL); + return 0; } static void stimer_expiration(struct kvm_vcpu_hv_stimer *stimer) @@ -1849,7 +1851,13 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid, ent->edx |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE; ent->edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE; - ent->edx |= HV_STIMER_DIRECT_MODE_AVAILABLE; + + /* + * Direct Synthetic timers only make sense with in-kernel + * LAPIC + */ + if (lapic_in_kernel(vcpu)) + ent->edx |= HV_STIMER_DIRECT_MODE_AVAILABLE; break;