Re: [kvm-unit-tests PATCH v3 01/17] x86/run_in_user: Add an "end branch" marker on the user_mode destination

Mathias Krause
2025-11-14T15:51:35Z

[-- Attachment #1: Type: text/plain, Size: 1746 bytes --]

On 14.11.25 01:12, Sean Christopherson wrote:
> Add an endbr64 at the user_mode "entry point" so that run_in_user() can be
> used when CET's Indirect Branch Tracking is enabled.

I don't think that's needed, as 'user_mode' is branched to via IRETQ and
that isn't covered by IBT -- nor is any other RET instruction.

>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> lib/x86/usermode.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/lib/x86/usermode.c b/lib/x86/usermode.c
> index c3ec0ad7..f4ba0af4 100644
> --- a/lib/x86/usermode.c
> +++ b/lib/x86/usermode.c
> @@ -68,6 +68,9 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
> "iretq\n"
>
> "user_mode:\n\t"
> +#ifdef __x86_64__
> + "endbr64\n\t"
> +#endif

The thing is, this ENDBR64 is actually masking a missing cleanup step in
the IBT tests. The first failing IBT test will make the CET_U IBT
tracker state get stuck in the WAIT_FOR_ENDBRANCH state. This means,
every time we return to userland (and thereby implicitly switching to
the CET_U state again), it wants to see an ENDBR64 first or it will
directly trigger a #CP(3) again. This ENDBR64 will please that demand
and make it transition to IDLE and allow executing the test. However,
it's really the old test that should have fixed the tracker state and
not a blanket ENDBR64 when entering usermode.

Attached is a patch on top of [1] that does that but is, admitted, a
little hacky and evolved. However, it shows that above ENDBR64 is, in
fact, not needed.

Thanks,
Mathias

[1]
https://lore.kernel.org/kvm/fc886a22-49f3-4627-8ba6-933099e7640d@grsecurity.net/

> /* Back up volatile registers before invoking func */
> "push %%rcx\n\t"
> "push %%rdx\n\t"

[-- Attachment #2: 0001-x86-cet-Reset-IBT-tracker-state-on-CP-violations.patch --]
[-- Type: text/x-patch, Size: 5355 bytes --]

From 92f2a8000b2c6954825bde451f311eb513cef0e8 Mon Sep 17 00:00:00 2001
From: Mathias Krause <minipli@grsecurity.net>
Date: Fri, 14 Nov 2025 16:45:54 +0100
Subject: [kvm-unit-tests PATCH] x86: cet: Reset IBT tracker state on #CP
violations

Reset the IBT tracker state back to IDLE on #CP violations to not
influence follow-up tests with a poisoned starting state.

Signed-off-by: Mathias Krause <minipli@grsecurity.net>
---
lib/x86/usermode.h | 12 ++++++++++--
lib/x86/usermode.c | 13 ++++++++-----
x86/cet.c | 25 ++++++++++++++++++++++---
3 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/lib/x86/usermode.h b/lib/x86/usermode.h
index 04e358e2a3a3..5d42f47ebbc5 100644
--- a/lib/x86/usermode.h
+++ b/lib/x86/usermode.h
@@ -20,11 +20,19 @@ typedef uint64_t (*usermode_func)(void);
* Supports running functions with up to 4 arguments.
* fault_vector: exception vector that might get thrown during the function.
* raised_vector: outputs true if exception occurred.
+ * ex_extra: additional handler to call when handling traps related to fault_vector
*
* returns: return value returned by function, or 0 if an exception occurred.
*/
+uint64_t run_in_user_ex(usermode_func func, unsigned int fault_vector,
+ uint64_t arg1, uint64_t arg2, uint64_t arg3,
+ uint64_t arg4, bool *raised_vector, handler ex_extra);
+
+static inline
uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
uint64_t arg1, uint64_t arg2, uint64_t arg3,
- uint64_t arg4, bool *raised_vector);
-
+ uint64_t arg4, bool *raised_vector)
+{
+ return run_in_user_ex(func, fault_vector, arg1, arg2, arg3, arg4, raised_vector, NULL);
+}
#endif
diff --git a/lib/x86/usermode.c b/lib/x86/usermode.c
index 69dd64ffbc90..305e823208df 100644
--- a/lib/x86/usermode.c
+++ b/lib/x86/usermode.c
@@ -21,12 +21,17 @@ static void restore_exec_to_jmpbuf(void)
longjmp(jmpbuf, 1);
}

