Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753028AbdHJSL4 (ORCPT ); Thu, 10 Aug 2017 14:11:56 -0400 Received: from mail-wm0-f52.google.com ([74.125.82.52]:38733 "EHLO mail-wm0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752740AbdHJSLy (ORCPT ); Thu, 10 Aug 2017 14:11:54 -0400 MIME-Version: 1.0 In-Reply-To: <883867aa-587a-c273-9fc0-86f4d2d39898@redhat.com> References: <1502343192-4749-1-git-send-email-wanpeng.li@hotmail.com> <883867aa-587a-c273-9fc0-86f4d2d39898@redhat.com> From: Jim Mattson Date: Thu, 10 Aug 2017 11:11:52 -0700 Message-ID: Subject: Re: [PATCH] KVM: X86: Fix residual mmio emulation request to userspace To: Paolo Bonzini Cc: Dmitry Vyukov , Wanpeng Li , LKML , KVM list , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Wanpeng Li Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v7AIC0QH003787 Content-Length: 5371 Lines: 154 I'm not convinced that this plugs all of the mmio_needed/pio.count leaks, but it's a start. Syzkaller will eventually let us know about the others. On Thu, Aug 10, 2017 at 7:23 AM, Paolo Bonzini wrote: > On 10/08/2017 16:09, Dmitry Vyukov wrote: >> On Thu, Aug 10, 2017 at 3:44 PM, Paolo Bonzini wrote: >>> On 10/08/2017 07:33, Wanpeng Li wrote: >>>> Reported by syzkaller: >>>> >>>> The kvm-intel.unrestricted_guest=0 >>>> >>>> WARNING: CPU: 5 PID: 1014 at /home/kernel/data/kvm/arch/x86/kvm//x86.c:7227 kvm_arch_vcpu_ioctl_run+0x38b/0x1be0 [kvm] >>>> CPU: 5 PID: 1014 Comm: warn_test Tainted: G W OE 4.13.0-rc3+ #8 >>>> RIP: 0010:kvm_arch_vcpu_ioctl_run+0x38b/0x1be0 [kvm] >>>> Call Trace: >>>> ? put_pid+0x3a/0x50 >>>> ? rcu_read_lock_sched_held+0x79/0x80 >>>> ? kmem_cache_free+0x2f2/0x350 >>>> kvm_vcpu_ioctl+0x340/0x700 [kvm] >>>> ? kvm_vcpu_ioctl+0x340/0x700 [kvm] >>>> ? __fget+0xfc/0x210 >>>> do_vfs_ioctl+0xa4/0x6a0 >>>> ? __fget+0x11d/0x210 >>>> SyS_ioctl+0x79/0x90 >>>> entry_SYSCALL_64_fastpath+0x23/0xc2 >>>> ? __this_cpu_preempt_check+0x13/0x20 >>>> >>>> The syszkaller folks reported a residual mmio emulation request to userspace >>>> due to vm86 fails to emulate inject real mode interrupt(fails to read CS) and >>>> incurs a triple fault. The vCPU returns to userspace with vcpu->mmio_needed == true >>>> and KVM_EXIT_SHUTDOWN exit reason. However, the syszkaller testcase constructs >>>> several threads to launch the same vCPU, the thread which lauch this vCPU after >>>> the thread whichs get the vcpu->mmio_needed == true and KVM_EXIT_SHUTDOWN will >>>> trigger the warning. >>>> >>>> #define _GNU_SOURCE >>>> #include >>>> #include >>>> #include >>>> #include >>>> #include >>>> #include >>>> #include >>>> #include >>>> #include >>>> #include >>>> #include >>>> #include >>>> >>>> int kvmcpu; >>>> struct kvm_run *run; >>>> >>>> void* thr(void* arg) >>>> { >>>> int res; >>>> res = ioctl(kvmcpu, KVM_RUN, 0); >>>> printf("ret1=%d exit_reason=%d suberror=%d\n", >>>> res, run->exit_reason, run->internal.suberror); >>>> return 0; >>>> } >>>> >>>> void test() >>>> { >>>> int i, kvm, kvmvm; >>>> pthread_t th[4]; >>>> >>>> kvm = open("/dev/kvm", O_RDWR); >>>> kvmvm = ioctl(kvm, KVM_CREATE_VM, 0); >>>> kvmcpu = ioctl(kvmvm, KVM_CREATE_VCPU, 0); >>>> run = (struct kvm_run*)mmap(0, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, kvmcpu, 0); >>>> srand(getpid()); >>>> for (i = 0; i < 4; i++) { >>>> pthread_create(&th[i], 0, thr, 0); >>>> usleep(rand() % 10000); >>>> } >>>> for (i = 0; i < 4; i++) >>>> pthread_join(th[i], 0); >>>> } >>>> >>>> int main() >>>> { >>>> for (;;) { >>>> int pid = fork(); >>>> if (pid < 0) >>>> exit(1); >>>> if (pid == 0) { >>>> test(); >>>> exit(0); >>>> } >>>> int status; >>>> while (waitpid(pid, &status, __WALL) != pid) {} >>>> } >>>> return 0; >>>> } >>>> >>>> This patch fixes it by resetting the vcpu->mmio_needed once we receive >>>> the triple fault to avoid the residue. >>>> >>>> Reported-by: Dmitry Vyukov >>>> Cc: Paolo Bonzini >>>> Cc: Radim Krčmář >>>> Cc: Dmitry Vyukov >>>> Signed-off-by: Wanpeng Li >>>> --- >>>> arch/x86/kvm/vmx.c | 1 + >>>> arch/x86/kvm/x86.c | 1 + >>>> 2 files changed, 2 insertions(+) >>>> >>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>>> index 8e4a2dc..77ab10b 100644 >>>> --- a/arch/x86/kvm/vmx.c >>>> +++ b/arch/x86/kvm/vmx.c >>>> @@ -5864,6 +5864,7 @@ static int handle_external_interrupt(struct kvm_vcpu *vcpu) >>>> static int handle_triple_fault(struct kvm_vcpu *vcpu) >>>> { >>>> vcpu->run->exit_reason = KVM_EXIT_SHUTDOWN; >>>> + vcpu->mmio_needed = 0; >>>> return 0; >>>> } >>>> >>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>>> index 72d82ab..1e143f7 100644 >>>> --- a/arch/x86/kvm/x86.c >>>> +++ b/arch/x86/kvm/x86.c >>>> @@ -6776,6 +6776,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) >>>> } >>>> if (kvm_check_request(KVM_REQ_TRIPLE_FAULT, vcpu)) { >>>> vcpu->run->exit_reason = KVM_EXIT_SHUTDOWN; >>>> + vcpu->mmio_needed = 0; >>>> r = 0; >>>> goto out; >>>> } >>>> >>> >>> >>> Queued, thanks. >> >> Hi Paolo, >> >> Where is it queued? I've checked >> git://git.kernel.org/pub/scm/virt/kvm/kvm.git >> {next,master,fixes,queue} and can't find it. > > I do run some tests before pushing. :) Sometimes I don't have time the > same day, so I just run kvm-unit-tests and push to the queue branch, > which is rebased. Sometimes I do, and it takes a few hours before it > ends up with a definitive SHA1 commit hash on the master and next branches. > > Today it's the latter, so you'll find it in a couple hours if everything > goes according to the plan. > > Paolo >