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 <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Radim Krčmář <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
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
On Thu, Aug 10, 2017 at 7:33 AM, Wanpeng Li <[email protected]> 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 <[email protected]>
> Reported-by: Dmitry Vyukov <[email protected]>
> Cc: Paolo Bonzini <[email protected]>
> Cc: Radim Krčmář <[email protected]>
> Cc: Dmitry Vyukov <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> 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
>
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 <[email protected]>
> Cc: Paolo Bonzini <[email protected]>
> Cc: Radim Krčmář <[email protected]>
> Cc: Dmitry Vyukov <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> 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
On Thu, Aug 10, 2017 at 3:44 PM, Paolo Bonzini <[email protected]> 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 <[email protected]>
>> Cc: Paolo Bonzini <[email protected]>
>> Cc: Radim Krčmář <[email protected]>
>> Cc: Dmitry Vyukov <[email protected]>
>> Signed-off-by: Wanpeng Li <[email protected]>
>> ---
>> 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.
On 10/08/2017 16:09, Dmitry Vyukov wrote:
> On Thu, Aug 10, 2017 at 3:44 PM, Paolo Bonzini <[email protected]> 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 <[email protected]>
>>> Cc: Paolo Bonzini <[email protected]>
>>> Cc: Radim Krčmář <[email protected]>
>>> Cc: Dmitry Vyukov <[email protected]>
>>> Signed-off-by: Wanpeng Li <[email protected]>
>>> ---
>>> 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
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 <[email protected]> wrote:
> On 10/08/2017 16:09, Dmitry Vyukov wrote:
>> On Thu, Aug 10, 2017 at 3:44 PM, Paolo Bonzini <[email protected]> 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 <[email protected]>
>>>> Cc: Paolo Bonzini <[email protected]>
>>>> Cc: Radim Krčmář <[email protected]>
>>>> Cc: Dmitry Vyukov <[email protected]>
>>>> Signed-off-by: Wanpeng Li <[email protected]>
>>>> ---
>>>> 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
>