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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAGtprH9dCCxK=GwVZTUKCeERQGbYD78-t4xDzQprmwtGxDoZXw@mail.gmail.com>
Date: Fri, 11 Jul 2025 11:38:10 -0700
From: Vishal Annapurve <vannapurve@...gle.com>
To: Michael Roth <michael.roth@....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 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.

> 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.
>