linux-kernel - Re: [PATCH] KVM: x86: fix escape of guest dr6 to the host

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANRm+CwFsdKOiZhAwW4-2P6NptW4XJX-O5QqSxTnrtNw2c8-zw@mail.gmail.com>
Date:   Wed, 13 Dec 2017 17:58:28 +0800
From:   Wanpeng Li <kernellwp@...il.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        kvm <kvm@...r.kernel.org>,
        Radim Krčmář <rkrcmar@...hat.com>,
        David Hildenbrand <david@...hat.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Wanpeng Li <wanpeng.li@...mail.com>
Subject: Re: [PATCH] KVM: x86: fix escape of guest dr6 to the host

2017-12-13 17:53 GMT+08:00 Paolo Bonzini <pbonzini@...hat.com>:
> syzkaller reported:
>
>    WARNING: CPU: 0 PID: 12927 at arch/x86/kernel/traps.c:780 do_debug+0x222/0x250
>    CPU: 0 PID: 12927 Comm: syz-executor Tainted: G           OE    4.15.0-rc2+ #16
>    RIP: 0010:do_debug+0x222/0x250
>    Call Trace:
>     <#DB>
>     debug+0x3e/0x70
>    RIP: 0010:copy_user_enhanced_fast_string+0x10/0x20
>     </#DB>
>     _copy_from_user+0x5b/0x90
>     SyS_timer_create+0x33/0x80
>     entry_SYSCALL_64_fastpath+0x23/0x9a
>
> The testcase sets a watchpoint (with perf_event_open) on a buffer that is
> passed to timer_create() as the struct sigevent argument.  In timer_create(),
> copy_from_user()'s rep movsb triggers the BP.  The testcase also sets
> the debug registers for the guest.
>
> However, KVM only restores host debug registers when the host has active
> watchpoints, which triggers a race condition when running the testcase with
> multiple threads.  The guest's DR6.BS bit can escape to the host before
> another thread invokes timer_create(), and do_debug() complains.
>
> The fix is to respect do_debug()'s dr6 invariant when leaving KVM.
>
> Reported-by: Dmitry Vyukov <dvyukov@...gle.com>
> Cc: Paolo Bonzini <pbonzini@...hat.com>
> Cc: Radim Krčmář <rkrcmar@...hat.com>
> Cc: David Hildenbrand <david@...hat.com>
> Cc: Dmitry Vyukov <dvyukov@...gle.com>
> Reviewed-by: David Hildenbrand <david@...hat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@...mail.com>

Thanks for the help! :)

Regards,
Wanpeng Li

> ---
>  arch/x86/kvm/x86.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 9212dad176a7..eed73217e96f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2951,6 +2951,12 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>         pagefault_enable();
>         kvm_x86_ops->vcpu_put(vcpu);
>         vcpu->arch.last_host_tsc = rdtsc();
> +       /*
> +        * If userspace has set any breakpoints or watchpoints, dr6 is restored
> +        * on every vmexit, but if not, we might have a stale dr6 from the
> +        * guest. do_debug expects dr6 to be cleared after it runs, do the same.
> +        */
> +       set_debugreg(0, 6);
>  }
>
>  static int kvm_vcpu_ioctl_get_lapic(struct kvm_vcpu *vcpu,
> --
> 1.8.3.1
>