linux-kernel - RE: [RFC PATCH v2 1/3] KVM: x86: only allow exits disable before vCPUs created

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: <DM6PR12MB3500C42C449D46AB22B7AB8ACA519@DM6PR12MB3500.namprd12.prod.outlook.com>
Date:   Tue, 11 Jan 2022 06:38:54 +0000
From:   Kechen Lu <kechenl@...dia.com>
To:     Sean Christopherson <seanjc@...gle.com>
CC:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "wanpengli@...cent.com" <wanpengli@...cent.com>,
        "vkuznets@...hat.com" <vkuznets@...hat.com>,
        "mst@...hat.com" <mst@...hat.com>,
        Somdutta Roy <somduttar@...dia.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH v2 1/3] KVM: x86: only allow exits disable before
 vCPUs created

Hi Sean,

> -----Original Message-----
> From: Sean Christopherson <seanjc@...gle.com>
> Sent: Monday, January 10, 2022 10:50 AM
> To: Kechen Lu <kechenl@...dia.com>
> Cc: kvm@...r.kernel.org; pbonzini@...hat.com; wanpengli@...cent.com;
> vkuznets@...hat.com; mst@...hat.com; Somdutta Roy
> <somduttar@...dia.com>; linux-kernel@...r.kernel.org
> Subject: Re: [RFC PATCH v2 1/3] KVM: x86: only allow exits disable before
> vCPUs created
> 
> External email: Use caution opening links or attachments
> 
> 
> On Tue, Dec 21, 2021, Kechen Lu wrote:
> > Since VMX and SVM both would never update the control bits if exits
> > are disable after vCPUs are created, only allow setting exits disable
> > flag before vCPU creation.
> >
> > Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> 
> For this to carry my SOB, I should be attributed as the author, or add a
> Co-developed-by: for me.  I'm also totally ok with a Suggested-by: or
> Reported-by:
> 

My apologies for putting incorrect SOB format :P Will fix it!

> And we should at least have
> 
>   Fixes: 4d5422cea3b6 ("KVM: X86: Provide a capability to disable MWAIT
> intercepts")
> 

Ack! Will mention it in the description.

> andy maybe Cc: stable@...r.kernel.org, though I'm not entirely sure this is
> stable material as it could in theory do more harm than good if there's a
> busted userspace out there.
> 

I see, will cc stable mailing list. IMO with this patch, incorrect behavior from userspace
only cause the set flag "ineffective", not sure if this breaks some userspace seriously.

Best Regards,
Kechen

> If this doesn't carry my SOB...
> 
> Reviewed-by: Sean Christopherson <seanjc@...gle.com>
> 
> > Signed-off-by: Kechen Lu <kechenl@...dia.com>
> > ---
> >  Documentation/virt/kvm/api.rst | 1 +
> >  arch/x86/kvm/x86.c             | 6 ++++++
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/Documentation/virt/kvm/api.rst
> > b/Documentation/virt/kvm/api.rst index aeeb071c7688..d1c50b95bbc1
> > 100644
> > --- a/Documentation/virt/kvm/api.rst
> > +++ b/Documentation/virt/kvm/api.rst
> > @@ -6581,6 +6581,7 @@ branch to guests' 0x200 interrupt vector.
> >  :Architectures: x86
> >  :Parameters: args[0] defines which exits are disabled
> >  :Returns: 0 on success, -EINVAL when args[0] contains invalid exits
> > +          or if any vCPU has already been created
> >
> >  Valid bits in args[0] are::
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index
> > 0cf1082455df..37529c0c279d 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -5764,6 +5764,10 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> >               if (cap->args[0] & ~KVM_X86_DISABLE_VALID_EXITS)
> >                       break;
> >
> > +             mutex_lock(&kvm->lock);
> > +             if (kvm->created_vcpus)
> > +                     goto disable_exits_unlock;
> > +
> >               if ((cap->args[0] & KVM_X86_DISABLE_EXITS_MWAIT) &&
> >                       kvm_can_mwait_in_guest())
> >                       kvm->arch.mwait_in_guest = true; @@ -5774,6
> > +5778,8 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> >               if (cap->args[0] & KVM_X86_DISABLE_EXITS_CSTATE)
> >                       kvm->arch.cstate_in_guest = true;
> >               r = 0;
> > +disable_exits_unlock:
> > +             mutex_unlock(&kvm->lock);
> >               break;
> >       case KVM_CAP_MSR_PLATFORM_INFO:
> >               kvm->arch.guest_can_read_msr_platform_info =
> > cap->args[0];
> > --
> > 2.30.2
> >