[PATCH v2 1/2] KVM: TDX: Reject INIT_MEM_REGION if number of bytes would overflow a u64

Sean Christopherson <[email protected]>
Newsgroups org.kernel.vger.kvm,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
From: Haotian Jiang <[email protected]>

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() to correctly
detect the wrap-around.

Note, the manual wrap-around check on the gpa+size technically has a benign
off-by-one bug, and can also use check_add_overflow().  Those flaws will be
addressed shortly.

Opportunistically separate the initial sanity checks from the more involved
checks to try and make the code easier to read.

Fixes: c846b451d3c5 ("KVM: TDX: Add an ioctl to create initial guest memory")
Reported-by: Sashiko Bot <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]
Cc: Yan Zhao <[email protected]>
Cc: Binbin Wu <[email protected]>
Cc: Ackerley Tng <[email protected]>
Signed-off-by: Haotian Jiang <[email protected]>
[sean: use gpa_t, isolate check_shl_overflow() change, tweak changelog]
Signed-off-by: Sean Christopherson <[email protected]>
---
 arch/x86/kvm/vmx/tdx.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index b272c20586a7..929115aeb9ec 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -3219,6 +3219,7 @@ static int tdx_vcpu_init_mem_region(struct kvm_vcpu *vcpu, struct kvm_tdx_cmd *c
 	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
 	struct kvm_tdx_init_mem_region region;
 	struct tdx_gmem_post_populate_arg arg;
+	gpa_t nr_bytes;
 	long gmem_ret;
 	int ret;
 
@@ -3236,10 +3237,13 @@ static int tdx_vcpu_init_mem_region(struct kvm_vcpu *vcpu, struct kvm_tdx_cmd *c
 		return -EFAULT;
 
 	if (!PAGE_ALIGNED(region.source_addr) || !region.source_addr ||
-	    !PAGE_ALIGNED(region.gpa) || !region.nr_pages ||
-	    region.gpa + (region.nr_pages << PAGE_SHIFT) <= region.gpa ||
+	    !PAGE_ALIGNED(region.gpa) || !region.nr_pages)
+		return -EINVAL;
+
+	if (check_shl_overflow(region.nr_pages, PAGE_SHIFT, &nr_bytes) ||
+	    region.gpa + nr_bytes <= region.gpa ||
 	    !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, region.gpa + nr_bytes - 1))
 		return -EINVAL;
 
 	ret = 0;
-- 
2.55.0.679.g6767b8d81c-goog
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.