Re: [PATCH v15 16/23] KVM: selftests: Load per-vCPU guest stack in TDX boot parameters

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] kvm: selftests: OOB array access in tdx_vcpu_load_boot_parameters()
- [Medium] kvm: selftests: silent integer truncation bypasses bounds check in tdx_vcpu_load_boot_parameters()
--

commit cc6d621273aee216e856ad9b309ad6da83f69649
Author: Sagi Shahar <sagis@google.com>

KVM: selftests: Load per-vCPU guest stack in TDX boot parameters

This commit allocates a guest stack for each vCPU and records its GVA in
the TDX boot parameters region. This allows proper vCPU initialization.

> diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> index 40ee9b1951c28..270a4ee1b7ebc 100644
> --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
[ ... ]
> @@ -90,6 +90,18 @@ void tdx_vm_load_common_boot_parameters(struct kvm_vm *vm)
>  		    "gdt must be within 32-bit address space for 32-bit boot code");
>  }
>  
> +void tdx_vcpu_load_boot_parameters(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
> +{
> +	struct td_boot_parameters *params =
> +		addr_gpa2hva(vm, TD_BOOT_PARAMETERS_GPA);
> +	struct td_per_vcpu_parameters *vcpu_params =
> +		&params->per_vcpu[vcpu->id];

[Severity: Medium]
Will this lead to an out-of-bounds array access in
tdx_vcpu_load_boot_parameters() if a test uses a sparse vcpu->id?

The boot parameters region appears to allocate memory for exactly
nr_runnable_vcpus instances of struct td_per_vcpu_parameters in
tdx_vm_setup_boot_parameters_region():

    size_t boot_params_size = sizeof(struct td_boot_parameters) +
        nr_runnable_vcpus * sizeof(struct td_per_vcpu_parameters);

Since vcpu->id represents the KVM vCPU ID (which can be the APIC ID on x86)
and can be larger than or equal to nr_runnable_vcpus, could this write beyond
the allocated bounds and corrupt adjacent host memory and guest physical pages?

This could be triggered if a selftest creating a TDX VM specifies a sparse
vcpu->id that is larger than or equal to the total number of runnable vCPUs
passed to vm_create().

> +
> +	vcpu_params->esp_gva = kvm_allocate_vcpu_stack(vm);
> +	TEST_ASSERT(vcpu_params->esp_gva < (1ULL << 32),
> +		    "ESP GVA must be within 32-bit address space for 32-bit boot code");

[Severity: Medium]
Does this bounds check in tdx_vcpu_load_boot_parameters() actually work, or is
the value silently truncated before the check?

Because kvm_allocate_vcpu_stack() returns a 64-bit gva_t, assigning it
to vcpu_params->esp_gva truncates the value to 32 bits since esp_gva is
declared as a u32.

The TEST_ASSERT then evaluates the already truncated 32-bit value against
(1ULL << 32), which will always be true. Could this bypass the intended
safety check and cause silent stack corruption and test failures instead of
an explicit assertion failure?

This could be triggered if a selftest allocates enough memory such that the
guest stack virtual address exceeds 4GB.

> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001-tdx-selftests-v15-0-7c62a5d8a992@google.com?part=16