linux-kernel - Re: [PATCH v2] kvm/svm: PKU not currently supported

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191223152102.7wy5fxmxhkpooa7y@mojo.amd.com>
Date:   Mon, 23 Dec 2019 09:21:02 -0600
From:   John Allen <john.allen@....com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Sean Christopherson <sean.j.christopherson@...el.com>,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        rkrcmar@...hat.com, vkuznets@...hat.com
Subject: Re: [PATCH v2] kvm/svm: PKU not currently supported

On Fri, Dec 20, 2019 at 10:25:16AM +0100, Paolo Bonzini wrote:
> On 19/12/19 21:32, Sean Christopherson wrote:
> > On Thu, Dec 19, 2019 at 02:17:59PM -0600, John Allen wrote:
> >> Current SVM implementation does not have support for handling PKU. Guests
> >> running on a host with future AMD cpus that support the feature will read
> >> garbage from the PKRU register and will hit segmentation faults on boot as
> >> memory is getting marked as protected that should not be. Ensure that cpuid
> >> from SVM does not advertise the feature.
> >>
> >> Signed-off-by: John Allen <john.allen@....com>
> >> ---
> >> v2:
> >>   -Introduce kvm_x86_ops->pku_supported()
> > 
> > I like the v1 approach better, it's less code to unwind when SVM gains
> > support for virtualizaing PKU.
> > 
> > The existing cases of kvm_x86_ops->*_supported() in __do_cpuid_func() are
> > necessary to handle cases where it may not be possible to expose a feature
> > even though it's supported in hardware, host and KVM, e.g. VMX's separate
> > MSR-based features and PT's software control to hide it from guest.  In
> > this case, hiding PKU is purely due to lack of support in KVM.  The SVM
> > series to enable PKU can then delete a single line of SVM code instead of
> > having to go back in and do surgery on x86 and VMX.
> > 
> 
> I sort of liked the V1 approach better, in that I liked using
> set_supported_cpuid but I didn't like *removing* features from it.
> 
> I think all *_supported() should be removed, and the code moved from
> __do_cpuid_func() to set_supported_cpuid.
> 
> For now, however, this one is consistent with other features so I am
> applying it.

Hey Paolo,

If you haven't already applied this, would it be too much trouble to add a
fixes tag? If it's already applied, don't worry about it.

...
Fixes: 0556cbdc2fbc ("x86/pkeys: Don't check if PKRU is zero before writing it")

Thanks,
John

> 
> Paolo
>