+static handler extra_ex;
+
static void restore_exec_to_jmpbuf_exception_handler(struct ex_regs *regs)
{
this_cpu_write_exception_vector(regs->vector);
this_cpu_write_exception_rflags_rf((regs->rflags >> 16) & 1);
this_cpu_write_exception_error_code(regs->error_code);

+ if (extra_ex)
+ extra_ex(regs);
+
/* longjmp must happen after iret, so do not do it now. */
regs->rip = (unsigned long)&restore_exec_to_jmpbuf;
regs->cs = KERNEL_CS;
@@ -35,9 +40,9 @@ static void restore_exec_to_jmpbuf_exception_handler(struct ex_regs *regs)
#endif
}

-uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
+uint64_t run_in_user_ex(usermode_func func, unsigned int fault_vector,
uint64_t arg1, uint64_t arg2, uint64_t arg3,
- uint64_t arg4, bool *raised_vector)
+ uint64_t arg4, bool *raised_vector, handler ex_extra)
{
extern char ret_to_kernel;
volatile uint64_t rax = 0;
@@ -45,6 +50,7 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
handler old_ex;

*raised_vector = 0;
+ extra_ex = ex_extra;
set_idt_entry(RET_TO_KERNEL_IRQ, &ret_to_kernel, 3);
old_ex = handle_exception(fault_vector,
restore_exec_to_jmpbuf_exception_handler);
@@ -72,9 +78,6 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
"iretq\n"

"user_mode:\n\t"
-#ifdef __x86_64__
- "endbr64\n\t"
-#endif
/* Back up volatile registers before invoking func */
"push %%rcx\n\t"
"push %%rdx\n\t"
diff --git a/x86/cet.c b/x86/cet.c
index 801d8da6e929..bcf1ca6d740a 100644
--- a/x86/cet.c
+++ b/x86/cet.c
@@ -1,4 +1,3 @@
-
#include "libcflat.h"
#include "x86/desc.h"
#include "x86/processor.h"
@@ -192,6 +191,10 @@ static uint64_t cet_ibt_emulation(void)
#define CET_ENABLE_SHSTK BIT(0)
#define CET_ENABLE_IBT BIT(2)
#define CET_ENABLE_NOTRACK BIT(4)
+#define CET_IBT_SUPPRESS BIT(10)
+#define CET_IBT_TRACKER_STATE BIT(11)
+#define IBT_TRACKER_IDLE 0
+#define IBT_TRACKER_WAIT_FOR_ENDBRANCH BIT(11)

static void test_shstk(void)
{
@@ -244,6 +247,22 @@ static void test_shstk(void)
report(vector == GP_VECTOR, "MSR_IA32_PL3_SSP alignment test.");
}

+static void ibt_tracker_fixup(struct ex_regs *regs)
+{
+ u64 cet_u = rdmsr(MSR_IA32_U_CET);
+
+ /*
+ * Switch the IBT tracker state to IDLE to have a clean state for
+ * following tests.
+ */
+ if ((cet_u & CET_IBT_TRACKER_STATE) == IBT_TRACKER_WAIT_FOR_ENDBRANCH) {
+ cet_u &= ~IBT_TRACKER_WAIT_FOR_ENDBRANCH;
+ printf("CET: suppressing IBT WAIT_FOR_ENDBRANCH state at RIP: %lx\n",
+ regs->rip);
+ wrmsr(MSR_IA32_U_CET, cet_u);
+ }
+}
+
static void test_ibt(void)
{
uint64_t l;
@@ -257,12 +276,12 @@ static void test_ibt(void)
/* Enable indirect-branch tracking (notrack handling for jump tables) */
wrmsr(MSR_IA32_U_CET, CET_ENABLE_IBT | CET_ENABLE_NOTRACK);

- run_in_user(cet_ibt_func, CP_VECTOR, 0, 0, 0, 0, &rvc);
+ run_in_user_ex(cet_ibt_func, CP_VECTOR, 0, 0, 0, 0, &rvc, ibt_tracker_fixup);
report(rvc && exception_error_code() == CP_ERR_ENDBR,
"Indirect-branch tracking test");

if (is_fep_available &&
- ((l = run_in_user(cet_ibt_emulation, CP_VECTOR, 0, 0, 0, 0, &rvc)) || rvc))
+ ((l = run_in_user_ex(cet_ibt_emulation, CP_VECTOR, 0, 0, 0, 0, &rvc, ibt_tracker_fixup)) || rvc))
report_fail("Forced emulation with IBT generated %s(%u) at line %lu",
exception_mnemonic(exception_vector()),
exception_error_code(), l);
--
2.51.0