From: Sean Christopherson <seanjc@google.com>
To: Mathias Krause <minipli@grsecurity.net>
Cc: Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org
Subject: Re: [kvm-unit-tests PATCH] x86/emulator64: Extend non-canonical memory access tests with CR2 coverage
Date: Thu, 26 Jun 2025 07:49:18 -0700 [thread overview]
Message-ID: <aF1d7rh_vbr8cr7j@google.com> (raw)
In-Reply-To: <20250612141637.131314-1-minipli@grsecurity.net>
[-- Attachment #1: Type: text/plain, Size: 3659 bytes --]
On Thu, Jun 12, 2025, Mathias Krause wrote:
> Extend the non-canonical memory access tests to verify CR2 stays
> unchanged.
>
> There's currently a bug in QEMU/TCG that breaks that assumption.
>
> Link: https://gitlab.com/qemu-project/qemu/-/issues/928
> Signed-off-by: Mathias Krause <minipli@grsecurity.net>
> ---
> x86/emulator64.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/x86/emulator64.c b/x86/emulator64.c
> index 5d1bb0f06d4f..abef2bda29f1 100644
> --- a/x86/emulator64.c
> +++ b/x86/emulator64.c
> @@ -325,16 +325,39 @@ static void test_mmx_movq_mf(uint64_t *mem)
> report(exception_vector() == MF_VECTOR, "movq mmx generates #MF");
> }
>
> +#define CR2_REF_VALUE 0xdecafbadUL
> +
> +static void setup_cr2(void)
> +{
> + write_cr2(CR2_REF_VALUE);
> +}
> +
> +static void check_cr2(void)
> +{
> + unsigned long cr2 = read_cr2();
> +
> + if (cr2 == CR2_REF_VALUE) {
> + report(true, "CR2 unchanged");
> + } else {
> + report(false, "CR2 changed from %#lx to %#lx", CR2_REF_VALUE, cr2);
> + setup_cr2();
Writing CR2 isn't expensive in the grand scheme, so rather than conditionally
re-write CR2, I think it makes sense to write CR2 at the start of every testcase,
and then just do "report(cr2 == CR2_REF_VALUE".
> + }
> +}
> +
> static void test_jmp_noncanonical(uint64_t *mem)
> {
> + setup_cr2();
> *mem = NONCANONICAL;
> asm volatile (ASM_TRY("1f") "jmp *%0; 1:" : : "m"(*mem));
> report(exception_vector() == GP_VECTOR,
> "jump to non-canonical address");
> + check_cr2();
> }
>
> static void test_reg_noncanonical(void)
> {
> + setup_cr2();
> +
> /* RAX based, should #GP(0) */
> asm volatile(ASM_TRY("1f") "orq $0, (%[noncanonical]); 1:"
> : : [noncanonical]"a"(NONCANONICAL));
> @@ -342,6 +365,7 @@ static void test_reg_noncanonical(void)
> "non-canonical memory access, should %s(0), got %s(%u)",
> exception_mnemonic(GP_VECTOR),
> exception_mnemonic(exception_vector()), exception_error_code());
> + check_cr2();
And then rather than add more copy+paste, what if we add a macro to handle the
checks? Then the CR2 validation can slot in nicely (and maybe someday the macro
could be used outside of the x86/emulator64.c).
Attached patches yield:
#define CR2_REF_VALUE 0xdecafbadUL
#define ASM_TRY_NONCANONICAL(insn, inputs, access, ex_vector) \
do { \
unsigned int vector, ec; \
\
write_cr2(CR2_REF_VALUE); \
\
asm volatile(ASM_TRY("1f") insn "; 1:" :: inputs); \
\
vector = exception_vector(); \
ec = exception_error_code(); \
\
report(vector == ex_vector && !ec, \
"non-canonical " access ", should %s(0), got %s(%u)", \
exception_mnemonic(ex_vector), exception_mnemonic(vector), ec); \
\
if (vector != PF_VECTOR) { \
unsigned long cr2 = read_cr2(); \
\
report(cr2 == CR2_REF_VALUE, \
"Wanted CR2 '0x%lx', got '0x%lx", CR2_REF_VALUE, cr2); \
} \
} while (0)
static void test_jmp_noncanonical(uint64_t *mem)
{
*mem = NONCANONICAL;
ASM_TRY_NONCANONICAL("jmp *%0", "m"(*mem), "jmp", GP_VECTOR);
}
static void test_reg_noncanonical(void)
{
/* RAX based, should #GP(0) */
ASM_TRY_NONCANONICAL("orq $0, (%[nc])", [nc]"a"(NONCANONICAL),
"memory access", GP_VECTOR);
/* RSP based, should #SS(0) */
ASM_TRY_NONCANONICAL("orq $0, (%%rsp,%[nc],1)", [nc]"r"(NONCANONICAL),
"rsp-based access", SS_VECTOR);
/* RBP based, should #SS(0) */
ASM_TRY_NONCANONICAL("orq $0, (%%rbp,%[nc],1)", [nc]"r"(NONCANONICAL),
"rbp-based access", SS_VECTOR);
}
[-- Attachment #2: 0001-x86-emulator64-Add-macro-to-test-emulation-of-non-ca.patch --]
[-- Type: text/x-diff, Size: 3492 bytes --]
From 0a7ee3543ef899ea36614ddff56c306dd63c341c Mon Sep 17 00:00:00 2001
From: Sean Christopherson <seanjc@google.com>
Date: Thu, 26 Jun 2025 07:39:43 -0700
Subject: [PATCH 1/2] x86/emulator64: Add macro to test emulation of
non-canonical accesses
Add a macro to "try" and check a non-canonical access. In addition to
de-duplicating the checking logic, this will allow extending the logic to
verify that CR2 isn't incorrectly modified, e.g. on #GP/#SS.
No functional change intended (ignoring the newly added check on the error
code for the JMP case).
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
x86/emulator64.c | 43 ++++++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/x86/emulator64.c b/x86/emulator64.c
index 138903af..21df3b0a 100644
--- a/x86/emulator64.c
+++ b/x86/emulator64.c
@@ -325,39 +325,40 @@ static void test_mmx_movq_mf(uint64_t *mem)
report(exception_vector() == MF_VECTOR, "movq mmx generates #MF");
}
+#define ASM_TRY_NONCANONICAL(insn, inputs, access, ex_vector) \
+do { \
+ unsigned int vector, ec; \
+ \
+ asm volatile(ASM_TRY("1f") insn "; 1:" :: inputs); \
+ \
+ vector = exception_vector(); \
+ ec = exception_error_code(); \
+ \
+ report(vector == ex_vector && !ec, \
+ "non-canonical " access ", should %s(0), got %s(%u)", \
+ exception_mnemonic(ex_vector), exception_mnemonic(vector), ec); \
+} while (0)
+
static void test_jmp_noncanonical(uint64_t *mem)
{
*mem = NONCANONICAL;
- asm volatile (ASM_TRY("1f") "jmp *%0; 1:" : : "m"(*mem));
- report(exception_vector() == GP_VECTOR,
- "jump to non-canonical address");
+
+ ASM_TRY_NONCANONICAL("jmp *%0", "m"(*mem), "jmp", GP_VECTOR);
}
static void test_reg_noncanonical(void)
{
/* RAX based, should #GP(0) */
- asm volatile(ASM_TRY("1f") "orq $0, (%[noncanonical]); 1:"
- : : [noncanonical]"a"(NONCANONICAL));
- report(exception_vector() == GP_VECTOR && exception_error_code() == 0,
- "non-canonical memory access, should %s(0), got %s(%u)",
- exception_mnemonic(GP_VECTOR),
- exception_mnemonic(exception_vector()), exception_error_code());
+ ASM_TRY_NONCANONICAL("orq $0, (%[nc])", [nc]"a"(NONCANONICAL),
+ "memory access", GP_VECTOR);
/* RSP based, should #SS(0) */
- asm volatile(ASM_TRY("1f") "orq $0, (%%rsp,%[noncanonical],1); 1:"
- : : [noncanonical]"r"(NONCANONICAL));
- report(exception_vector() == SS_VECTOR && exception_error_code() == 0,
- "non-canonical rsp-based access, should %s(0), got %s(%u)",
- exception_mnemonic(SS_VECTOR),
- exception_mnemonic(exception_vector()), exception_error_code());
+ ASM_TRY_NONCANONICAL("orq $0, (%%rsp,%[nc],1)", [nc]"r"(NONCANONICAL),
+ "rsp-based access", SS_VECTOR);
/* RBP based, should #SS(0) */
- asm volatile(ASM_TRY("1f") "orq $0, (%%rbp,%[noncanonical],1); 1:"
- : : [noncanonical]"r"(NONCANONICAL));
- report(exception_vector() == SS_VECTOR && exception_error_code() == 0,
- "non-canonical rbp-based access, should %s(0), got %s(%u)",
- exception_mnemonic(SS_VECTOR),
- exception_mnemonic(exception_vector()), exception_error_code());
+ ASM_TRY_NONCANONICAL("orq $0, (%%rbp,%[nc],1)", [nc]"r"(NONCANONICAL),
+ "rbp-based access", SS_VECTOR);
}
static void test_movabs(uint64_t *mem)
base-commit: 525bdb5d65d51a367341f471eb1bcd505d73c51f
--
2.50.0.727.gbf7dc18ff4-goog
[-- Attachment #3: 0002-x86-emulator64-Extend-non-canonical-memory-access-te.patch --]
[-- Type: text/x-diff, Size: 1767 bytes --]
From efb11a007a0e041ef029753b0b98abe071008334 Mon Sep 17 00:00:00 2001
From: Mathias Krause <minipli@grsecurity.net>
Date: Thu, 26 Jun 2025 07:42:51 -0700
Subject: [PATCH 2/2] x86/emulator64: Extend non-canonical memory access tests
with CR2 coverage
Extend the non-canonical memory access tests to verify CR2 stays
unchanged.
There's currently a bug in QEMU/TCG that breaks that assumption.
Link: https://gitlab.com/qemu-project/qemu/-/issues/928
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
x86/emulator64.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/x86/emulator64.c b/x86/emulator64.c
index 21df3b0a..6a85122f 100644
--- a/x86/emulator64.c
+++ b/x86/emulator64.c
@@ -325,10 +325,14 @@ static void test_mmx_movq_mf(uint64_t *mem)
report(exception_vector() == MF_VECTOR, "movq mmx generates #MF");
}
+#define CR2_REF_VALUE 0xdecafbadUL
+
#define ASM_TRY_NONCANONICAL(insn, inputs, access, ex_vector) \
do { \
unsigned int vector, ec; \
\
+ write_cr2(CR2_REF_VALUE); \
+ \
asm volatile(ASM_TRY("1f") insn "; 1:" :: inputs); \
\
vector = exception_vector(); \
@@ -337,6 +341,13 @@ do { \
report(vector == ex_vector && !ec, \
"non-canonical " access ", should %s(0), got %s(%u)", \
exception_mnemonic(ex_vector), exception_mnemonic(vector), ec); \
+ \
+ if (vector != PF_VECTOR) { \
+ unsigned long cr2 = read_cr2(); \
+ \
+ report(cr2 == CR2_REF_VALUE, \
+ "Wanted CR2 '0x%lx', got '0x%lx", CR2_REF_VALUE, cr2); \
+ } \
} while (0)
static void test_jmp_noncanonical(uint64_t *mem)
--
2.50.0.727.gbf7dc18ff4-goog
next prev parent reply other threads:[~2025-06-26 14:49 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-12 14:16 [kvm-unit-tests PATCH] x86/emulator64: Extend non-canonical memory access tests with CR2 coverage Mathias Krause
2025-06-26 14:49 ` Sean Christopherson [this message]
2025-06-27 14:33 ` Mathias Krause
2025-11-18 22:39 ` Sean Christopherson
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=aF1d7rh_vbr8cr7j@google.com \
--to=seanjc@google.com \
--cc=kvm@vger.kernel.org \
--cc=minipli@grsecurity.net \
--cc=pbonzini@redhat.com \
/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