linux-kernel - Re: [PATCH v3 00/24] KVM: TDX huge page support for private memory

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: <435b8d81-b4de-4933-b0ae-357dea311488@intel.com>
Date: Fri, 16 Jan 2026 08:57:05 -0800
From: Dave Hansen <dave.hansen@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Yan Zhao <yan.y.zhao@...el.com>, Ackerley Tng <ackerleytng@...gle.com>,
 Vishal Annapurve <vannapurve@...gle.com>, pbonzini@...hat.com,
 linux-kernel@...r.kernel.org, kvm@...r.kernel.org, x86@...nel.org,
 rick.p.edgecombe@...el.com, kas@...nel.org, tabba@...gle.com,
 michael.roth@....com, david@...nel.org, sagis@...gle.com, vbabka@...e.cz,
 thomas.lendacky@....com, nik.borisov@...e.com, pgonda@...gle.com,
 fan.du@...el.com, jun.miao@...el.com, francescolavra.fl@...il.com,
 jgross@...e.com, ira.weiny@...el.com, isaku.yamahata@...el.com,
 xiaoyao.li@...el.com, kai.huang@...el.com, binbin.wu@...ux.intel.com,
 chao.p.peng@...el.com, chao.gao@...el.com
Subject: Re: [PATCH v3 00/24] KVM: TDX huge page support for private memory

On 1/14/26 16:19, Sean Christopherson wrote:
>> 'struct page' gives us two things: One is the type safety, but I'm
>> pretty flexible on how that's implemented as long as it's not a raw u64
>> getting passed around everywhere.
> I don't necessarily disagree on the type safety front, but for the specific code
> in question, any type safety is a facade.  Everything leading up to the TDX code
> is dealing with raw PFNs and/or PTEs.  Then the TDX code assumes that the PFN
> being mapped into the guest is backed by a struct page, and that the folio size
> is consistent with @level, without _any_ checks whatsover.  This is providing
> the exact opposite of safety.
> 
>   static int tdx_mem_page_aug(struct kvm *kvm, gfn_t gfn,
> 			    enum pg_level level, kvm_pfn_t pfn)
>   {
> 	int tdx_level = pg_level_to_tdx_sept_level(level);
> 	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
> 	struct page *page = pfn_to_page(pfn);    <==================

I of course agree that this is fundamentally unsafe, it's just not
necessarily bad code.

I hope we both agree that this could be made _more_ safe by, for
instance, making sure the page is in a zone, pfn_valid(), and a few more
things.

In a perfect world, these conversions would happen at a well-defined
layer (KVM=>TDX) and in relatively few places. That layer transition is
where the sanity checks happen. It's super useful to have:

struct page *kvm_pfn_to_tdx_private_page(kvm_pfn_t pfn)
{
	struct page *page = pfn_to_page(pfn);
#ifdef DEBUG
	WARN_ON_ONCE(pfn_valid(pfn));
	// page must be from a "file"???
	WARN_ON_ONCE(!page_mapping(page));
	WARN_ON_ONCE(...);
#endif
	return page;
}

*EVEN* if the pfn_to_page() itself is unsafe, and even if the WARN()s
are compiled out, this explicitly lays out the assumptions and it means
someone reading TDX code has an easier idea comprehending it.

It's also not a crime to do the *same* checking on kvm_pfn_t and not
have a type transition. I just like the idea of changing the type so
that the transition line is clear and the concept is carried (forced,
even) through the layers of helpers.