linux-kernel - Re: [PATCH 0/5] Introduce a quirk to control memslot zap behavior

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: <ZnGa550k46ow2N3L@google.com>
Date: Tue, 18 Jun 2024 07:34:15 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Yan Zhao <yan.y.zhao@...el.com>
Cc: Rick P Edgecombe <rick.p.edgecombe@...el.com>, "pbonzini@...hat.com" <pbonzini@...hat.com>, 
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "sagis@...gle.com" <sagis@...gle.com>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "dmatlack@...gle.com" <dmatlack@...gle.com>, 
	Kai Huang <kai.huang@...el.com>, Isaku Yamahata <isaku.yamahata@...el.com>, 
	Erdem Aktas <erdemaktas@...gle.com>
Subject: Re: [PATCH 0/5] Introduce a quirk to control memslot zap behavior

On Mon, Jun 17, 2024, Yan Zhao wrote:
> On Fri, Jun 14, 2024 at 04:01:07AM +0800, Edgecombe, Rick P wrote:
> > On Thu, 2024-06-13 at 14:06 +0800, Yan Zhao wrote:
> > >       a) Add a condition for TDX VM type in kvm_arch_flush_shadow_memslot()
> > >          besides the testing of kvm_check_has_quirk(). It is similar to
> > >          "all new VM types have the quirk disabled". e.g.
> > > 
> > >          static inline bool kvm_memslot_flush_zap_all(struct kvm
> > > *kvm)                    
> > >         
> > > {                                                                             
> > >    
> > >               return kvm->arch.vm_type != KVM_X86_TDX_VM
> > > &&                               
> > >                      kvm_check_has_quirk(kvm,
> > > KVM_X86_QUIRK_SLOT_ZAP_ALL);                
> > >          }
> > >          
> > >       b) Init the disabled_quirks based on VM type in kernel, extend
> > >          disabled_quirk querying/setting interface to enforce the quirk to
> > >          be disabled for TDX.

There's also option:

            c) Init disabled_quirks based on VM type.

I.e. let userspace enable the quirk.  If the VMM wants to shoot its TDX VM guests,
then so be it.  That said, I don't like this option because it would create a very
bizarre ABI.

> > 
> > I'd prefer to go with option (a) here. Because we don't have any behavior
> > defined yet for KVM_X86_TDX_VM, we don't really need to "disable a quirk" of it.

I vote for (a) as well.

> > Instead we could just define KVM_X86_QUIRK_SLOT_ZAP_ALL to be about the behavior
> > of the existing vm_types. It would be a few lines of documentation to save
> > implementing and maintaining a whole interface with special logic for TDX. So to
> > me it doesn't seem worth it, unless there is some other user for a new more
> > complex quirk interface.
> What about introducing a forced disabled_quirk field?

Nah, it'd require manual opt-in for every VM type for almost no benefit.  In fact,
IMO the code itself would be a net negative versus:

		return kvm->arch.vm_type == KVM_X86_DEFAULT_VM &&
		       kvm_check_has_quirk(kvm, KVM_X86_QUIRK_SLOT_ZAP_ALL);

because explicitly checking for KVM_X86_DEFAULT_VM would directly match the
documentation (which would state that the quirk only applies to DEFAULT_VM).