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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.