Re: [PATCH] KVM: TDX: Use check_shl_overflow() for nr_pages validation

[Sean Christopherson <seanjc@google.com>]

On Thu, Jul 02, 2026, Sunday Jiang wrote:
> From 4be3dcf1fca4acd2bf9c22bd604c913ba1774035 Mon Sep 17 00:00:00 2001
> From: Haotian Jiang <jianghaotian.sunday@gmail.com>
> Date: Thu, 2 Jul 2026 14:13:27 +0800
> Subject: [PATCH] KVM: TDX: Use check_shl_overflow() for nr_pages validation

This is all kinds of mangled, patches need to be sent as plaintext.

> The nr_pages field in struct kvm_tdx_init_mem_region is a u64 that comes
> directly from userspace via copy_from_user().  The current validation
> uses a manual overflow check:
> 
>     region.gpa + (region.nr_pages << PAGE_SHIFT) <= region.gpa
> 
> When nr_pages >= 2^52, the shift (nr_pages << PAGE_SHIFT) wraps around
> to a small value, bypassing the wrap check.  While downstream protections
> (gfn_to_memslot() returning NULL for GFNs outside any memslot, and
> kvm_slot_has_gmem() checking for NULL) prevent any actual out-of-bounds
> access, the overflow itself is a real bug that should be caught at the
> validation layer.
> 
> Replace the manual overflow check with check_shl_overflow() and
> check_add_overflow() to correctly detect the wrap-around, and use the
> computed end_gpa for the subsequent vt_is_tdx_private_gpa() check to
> avoid redundant and potentially overflowing arithmetic.
> 
> Signed-off-by: Haotian Jiang <jianghaotian.sunday@gmail.com>
> ---
>  arch/x86/kvm/vmx/tdx.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 989ab29b8c6f..9c6aa95ea1e1 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -3234,14 +3234,20 @@ static int tdx_vcpu_init_mem_region(struct kvm_vcpu
> *vcpu, struct kvm_tdx_cmd *c
>   if (cmd->flags & ~KVM_TDX_MEASURE_MEMORY_REGION)
>   return -EINVAL;
> 
> + u64 nr_bytes, end_gpa;

Declarations go at the top.  These should also be gpa_t, not raw u64.

>   if (copy_from_user(&region, u64_to_user_ptr(cmd->data), sizeof(region)))
>   return -EFAULT;
> 
> + if (check_shl_overflow(region.nr_pages, PAGE_SHIFT, &nr_bytes) ||

I much prefer to put these after the initial sanity checks.

> +    check_add_overflow(region.gpa, nr_bytes, &end_gpa))

Heh, the existing code is technically wrong, as it's ok if the last GPA is
-1ull.  Of course, that can't work in practice because x86 supports at most
MAXPHYADDR=52.

> + return -EINVAL;
> +
>   if (!PAGE_ALIGNED(region.source_addr) || !PAGE_ALIGNED(region.gpa) ||
>      !region.nr_pages ||
> -    region.gpa + (region.nr_pages << PAGE_SHIFT) <= region.gpa ||
> +    end_gpa <= region.gpa ||

This is unnecessary if we switch to check_add_overflow().

>      !vt_is_tdx_private_gpa(kvm, region.gpa) ||
> -    !vt_is_tdx_private_gpa(kvm, region.gpa + (region.nr_pages <<
> PAGE_SHIFT) - 1))
> +    !vt_is_tdx_private_gpa(kvm, end_gpa - 1))

And then if we fix the off-by-one "bug", this becomes:

	!vt_is_tdx_private_gpa(kvm, end_gpa)

Putting it all together:

	if (!PAGE_ALIGNED(region.source_addr) || !region.source_addr ||
	    !PAGE_ALIGNED(region.gpa) || !region.nr_pages)
		return -EINVAL;

	if (check_shl_overflow(region.nr_pages, PAGE_SHIFT, &nr_bytes) ||
	    check_add_overflow(region.gpa, nr_bytes - 1, &end_gpa) ||
	    !vt_is_tdx_private_gpa(kvm, region.gpa) ||
	    !vt_is_tdx_private_gpa(kvm, end_gpa))
		return -EINVAL;

I'll post a 2-patch v2 mini-series, I want to base this on the patch[*] where
Sashiko reported this.

[*] https://lore.kernel.org/all/20260630213711.479692-3-seanjc@google.com