Re: [PATCH v4 02/12] KVM: selftests: Fix RAX and RFLAGS VMCB offsets when running L2

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The hardcoded offsets in `LOAD_GPR_C` and `SAVE_GPR_C` are misaligned with `struct gpr64_regs` by 8 bytes, causing out-of-bounds memory corruption and shifting L1/L2 registers to incorrect offsets.
--

--- Patch [2]: [PATCH v4 02/12] KVM: selftests: Fix RAX and RFLAGS VMCB offsets when running L2 ---
commit 828b231275dca6c618d4ec1f5e43abd20a4b5e54
Author: Yosry Ahmed <yosry@kernel.org>

KVM: selftests: Fix RAX and RFLAGS VMCB offsets when running L2

This commit fixes incorrect vmcb_save_area offsets by using vmcb->save.rax
and vmcb->save.rflags as parameters to the assembly block. This avoids
hardcoding offsets completely and also directly uses guest_regs.rax instead
of assuming it is at offset 0.

Link: https://lore.kernel.org/all/20260521092311.86030-1-pbonzini@redhat.com/
Closes: https://sashiko.dev/#/patchset/20260518202514.2037078-1-yosry%40kernel.org?part=1

> diff --git a/tools/testing/selftests/kvm/lib/x86/svm.c b/tools/testing/selftests/kvm/lib/x86/svm.c
> index 1445b890986fd..766d15f1d534a 100644
> --- a/tools/testing/selftests/kvm/lib/x86/svm.c
> +++ b/tools/testing/selftests/kvm/lib/x86/svm.c
> @@ -164,19 +164,22 @@ void run_guest(struct vmcb *vmcb, u64 vmcb_gpa)
>  {
>  	asm volatile (
>  		"vmload %[vmcb_gpa]\n\t"
> -		"mov rflags, %%r15\n\t"	// rflags
> -		"mov %%r15, 0x170(%[vmcb])\n\t"
> -		"mov guest_regs, %%r15\n\t"	// rax
> -		"mov %%r15, 0x1f8(%[vmcb])\n\t"
> +		"mov rflags, %%r15\n\t"
> +		"mov %%r15, %[vmcb_rflags]\n\t"
> +		"mov %[guest_regs_rax], %%r15\n\t"
> +		"mov %%r15, %[vmcb_rax]\n\t"
>  		LOAD_GPR_C
>  		"vmrun %[vmcb_gpa]\n\t"
>  		SAVE_GPR_C

[Severity: High]
This isn't a bug introduced by this patch, but do the hardcoded memory
offsets in LOAD_GPR_C and SAVE_GPR_C cause an out-of-bounds memory access?

These macros use offsets that appear to be misaligned with the layout
of struct gpr64_regs by 8 bytes. For instance, SAVE_GPR_C exchanges
%r15 with guest_regs+0x80.

Since guest_regs has a total size of exactly 0x80 bytes, doesn't this
result in an 8-byte out-of-bounds memory write past the end of the
structure that could corrupt adjacent globals?

> -		"mov 0x170(%[vmcb]), %%r15\n\t"	// rflags
>  		"mov %%r15, rflags\n\t"
> -		"mov 0x1f8(%[vmcb]), %%r15\n\t"	// rax
> -		"mov %%r15, guest_regs\n\t"
> +		"mov %[vmcb_rax], %%r15\n\t"	// rax
> +		"mov %%r15, %[guest_regs_rax]\n\t"
>  		"vmsave %[vmcb_gpa]\n\t"
> -		: : [vmcb] "r" (vmcb), [vmcb_gpa] "a" (vmcb_gpa)
> +		: [vmcb_rflags] "+m" (vmcb->save.rflags),
> +		  [vmcb_rax] "+m" (vmcb->save.rax),
> +		  [guest_regs_rax] "+rm" (guest_regs.rax)
> +		: [vmcb_gpa] "a" (vmcb_gpa)
>  		: "r15", "memory");
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727235228.1007324-1-yosry@kernel.org?part=2