Re: [PATCH v2 04/10] KVM: selftests: Move GPR load/save definitions outside of nSVM code

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `guest_regs` array is incorrectly sized (15 instead of 16), leading to an out-of-bounds memory access when saving/restoring the R15 register.
--

commit 776bec433fbb092e9b3d73813e7a34673cb5fbbb
Author: Yosry Ahmed <yosry@kernel.org>

KVM: selftests: Move GPR load/save definitions outside of nSVM code

This commit moves the guest_regs array declaration and the GPR switching
macro to processor.h. This is done in preparation for reusing the code for
nVMX tests.

> diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> index ff0b270bfe27f..57f81f0bcc42d 100644
> --- a/tools/testing/selftests/kvm/include/x86/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86/processor.h
> @@ -414,6 +414,11 @@ static inline unsigned int x86_model(unsigned int eax)
>  #define GUEST_REGS_R15	15
>  #define NR_GUEST_REGS	GUEST_REGS_R15
>  
> +extern u64 guest_regs[NR_GUEST_REGS];
> +
> +#define GUEST_SWITCH_GPR_ASM(reg, idx) \
> +	"xchg %%" #reg ", guest_regs + 8 *" XSTR(idx) "\n\t"

[Severity: Medium]
Since NR_GUEST_REGS is defined as GUEST_REGS_R15 (15), does this allocate
guest_regs with only 15 elements (indices 0 to 14)?

If so, won't saving and restoring the R15 register use index 15, which
evaluates to guest_regs + 120, resulting in an 8-byte out-of-bounds read/write?

This off-by-one error was introduced earlier in the patch series when replacing
the 16-element gpr64_regs struct with an array. Moving guest_regs to
processor.c in this patch alters the memory layout. Previously, the
out-of-bounds access silently corrupted the adjacently declared rflags variable
(which was safely overwritten immediately afterward), but now it will silently
corrupt whichever variable immediately follows guest_regs in the .bss section.

> diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
> index 4ca48de7a926d..44a7233d69900 100644
> --- a/tools/testing/selftests/kvm/lib/x86/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86/processor.c
[ ... ]
> @@ -29,6 +29,8 @@ bool host_cpu_is_amd_compatible;
>  bool is_forced_emulation_enabled;
>  u64 guest_tsc_khz;
>  
> +u64 guest_regs[NR_GUEST_REGS];
> +
>  const char *ex_str(int vector)

> diff --git a/tools/testing/selftests/kvm/lib/x86/svm.c b/tools/testing/selftests/kvm/lib/x86/svm.c
> index 1bd111e8bed26..6528a6416c178 100644
> --- a/tools/testing/selftests/kvm/lib/x86/svm.c
> +++ b/tools/testing/selftests/kvm/lib/x86/svm.c
[ ... ]
> @@ -13,7 +13,6 @@
>  
>  #define SEV_DEV_PATH "/dev/sev"
>  
> -u64 guest_regs[NR_GUEST_REGS];
>  u64 rflags;
>  
>  /* Allocate memory regions for nested SVM tests.

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