Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1708222 > unrolled thread

[PATCH] KVM: X86: Fix residual mmio emulation request to userspace

Started byWanpeng Li <kernellwp@gmail.com>
First post2017-08-10 07:40 +0200
Last post2017-08-10 20:20 +0200
Articles 6 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] KVM: X86: Fix residual mmio emulation request to userspace Wanpeng Li <kernellwp@gmail.com> - 2017-08-10 07:40 +0200
    Re: [PATCH] KVM: X86: Fix residual mmio emulation request to userspace Dmitry Vyukov <dvyukov@google.com> - 2017-08-10 14:40 +0200
    Re: [PATCH] KVM: X86: Fix residual mmio emulation request to  userspace Paolo Bonzini <pbonzini@redhat.com> - 2017-08-10 15:50 +0200
      Re: [PATCH] KVM: X86: Fix residual mmio emulation request to userspace Dmitry Vyukov <dvyukov@google.com> - 2017-08-10 16:10 +0200
        Re: [PATCH] KVM: X86: Fix residual mmio emulation request to  userspace Paolo Bonzini <pbonzini@redhat.com> - 2017-08-10 16:30 +0200
          Re: [PATCH] KVM: X86: Fix residual mmio emulation request to userspace Jim Mattson <jmattson@google.com> - 2017-08-10 20:20 +0200

#1708222 — [PATCH] KVM: X86: Fix residual mmio emulation request to userspace

FromWanpeng Li <kernellwp@gmail.com>
Date2017-08-10 07:40 +0200
Subject[PATCH] KVM: X86: Fix residual mmio emulation request to userspace
Message-ID<ucOml-1d2-3@gated-at.bofh.it>
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 <pthread.h>
   #include <stdio.h>
   #include <stdlib.h>
   #include <string.h>
   #include <sys/wait.h>
   #include <sys/types.h>
   #include <sys/stat.h>
   #include <sys/mman.h>
   #include <fcntl.h>
   #include <unistd.h>
   #include <linux/kvm.h>
   #include <stdio.h>
   
   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 <dvyukov@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 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;
 		}
-- 
2.7.4

[toc] | [next] | [standalone]


#1708585

FromDmitry Vyukov <dvyukov@google.com>
Date2017-08-10 14:40 +0200
Message-ID<ucUUO-5zH-11@gated-at.bofh.it>
In reply to#1708222
On Thu, Aug 10, 2017 at 7:33 AM, Wanpeng Li <kernellwp@gmail.com> 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 <pthread.h>
>    #include <stdio.h>
>    #include <stdlib.h>
>    #include <string.h>
>    #include <sys/wait.h>
>    #include <sys/types.h>
>    #include <sys/stat.h>
>    #include <sys/mman.h>
>    #include <fcntl.h>
>    #include <unistd.h>
>    #include <linux/kvm.h>
>    #include <stdio.h>
>
>    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.


Fixes the warning for me. Thanks for the quick fix!

Tested-by: Dmitry Vyukov <dvyukov@google.com>


> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  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;
>                 }
> --
> 2.7.4
>

[toc] | [prev] | [next] | [standalone]


#1708636 — Re: [PATCH] KVM: X86: Fix residual mmio emulation request to userspace

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-08-10 15:50 +0200
SubjectRe: [PATCH] KVM: X86: Fix residual mmio emulation request to userspace
Message-ID<ucW0y-6lE-3@gated-at.bofh.it>
In reply to#1708222
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 <pthread.h>
>    #include <stdio.h>
>    #include <stdlib.h>
>    #include <string.h>
>    #include <sys/wait.h>
>    #include <sys/types.h>
>    #include <sys/stat.h>
>    #include <sys/mman.h>
>    #include <fcntl.h>
>    #include <unistd.h>
>    #include <linux/kvm.h>
>    #include <stdio.h>
>    
>    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 <dvyukov@google.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  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.

Paolo

[toc] | [prev] | [next] | [standalone]


#1708664

FromDmitry Vyukov <dvyukov@google.com>
Date2017-08-10 16:10 +0200
Message-ID<ucWjU-6I0-15@gated-at.bofh.it>
In reply to#1708636
On Thu, Aug 10, 2017 at 3:44 PM, Paolo Bonzini <pbonzini@redhat.com> 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 <pthread.h>
>>    #include <stdio.h>
>>    #include <stdlib.h>
>>    #include <string.h>
>>    #include <sys/wait.h>
>>    #include <sys/types.h>
>>    #include <sys/stat.h>
>>    #include <sys/mman.h>
>>    #include <fcntl.h>
>>    #include <unistd.h>
>>    #include <linux/kvm.h>
>>    #include <stdio.h>
>>
>>    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 <dvyukov@google.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: Dmitry Vyukov <dvyukov@google.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>>  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.

[toc] | [prev] | [next] | [standalone]


#1708676 — Re: [PATCH] KVM: X86: Fix residual mmio emulation request to userspace

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-08-10 16:30 +0200
SubjectRe: [PATCH] KVM: X86: Fix residual mmio emulation request to userspace
Message-ID<ucWDg-6RR-15@gated-at.bofh.it>
In reply to#1708664
On 10/08/2017 16:09, Dmitry Vyukov wrote:
> On Thu, Aug 10, 2017 at 3:44 PM, Paolo Bonzini <pbonzini@redhat.com> 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 <pthread.h>
>>>    #include <stdio.h>
>>>    #include <stdlib.h>
>>>    #include <string.h>
>>>    #include <sys/wait.h>
>>>    #include <sys/types.h>
>>>    #include <sys/stat.h>
>>>    #include <sys/mman.h>
>>>    #include <fcntl.h>
>>>    #include <unistd.h>
>>>    #include <linux/kvm.h>
>>>    #include <stdio.h>
>>>
>>>    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 <dvyukov@google.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>>> Cc: Dmitry Vyukov <dvyukov@google.com>
>>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>>> ---
>>>  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

[toc] | [prev] | [next] | [standalone]


#1708951

FromJim Mattson <jmattson@google.com>
Date2017-08-10 20:20 +0200
Message-ID<ud0dQ-Jr-19@gated-at.bofh.it>
In reply to#1708676
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 <pbonzini@redhat.com> wrote:
> On 10/08/2017 16:09, Dmitry Vyukov wrote:
>> On Thu, Aug 10, 2017 at 3:44 PM, Paolo Bonzini <pbonzini@redhat.com> 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 <pthread.h>
>>>>    #include <stdio.h>
>>>>    #include <stdlib.h>
>>>>    #include <string.h>
>>>>    #include <sys/wait.h>
>>>>    #include <sys/types.h>
>>>>    #include <sys/stat.h>
>>>>    #include <sys/mman.h>
>>>>    #include <fcntl.h>
>>>>    #include <unistd.h>
>>>>    #include <linux/kvm.h>
>>>>    #include <stdio.h>
>>>>
>>>>    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 <dvyukov@google.com>
>>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>>>> Cc: Dmitry Vyukov <dvyukov@google.com>
>>>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>>>> ---
>>>>  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
>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web