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
next prev parent 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