Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752847AbdLERxe (ORCPT ); Tue, 5 Dec 2017 12:53:34 -0500 Received: from mail-it0-f67.google.com ([209.85.214.67]:36239 "EHLO mail-it0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752579AbdLERxa (ORCPT ); Tue, 5 Dec 2017 12:53:30 -0500 X-Google-Smtp-Source: AGs4zMYm1ysZpe2L9qKmqeJ3LSNfrVxhhZDrwExUUyBY2hke00NRY0wSMIxCZ1CPS2XWdk7zWJa4QXRVaL+uT/TmE6k= MIME-Version: 1.0 In-Reply-To: <049b65c8-63a2-4c67-b66f-3b063038e3f8@redhat.com> References: <1511218341-6221-1-git-send-email-wanpeng.li@hotmail.com> <049b65c8-63a2-4c67-b66f-3b063038e3f8@redhat.com> From: Jim Mattson Date: Tue, 5 Dec 2017 09:53:28 -0800 Message-ID: Subject: Re: [PATCH v2] KVM: VMX: Fix rflags cache during vCPU reset To: Paolo Bonzini Cc: Wanpeng Li , "linux-kernel@vger.kernel.org" , kvm , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Wanpeng Li , Nadav Amit , Dmitry Vyukov 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-Length: 2167 Lines: 45 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 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