Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Yosry Ahmed" <yosry@kernel.org>
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH v2 03/10] KVM: selftests: Use an array for guest_regs (and fix offsets)
Date: Thu, 04 Jun 2026 20:44:15 +0000	[thread overview]
Message-ID: <20260604204415.9FE0F1F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260604203546.365658-4-yosry@kernel.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `guest_regs` array is under-allocated by one element, leading to an out-of-bounds write that clobbers adjacent memory when swapping the `r15` register.
--

commit 2b93b9d75a4b05f37bd5b56a9795d649f317160b
Author: Yosry Ahmed <yosry@kernel.org>

KVM: selftests: Use an array for guest_regs (and fix offsets)

This commit replaces the gpr64_regs struct with an array to eliminate
hardcoded offsets. It also renames the SAVE_GPR_C and LOAD_GPR_C macros 
to a single SVM_SWITCH_GPRS_ASM macro.

> diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> index 513e4a1075fac..ff0b270bfe27f 100644
> --- a/tools/testing/selftests/kvm/include/x86/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86/processor.h

[ ... ]

> +#define GUEST_REGS_R13	13
> +#define GUEST_REGS_R14	14
> +#define GUEST_REGS_R15	15
> +#define NR_GUEST_REGS	GUEST_REGS_R15

[Severity: Medium]
Does this under-allocate the guest_regs array?

GUEST_REGS_R15 evaluates to 15, making NR_GUEST_REGS 15. However, there are
16 registers in total when counting from index 0 (RAX) through 15 (R15).

>  
>  struct desc64 {
> diff --git a/tools/testing/selftests/kvm/lib/x86/svm.c b/tools/testing/selftests/kvm/lib/x86/svm.c
> index f9ea1a2aeb8bc..1bd111e8bed26 100644
> --- a/tools/testing/selftests/kvm/lib/x86/svm.c
> +++ b/tools/testing/selftests/kvm/lib/x86/svm.c
> @@ -13,7 +13,7 @@
>  
>  #define SEV_DEV_PATH "/dev/sev"
>  
> -struct gpr64_regs guest_regs;
> +u64 guest_regs[NR_GUEST_REGS];
>  u64 rflags;

[Severity: Medium]
Since NR_GUEST_REGS is 15, does this array allocation leave out space for the
final register?

[ ... ]

> +#define SVM_SWITCH_GPRS_ASM \
> +	GUEST_SWITCH_GPR_ASM(rbx, GUEST_REGS_RBX) \

[ ... ]

> +	GUEST_SWITCH_GPR_ASM(r14, GUEST_REGS_R14) \
> +	GUEST_SWITCH_GPR_ASM(r15, GUEST_REGS_R15)

[Severity: Medium]
Can this write to index 15 clobber adjacent memory?

Because the guest_regs array only has 15 elements, writing the r15 register
to GUEST_REGS_R15 (index 15) writes past the end of the array. This looks
like it will corrupt adjacent variables in memory, such as the rflags
variable defined right below it.

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

  reply	other threads:[~2026-06-04 20:44 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-04 20:35 [PATCH v2 00/10] KVM: selftests: Stress save+restore and #PF (ft. nested) Yosry Ahmed
2026-06-04 20:35 ` [PATCH v2 01/10] KVM: selftests: Move STR() and XSTR() definitions to test_util.h Yosry Ahmed
2026-06-04 20:35 ` [PATCH v2 02/10] KVM: selftests: Fix RAX and RFLAGS VMCB offsets when running L2 Yosry Ahmed
2026-06-04 20:35 ` [PATCH v2 03/10] KVM: selftests: Use an array for guest_regs (and fix offsets) Yosry Ahmed
2026-06-04 20:44   ` sashiko-bot [this message]
2026-06-04 20:49     ` Yosry Ahmed
2026-06-04 21:37       ` Yosry Ahmed
2026-06-04 20:35 ` [PATCH v2 04/10] KVM: selftests: Move GPR load/save definitions outside of nSVM code Yosry Ahmed
2026-06-04 20:47   ` sashiko-bot
2026-06-04 20:35 ` [PATCH v2 05/10] KVM: selftests: Reuse GPR switching logic for nVMX Yosry Ahmed
2026-06-04 20:52   ` sashiko-bot
2026-06-04 20:35 ` [PATCH v2 06/10] KVM: selftests: Drop HORRIFIC_L2_UCALL_CLOBBER_HACK Yosry Ahmed
2026-06-04 20:50   ` sashiko-bot
2026-06-04 21:11     ` Yosry Ahmed
2026-06-04 20:35 ` [PATCH v2 07/10] KVM: selftests: Add basic stress test for save+restore and #PF handling Yosry Ahmed
2026-06-05 16:31   ` Yosry Ahmed
2026-06-04 20:35 ` [PATCH v2 08/10] KVM: selftests: Trigger save+restore randomly in the #PF stress test Yosry Ahmed
2026-06-04 20:49   ` sashiko-bot
2026-06-04 20:55     ` Yosry Ahmed
2026-06-04 20:35 ` [PATCH v2 09/10] KVM: selftests: Support running stress save+restore and #PF test in L2 Yosry Ahmed
2026-06-04 20:35 ` [PATCH v2 10/10] KVM: selftests: Trigger L2->L1 exits stress save+restore and #PF test Yosry Ahmed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260604204415.9FE0F1F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=yosry@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox