linux-kernel - Re: [PATCH v2] KVM: VMX: Fix rflags cache during vCPU reset

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: <CALMp9eT094vd2ukXOMXYnzrSwJS9FnjYAa5xjZSWeQ4vCvMX7Q@mail.gmail.com>
Date:   Tue, 5 Dec 2017 09:53:28 -0800
From:   Jim Mattson <jmattson@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Wanpeng Li <kernellwp@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        kvm <kvm@...r.kernel.org>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Wanpeng Li <wanpeng.li@...mail.com>,
        Nadav Amit <nadav.amit@...il.com>,
        Dmitry Vyukov <dvyukov@...gle.com>
Subject: Re: [PATCH v2] KVM: VMX: Fix rflags cache during vCPU reset

Sorry; I didn't mean to derail this patch thread. Setting bit 1 of
RFLAGS on CPU reset is clearly correct.

I was just noting that if syzkaller is complaining about illegal
RFLAGS, it's trivial for userspace to set RFLAGS to an illegal value.
User space can set all kinds of illegal RFLAGS state...bits 63:22, 15,
5, or 3 can be set; bit 1 can be cleared; RFLAGS.VM can be set in long
mode or real-address mode; RFLAGS.IF can be set while injecting an
interrupt via KVM_SET_VCPU_EVENTS. Let's not get hung up too much on
the one case of bit 1 being clear.

I don't think KVM_SET_REGS, KVM_SET_SREGS, KVM_SET_VCPU_EVENTS, and
friends should necessarily be in the validation business. There are
too many context-dependent validation checks that might pass or fail
depending on the order in which these ioctls are called. In a way,
these ioctls are comparable to the VMWRITE instruction, which does no
validity checking.

Better, perhaps, would be to defer the validity checks to KVM_VMRUN
(and let the hardware take care of them). Unfortunately, this doesn't
interact very well with emulate_invalid_guest_state = true and
enable_unrestricted_guest = false, in which case invalid guest state
is passed to the kernel emulator to see how well it deals with the
illegal state. (By the way, that's a really fun configuration for
syzkaller, because it opens the door to a myriad of kvm warnings that
are difficult to exercise otherwise).

So, how should illegal RFLAGS bits be handled by KVM_SET_REGS? Sure,
we could set bit 1, and we could clear bits 63:22, 15, 5, and 3. But
what about RFLAGS.VM and RFLAGS.IF?

On Tue, Dec 5, 2017 at 5:54 AM, Paolo Bonzini <pbonzini@...hat.com> wrote:
> On 05/12/2017 01:53, Wanpeng Li wrote:
>>> That seems like a convoluted path to produce an illegal RFLAGS value.
>>> What's to prevent syzkaller from simply clearing bit 1 of RFLAGS with
>>> the KVM_SET_REGS ioctl?
>> Yeah, it can happen. Which do you prefer, ioctl fails or |
>> X86_EFLAGS_FIXED unconditionally in the ioctl handler in kvm?
>
> I suspect somebody might be passing an all-zero regs struct to
> KVM_SET_REGS, so ORing X86_EFLAGS_FIXED is better.
>
> Thanks,
>
> Paolo