Linux Confidential Computing Development
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Ackerley Tng <ackerleytng@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Thomas Gleixner <tglx@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	 Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org,  "H. Peter Anvin" <hpa@zytor.com>,
	Kiryl Shutsemau <kas@kernel.org>,
	 Rick Edgecombe <rick.p.edgecombe@intel.com>,
	Vishal Annapurve <vannapurve@google.com>,
	 Yan Zhao <yan.y.zhao@intel.com>,
	Michael Roth <michael.roth@amd.com>,
	 Isaku Yamahata <isaku.yamahata@intel.com>,
	Chao Peng <chao.p.peng@linux.intel.com>,
	 Xiaoyao Li <xiaoyao.li@intel.com>,
	Zongyao Chen <ZongYao.Chen@linux.alibaba.com>,
	 kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-coco@lists.linux.dev,
	 Yu Zhang <yu.c.zhang@linux.intel.com>,
	Fuad Tabba <tabba@google.com>
Subject: Re: [PATCH v2 1/5] KVM: guest_memfd: Use write permissions when GUP-ing source pages
Date: Tue, 26 May 2026 09:13:21 -0700	[thread overview]
Message-ID: <ahXGoTaknK4KVuAz@google.com> (raw)
In-Reply-To: <20260522-fix-sev-gmem-post-populate-v2-1-3f196bfad5a1@google.com>

The shortlog is misleading, bordering on outright wrong.  I think most people
would read it as "ALWAYS Use write permissions when GUP-ing source pages".  I
also think it should be scoped to:

  KVM: SEV:

because this only affects SNP, and IMO is an SNP bug, not a guest_memfd bug.  E.g.

  KVM: SEV: Pin source page for write when adding CPUID data for SNP guest

On Fri, May 22, 2026, Ackerley Tng wrote:
> From: Sean Christopherson <seanjc@google.com>
> 
> sev_gmem_post_populate() may write to the source page if there was an error

Avoid referencing function names in changelogs when possible.  Unless the reader
is already familiar with the code, the name is meaningless.  The purpose of the
changelog is to complement the literal patch, not to provide a play-by-play
description.

> while performing SNP_LAUNCH_UPDATE.
> 
> Since GUP requested only reads, there is a chance sev_gmem_post_populate()
> could be writing to some read-only page.
> 
> sev_gmem_post_populate() will only ever write the source page if the type
> of page being LAUNCH_UPDATEd is a CPUID page. Hence, request a writable
> page only when loading the CPUID page.
> 
> Since TDX never writes to the source page, always pass false to
> kvm_gmem_populate().

Describe changes in human-friendly, conversational language.  And in a way that
doesn't require looking at the patch to understand the changelog: "pass false"
is meaningless without looking at the code to see what flag was added (or exists).

> With this, even if a read-only mapping or the global zero page was provided
> as the source page, GUP will do a copy-on-write, making it writable before
> the write happens in gvm_post_populate.

Objection, speculation.  If the mapping is truly read-only, i.e. doesn't allow
writes at all, then GUP will fail.  This is all superfluous information though;
"read-only" is a pretty ubiquitous concept, there's no need to explain it in
gory detail.


I'll rewrite to this when applying:

---
When populating a guest_memfd instance with the initial CPUID data for an
SNP guest, acquire a writable pin on the source page as KVM will write back
the "correct" CPUID information if the userspace provided data is rejected
by trusted firmware.  Because KVM writes to the source page using a kernel
mapping, pinning for read could result in KVM clobbering read-only memory.

Note, well-behaved VMMs are unlikely to be affected, as CPUID information
is almost always dynamically generated by userspace, i.e. it's unlikely for
the CPUID information to be backed by a read-only mapping.
---

> Fixes: 2a62345b30529 ("KVM: guest_memfd: GUP source pages prior to populating guest memory")
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Cc: stable@vger.kernel.org

> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---


  reply	other threads:[~2026-05-26 16:13 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-22 22:46 [PATCH v2 0/5] guest_memfd fixes for bind and populate Ackerley Tng via B4 Relay
2026-05-22 22:46 ` [PATCH v2 1/5] KVM: guest_memfd: Use write permissions when GUP-ing source pages Ackerley Tng via B4 Relay
2026-05-26 16:13   ` Sean Christopherson [this message]
2026-05-22 22:46 ` [PATCH v2 2/5] KVM: guest_memfd: Fix possible signed integer overflow Ackerley Tng via B4 Relay
2026-05-26 15:53   ` Sean Christopherson
2026-05-27 18:26     ` Ackerley Tng
2026-05-27 19:26       ` Sean Christopherson
2026-05-27 20:17         ` Ackerley Tng
2026-05-22 22:46 ` [PATCH v2 3/5] KVM: guest_memfd: Handle errors from xa_store_range() when binding Ackerley Tng via B4 Relay
2026-05-26 16:39   ` Sean Christopherson
2026-05-27 19:11     ` Ackerley Tng
2026-05-22 22:46 ` [PATCH v2 4/5] KVM: SNP: Fix kunmap_local() unmapping order Ackerley Tng via B4 Relay
2026-05-26 15:55   ` Sean Christopherson
2026-05-22 22:46 ` [PATCH v2 5/5] KVM: SNP: Mark source page dirty in sev_gmem_post_populate Ackerley Tng via B4 Relay
2026-05-26 16:47   ` Sean Christopherson
2026-05-27 19:14     ` Ackerley Tng
2026-05-26 16:55 ` [PATCH v2 0/5] guest_memfd fixes for bind and populate Sean Christopherson
2026-05-27 18:19 ` Sean Christopherson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ahXGoTaknK4KVuAz@google.com \
    --to=seanjc@google.com \
    --cc=ZongYao.Chen@linux.alibaba.com \
    --cc=ackerleytng@google.com \
    --cc=bp@alien8.de \
    --cc=chao.p.peng@linux.intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=isaku.yamahata@intel.com \
    --cc=kas@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-coco@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael.roth@amd.com \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=rick.p.edgecombe@intel.com \
    --cc=tabba@google.com \
    --cc=tglx@kernel.org \
    --cc=vannapurve@google.com \
    --cc=x86@kernel.org \
    --cc=xiaoyao.li@intel.com \
    --cc=yan.y.zhao@intel.com \
    --cc=yu.c.zhang@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox