linux-kernel - Re: [PATCH V4 0/1] KVM: TDX: Decrease TDX VM shutdown time

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: <aHGYvrdX4biqKYih@google.com>
Date: Fri, 11 Jul 2025 16:05:34 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Rick P Edgecombe <rick.p.edgecombe@...el.com>
Cc: Xiaoyao Li <xiaoyao.li@...el.com>, Chao Gao <chao.gao@...el.com>, 
	Kai Huang <kai.huang@...el.com>, 
	"binbin.wu@...ux.intel.com" <binbin.wu@...ux.intel.com>, Reinette Chatre <reinette.chatre@...el.com>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>, Adrian Hunter <adrian.hunter@...el.com>, 
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "pbonzini@...hat.com" <pbonzini@...hat.com>, 
	"tony.lindgren@...ux.intel.com" <tony.lindgren@...ux.intel.com>, Isaku Yamahata <isaku.yamahata@...el.com>, 
	Yan Y Zhao <yan.y.zhao@...el.com>
Subject: Re: [PATCH V4 0/1] KVM: TDX: Decrease TDX VM shutdown time

On Fri, Jul 11, 2025, Rick P Edgecombe wrote:
> On Fri, 2025-07-11 at 07:19 -0700, Sean Christopherson wrote:
> > --
> > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > index f4d4fd5cc6e8..9c2997665762 100644
> > --- a/arch/x86/kvm/vmx/tdx.c
> > +++ b/arch/x86/kvm/vmx/tdx.c
> > @@ -181,6 +181,8 @@ static int init_kvm_tdx_caps(const struct tdx_sys_info_td_conf *td_conf,
> >  {
> >         int i;
> >  
> > +       memset(caps->reserved, 0, sizeof(caps->reserved));
> > +
> >         caps->supported_attrs = tdx_get_supported_attrs(td_conf);
> >         if (!caps->supported_attrs)
> >                 return -EIO;
> > --
> 
> I started to try to help by chipping in a log for this, but I couldn't justify
> it very well. struct kvm_tdx_capabilities gets copied from userspace before
> being populated So a userspace that knows to look for something in the reserved
> area could know to zero it. If they left their own data in the reserved area,
> and then relied on that data to remain the same, and then we started setting a
> new field in it I guess it could disturb it. But that is strange, and I'm not
> sure it really reduces much risk. Anyway here is the attempt to justify it.
> 
> 
> KVM: TDX: Zero reserved reserved area in struct kvm_tdx_capabilities
> 
> Zero the reserved area in struct kvm_tdx_capabilities so that fields added in
> the reserved area won't disturb any userspace that previously had garbage there.

It's not only about disturbing userspace, it's also about actually being able to
repurpose the reserved fields in the future without needing *another* flag to tell
userspace that it's ok to read the previously-reserved fields.  I care about this
much more than I care about userspace using reserved fields as scratch space.

> struct kvm_tdx_capabilities holds information about the combined support of KVM
> and the TDX module. For future growth, there is an area of the struct marked as
> reserved. This way fields can be added into that space without increasing the
> size of the struct.
> 
> However, currently the reserved area is not zeroed, meaning any data that
> userspace left in the reserved area would be clobbered by a future field written
> in the reserved area. So zero the reserved area to reduce the risk that
> userspace might try to rely on some data there.