Re: [PATCH v2 2/2] KVM: TDX: Return EINVAL, not EOPNOTSUPP, for NULL INIT_MEM_REGION source

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The `region.nr_pages << PAGE_SHIFT` calculation can overflow a 64-bit integer, bypassing GPA boundary validation and leading to a host denial of service.
--

commit a54da29c3e4be3226e4ebf9df2f4182c350df998
Author: Sean Christopherson <seanjc@google.com>

KVM: TDX: Return EINVAL, not EOPNOTSUPP, for NULL INIT_MEM_REGION source

Return EINVAL instead of EOPNOTSUPP if userspace attempts to pass a NULL
pointer for the source page of INIT_MEM_REGION, so that KVM's ABI is
consistent between TDX and SNP.

> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index ffe9d0db58c59..b0ec054732b90 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c

[ ... ]

> @@ -3247,8 +3244,8 @@ static int tdx_vcpu_init_mem_region(struct kvm_vcpu *vcpu, struct kvm_tdx_cmd *c
>  	if (copy_from_user(&region, u64_to_user_ptr(cmd->data), sizeof(region)))
>  		return -EFAULT;
>  
> -	if (!PAGE_ALIGNED(region.source_addr) || !PAGE_ALIGNED(region.gpa) ||
> -	    !region.nr_pages ||
> +	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 ||
>  	    !vt_is_tdx_private_gpa(kvm, region.gpa) ||
>  	    !vt_is_tdx_private_gpa(kvm, region.gpa + (region.nr_pages << PAGE_SHIFT) - 1))

[Severity: High]
This is a pre-existing issue, but in tdx_vcpu_init_mem_region(), does the
calculation region.nr_pages << PAGE_SHIFT overflow a 64-bit integer?

If userspace calls KVM_TDX_INIT_MEM_REGION with region.nr_pages set to a
value where the top 12 bits are non-zero (e.g., (1ULL << 52) + 1), the left
shift by PAGE_SHIFT discards the top 12 bits, evaluating to a small value.

Does this allow a malicious host userspace to bypass the overflow check
region.gpa + (region.nr_pages << PAGE_SHIFT) <= region.gpa and the
vt_is_tdx_private_gpa bounds check?

If bypassed, the while loop would then iterate based on the large un-shifted
region.nr_pages value. If a guest_memfd is mapped at a shared GPA and
initialization is started just before it, the loop could incrementally walk
into the shared GPA space.

Since KVM invokes tdh_mr_extend on that shared GPA, which the TDX module
rejects, could this trigger KVM's TDX_BUG_ON_2 and cause a host denial of
service?

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