Re: [PATCH] KVM: SEV: Allocate full pages for {DE,EN}CRYPT ops on SNP-enabled hosts

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] kmalloc(PAGE_SIZE) does not guarantee an isolated, page-aligned physical page under KASAN/SLUB debug, leading to Host RMP #PF crashes.

Pre-existing issues:
- [High] The PSP driver miscalculates the number of pages to transition to firmware-owned state for guest-owned buffers, causing firmware RMP violations.
--

--- Patch [1]: [PATCH] KVM: SEV: Allocate full pages for {DE,EN}CRYPT ops on SNP-enabled hosts ---
commit d5055bec1548a3803ac93e35043a743044a123b7
Author: Sean Christopherson <seanjc@google.com>

KVM: SEV: Allocate full pages for {DE,EN}CRYPT ops on SNP-enabled hosts

When {de,en}crypting memory of an SEV or SEV-ES guest on an SNP-enabled host
via a temporary buffer, this commit allocates a full 4KiB page for the buffer.
This ensures the page containing the buffer is wholly owned by KVM. It intends
to prevent concurrent kernel accesses from triggering spurious RMP #PF
violations when firmware temporarily takes ownership of the page.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 74fb15551e83f..e3733ed9d5886 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1280,6 +1280,17 @@ static void *sev_dbg_crypt_slow_alloc(struct page *page, unsigned long __va,
>  	if (WARN_ON_ONCE((*pa & PAGE_MASK) != ((*pa + *nr_bytes - 1) & PAGE_MASK)))
>  		return NULL;
>  
> +	/*
> +	 * If SNP is enabled, i.e. the RMP is active, allocate a full page to
> +	 * prevent concurrent accesses to the page.  As required by firmware,
> +	 * the PSP driver updates the RMP to temporarily transfer ownership of
> +	 * the page to Firmware while the {DE,EN}CRYPT operation is in-progress,
> +	 * and so concurrent software accesses to the page will encounter
> +	 * seemingly spurious RMP #PF violations
> +	 */
> +	if (cc_platform_has(CC_ATTR_HOST_SEV_SNP))
> +		return kmalloc(PAGE_SIZE, GFP_KERNEL);

[Severity: High]
Does kmalloc(PAGE_SIZE, GFP_KERNEL) guarantee an isolated, page-aligned
physical page when SLUB debugging (e.g., SLAB_RED_ZONE) or KASAN is enabled?

When debugging is enabled, kmalloc(PAGE_SIZE) requires extra metadata, pushing
the allocation into a higher-order slab cache (like kmalloc-8k). The resulting
buffer is not guaranteed to be page-aligned and shares its underlying physical
pages with adjacent slab objects, KASAN redzones, and SLUB metadata.

When sev_issue_dbg_cmd() passes this buffer as dst_addr, the PSP driver in
snp_map_cmd_buf_desc() transitions the page to firmware-owned. Concurrent
host kernel accesses to the shared page (e.g., KASAN background checks or
another CPU accessing adjacent objects) will trigger a fatal Host RMP #PF and
crash the host kernel.

Should this use a page allocation API like __get_free_page(GFP_KERNEL) or
alloc_page() instead to guarantee an exclusive physical page?

[Severity: High]
This is a pre-existing issue, but the PSP driver miscalculates the number of
pages to transition to firmware-owned state for guest-owned buffers in
snp_map_cmd_buf_desc():

drivers/crypto/ccp/sev-dev.c:snp_map_cmd_buf_desc() {
    ...
    npages = PAGE_ALIGN(desc->len) >> PAGE_SHIFT;
    if (rmp_mark_pages_firmware(*desc->paddr_ptr, npages, true)) {
    ...
}

Because this calculation ignores the physical address offset
(desc->paddr_ptr & ~PAGE_MASK), an unaligned buffer that crosses a page
boundary but has a total length <= PAGE_SIZE evaluates to 1 page instead of 2.

If the unaligned buffer from the KASAN-enabled kmalloc() above crosses a page
boundary, rmp_mark_pages_firmware() will only transition the first physical
page.

When the SEV firmware accesses the tail of the buffer on the untransitioned
second page, will this trigger a firmware RMP violation and cause the command
to fail?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723000715.1235773-1-seanjc@google.com?part=1