linux-kernel - Re: [RFC PATCH] KVM: TDX: Decouple TDX init mem region from kvm_gmem

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250711194952.ppzljx7sb6ouiwix@amd.com>
Date: Fri, 11 Jul 2025 14:49:52 -0500
From: Michael Roth <michael.roth@....com>
To: Vishal Annapurve <vannapurve@...gle.com>
CC: Sean Christopherson <seanjc@...gle.com>, Yan Zhao <yan.y.zhao@...el.com>,
	<pbonzini@...hat.com>, <kvm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<rick.p.edgecombe@...el.com>, <kai.huang@...el.com>,
	<adrian.hunter@...el.com>, <reinette.chatre@...el.com>,
	<xiaoyao.li@...el.com>, <tony.lindgren@...el.com>,
	<binbin.wu@...ux.intel.com>, <dmatlack@...gle.com>,
	<isaku.yamahata@...el.com>, <ira.weiny@...el.com>, <david@...hat.com>,
	<ackerleytng@...gle.com>, <tabba@...gle.com>, <chao.p.peng@...el.com>
Subject: Re: [RFC PATCH] KVM: TDX: Decouple TDX init mem region from
 kvm_gmem_populate()

On Fri, Jul 11, 2025 at 11:38:10AM -0700, Vishal Annapurve wrote:
> On Fri, Jul 11, 2025 at 9:37 AM Michael Roth <michael.roth@....com> wrote:
> >
> > >
> > > static long __kvm_gmem_populate(struct kvm *kvm, struct kvm_memory_slot *slot,
> > >                               struct file *file, gfn_t gfn, void __user *src,
> > >                               kvm_gmem_populate_cb post_populate, void *opaque)
> > > {
> > >       pgoff_t index = kvm_gmem_get_index(slot, gfn);
> > >       struct page *src_page = NULL;
> > >       bool is_prepared = false;
> > >       struct folio *folio;
> > >       int ret, max_order;
> > >       kvm_pfn_t pfn;
> > >
> > >       if (src) {
> > >               ret = get_user_pages((unsigned long)src, 1, 0, &src_page);
> > >               if (ret < 0)
> > >                       return ret;
> > >               if (ret != 1)
> > >                       return -ENOMEM;
> > >       }
> >
> > One tricky part here is that the uAPI currently expects the pages to
> > have the private attribute set prior to calling kvm_gmem_populate(),
> > which gets enforced below.
> >
> > For in-place conversion: the idea is that userspace will convert
> > private->shared to update in-place, then immediately convert back
> > shared->private; so that approach would remain compatible with above
> > behavior. But if we pass a 'src' parameter to kvm_gmem_populate(),
> > and do a GUP or copy_from_user() on it at any point, regardless if
> > it is is outside of filemap_invalidate_lock(), then
> > kvm_gmem_fault_shared() will return -EACCES.
> 
> I think that's a fine way to fail the initial memory population, this
> simply means userspace didn't pass the right source address. Why do we
> have to work around this error? Userspace should simply pass the
> source buffer that is accessible to the host or pass null to indicate
> that the target gfn already has the needed contents.
> 
> That is, userspace can still bring a separate source buffer even with
> in-place conversion available.

I thought there was some agreement that mmap() be the 'blessed'
approach for initializing memory with in-place conversion to help
untangle some of these paths, so it made sense to enforce that in
kvm_gmem_populate() to make it 'official', but with Sean's suggested
rework I suppose we could support both approaches.

-Mike

> 
> > The only 2 ways I see
> > around that are to either a) stop enforcing that pages that get
> > processed by kvm_gmem_populate() are private for in-place conversion
> > case, or b) enforce that 'src' is NULL for in-place conversion case.
> >