Re: [PATCH v7 21/42] KVM: TDX: Make source page optional for KVM_TDX_INIT_MEM_REGION

[Sean Christopherson <seanjc@google.com>]

On Fri, Jun 05, 2026, Ackerley Tng wrote:
> Michael Roth <michael.roth@amd.com> writes:
> 
> >
> > [...snip...]
> >
> >> > > When KVM_TDX_INIT_MEM_REGION is called with a NULL source_addr,
> >> > > __kvm_gmem_populate() attempts an in-place conversion. If the target
> >> > > guest_memfd folio is unpopulated, __kvm_gmem_get_pfn() allocates a new folio
> >> > > using GFP_HIGHUSER, which lacks __GFP_ZERO.
> >>
> >> For in-place conversion, if src is NULL, it's generally because src *is*
> >> the target and it makes no sense to copy the page to itself because
> >> userspace already initialized it, which means userspace should have already
> >> trigged the uptodate flag to be set via kvm_gmem_fault_user_mapping().
> >>
> >> So maybe to address the malicious case that Sashiko seems to be worried
> >> about (where VMM purposely doesn't touch/init the memory to try to leak data
> >> into the guest), we just need to error out if: !src && !folio_uptodate(folio)?
> >
> > Of course immediately after posting this I think of exceptional cases, like
> > SEV-SNP can set up pre-zero'd private guest pages via this path, where
> > userspace wouldn't necessarily have done any touching/init of the page
> > in advance.
> >
> 
> Are you referring to KVM_SEV_SNP_PAGE_TYPE_ZERO?
> 
> > BUT.... in that case it would be okay, because the kernel knows
> > everything would get zero'd.
> >
> > So maybe the post-populate callbacks need to be the ones that would need
> > to implement these checks, or some API to that gmem can do it on their
> > behalf...
> >
> > So maybe your proposed patch is the more straightforward fix for now.
> >
> 
> Is KVM_SEV_SNP_PAGE_TYPE_ZERO expected to be used for a large number
> pages? If not, I think we can defer this to a future optimization, like
> in snp_launch_update(), if KVM_SEV_SNP_PAGE_TYPE_ZERO, pass a flag
> through to __kvm_gmem_get_pfn() to skip zeroing?
> 
> For all other page types, __kvm_gmem_get_pfn() MUST zero to avoid the
> issue Sashiko pointed out, right?

Or we do as Mike suggested, and outright reject the populate() call.  But I think
for ABI purposes, zeroing memory is the right approach, otherwise we'll end up
with a discrepancy between the userfault path and populate().

As I mentioned in the cover letter, initial guest image will typically be a tiny
subset of guest memory, so I don't have any concerns with unnecessarily zeroing
a few pages (memory that is ultimately overwritten by userspace).

> Currently, kvm_gmem_fault_user_mapping() will zero, and then userspace
> presumably writes the entire page. Can't avoid zeroing to avoid leaking
> uninitialized memory.
> 
> To avoid that zeroing, I guess there's the future guest_memfd write()
> syscall too, userspace can write() the entire page - write in the kernel
> can avoid zeroing since it knows which parts of the page was written and
> set uptodate. Then, kvm_gmem_fault_user_mapping() won't zero since the
> page is uptodate, and populate will not zero since it was also uptodate.