[v6,27/33] x86/fred: fixup fault on ERETU by jumping to fred_entrypoint_user

Message ID 20230327075838.5403-28-xin3.li@intel.com
State New
Headers
Series x86: enable FRED for x86-64 |

Commit Message

Li, Xin3 March 27, 2023, 7:58 a.m. UTC
  If the stack frame contains an invalid user context (e.g. due to invalid SS,
a non-canonical RIP, etc.) the ERETU instruction will trap (#SS or #GP).

From a Linux point of view, this really should be considered a user space
failure, so use the standard fault fixup mechanism to intercept the fault,
fix up the exception frame, and redirect execution to fred_entrypoint_user.
The end result is that it appears just as if the hardware had taken the
exception immediately after completing the transition to user space.

Suggested-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Tested-by: Shan Kang <shan.kang@intel.com>
Signed-off-by: Xin Li <xin3.li@intel.com>
---

Changes since v5:
* Move the NMI bit from an invalid stack frame, which caused ERETU to fault,
  to the fault handler's stack frame, thus to unblock NMI ASAP if NMI is blocked
  (Lai Jiangshan).
---
 arch/x86/entry/entry_64_fred.S             |  8 +++--
 arch/x86/include/asm/extable_fixup_types.h |  4 ++-
 arch/x86/mm/extable.c                      | 36 ++++++++++++++++++++++
 3 files changed, 45 insertions(+), 3 deletions(-)
  

Comments

Lai Jiangshan March 31, 2023, 8:03 a.m. UTC | #1
On Mon, Mar 27, 2023 at 4:24 PM Xin Li <xin3.li@intel.com> wrote:
>
> If the stack frame contains an invalid user context (e.g. due to invalid SS,
> a non-canonical RIP, etc.) the ERETU instruction will trap (#SS or #GP).
>
> From a Linux point of view, this really should be considered a user space
> failure, so use the standard fault fixup mechanism to intercept the fault,
> fix up the exception frame, and redirect execution to fred_entrypoint_user.
> The end result is that it appears just as if the hardware had taken the
> exception immediately after completing the transition to user space.
>
> Suggested-by: H. Peter Anvin (Intel) <hpa@zytor.com>
> Tested-by: Shan Kang <shan.kang@intel.com>
> Signed-off-by: Xin Li <xin3.li@intel.com>
> ---
>
> Changes since v5:
> * Move the NMI bit from an invalid stack frame, which caused ERETU to fault,
>   to the fault handler's stack frame, thus to unblock NMI ASAP if NMI is blocked
>   (Lai Jiangshan).
> ---
>  arch/x86/entry/entry_64_fred.S             |  8 +++--
>  arch/x86/include/asm/extable_fixup_types.h |  4 ++-
>  arch/x86/mm/extable.c                      | 36 ++++++++++++++++++++++
>  3 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
> index d975cacd060f..efe2bcd11273 100644
> --- a/arch/x86/entry/entry_64_fred.S
> +++ b/arch/x86/entry/entry_64_fred.S
> @@ -5,8 +5,10 @@
>   * The actual FRED entry points.
>   */
>  #include <linux/linkage.h>
> -#include <asm/errno.h>
> +#include <asm/asm.h>
>  #include <asm/asm-offsets.h>
> +#include <asm/errno.h>
> +#include <asm/export.h>
>  #include <asm/fred.h>
>
>  #include "calling.h"
> @@ -38,7 +40,9 @@ SYM_CODE_START_NOALIGN(fred_entrypoint_user)
>         call    fred_entry_from_user
>  SYM_INNER_LABEL(fred_exit_user, SYM_L_GLOBAL)
>         FRED_EXIT
> -       ERETU
> +1:     ERETU
> +
> +       _ASM_EXTABLE_TYPE(1b, fred_entrypoint_user, EX_TYPE_ERETU)
>  SYM_CODE_END(fred_entrypoint_user)
>
>  .fill fred_entrypoint_kernel - ., 1, 0xcc
> diff --git a/arch/x86/include/asm/extable_fixup_types.h b/arch/x86/include/asm/extable_fixup_types.h
> index 991e31cfde94..1585c798a02f 100644
> --- a/arch/x86/include/asm/extable_fixup_types.h
> +++ b/arch/x86/include/asm/extable_fixup_types.h
> @@ -64,6 +64,8 @@
>  #define        EX_TYPE_UCOPY_LEN4              (EX_TYPE_UCOPY_LEN | EX_DATA_IMM(4))
>  #define        EX_TYPE_UCOPY_LEN8              (EX_TYPE_UCOPY_LEN | EX_DATA_IMM(8))
>
> -#define EX_TYPE_ZEROPAD                        20 /* longword load with zeropad on fault */
> +#define        EX_TYPE_ZEROPAD                 20 /* longword load with zeropad on fault */
> +
> +#define        EX_TYPE_ERETU                   21
>
>  #endif
> diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
> index 60814e110a54..a5d75b27a993 100644
> --- a/arch/x86/mm/extable.c
> +++ b/arch/x86/mm/extable.c
> @@ -6,6 +6,7 @@
>  #include <xen/xen.h>
>
>  #include <asm/fpu/api.h>
> +#include <asm/fred.h>
>  #include <asm/sev.h>
>  #include <asm/traps.h>
>  #include <asm/kdebug.h>
> @@ -195,6 +196,37 @@ static bool ex_handler_ucopy_len(const struct exception_table_entry *fixup,
>         return ex_handler_uaccess(fixup, regs, trapnr);
>  }
>
> +#ifdef CONFIG_X86_FRED
> +static bool ex_handler_eretu(const struct exception_table_entry *fixup,
> +                            struct pt_regs *regs, unsigned long error_code)
> +{
> +       struct pt_regs *uregs = (struct pt_regs *)(regs->sp - offsetof(struct pt_regs, ip));
> +       unsigned short ss = uregs->ss;
> +       unsigned short cs = uregs->cs;
> +
> +       /*
> +        * Move the NMI bit from the invalid stack frame, which caused ERETU
> +        * to fault, to the fault handler's stack frame, thus to unblock NMI
> +        * with the fault handler's ERETS instruction ASAP if NMI is blocked.
> +        */
> +       regs->nmi = uregs->nmi;
> +
> +       fred_info(uregs)->edata = fred_event_data(regs);
> +       uregs->ssx = regs->ssx;
> +       uregs->ss = ss;
> +       uregs->csx = regs->csx;
> +       uregs->nmi = 0; /* The NMI bit was moved away above */
> +       uregs->current_stack_level = 0;
> +       uregs->cs = cs;
> +
> +       /* Copy error code to uregs and adjust stack pointer accordingly */
> +       uregs->orig_ax = error_code;

The address of uregs->orig_ax is below regs->sp, so I think
some comments are needed here to state why it is safe to
write to uregs->orig_ax (a.k.a it is not verlapped with regs).



Thanks
Lai

> +       regs->sp -= 8;
> +
> +       return ex_handler_default(fixup, regs);
> +}
> +#endif
> +
>  int ex_get_fixup_type(unsigned long ip)
>  {
>         const struct exception_table_entry *e = search_exception_tables(ip);
> @@ -272,6 +304,10 @@ int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
>                 return ex_handler_ucopy_len(e, regs, trapnr, reg, imm);
>         case EX_TYPE_ZEROPAD:
>                 return ex_handler_zeropad(e, regs, fault_addr);
> +#ifdef CONFIG_X86_FRED
> +       case EX_TYPE_ERETU:
> +               return ex_handler_eretu(e, regs, error_code);
> +#endif
>         }
>         BUG();
>  }
> --
> 2.34.1
>
  
Li, Xin3 April 1, 2023, 8:12 a.m. UTC | #2
> > +       /* Copy error code to uregs and adjust stack pointer accordingly */
> > +       uregs->orig_ax = error_code;
> 
> The address of uregs->orig_ax is below regs->sp, so I think some comments are
> needed here to state why it is safe to write to uregs->orig_ax (a.k.a it is not
> verlapped with regs).

Good point, because it's one of the nice FRED features.

The RSP used by FRED to push a stack frame is not the value in %rsp, it is
calculated from %rsp with the following 2 steps:
  1) RSP = %rsp - (IA32_FRED_CONFIG & 0x1c0) // REDZONE of (N * 64) bytes
  2) RSP = RSP & ~0x3f // Clearing RSP[5:0] to align to a 64-byte cache line
when the event delivery doesn't trigger a stack level change.

Thus the FRED stack frame error code, i.e., orig_ax, is _always_ on a 64-byte
cache line boundary, and a new stack frame is guaranteed to start below the
error code (An extra REDZONE of (N * 64) bytes may be pushed between), and
it is safe to write to uregs->orig_ax.

Here is an example with a N=1 REDZONE:

  64-byte cache line ==>  ______________
                         |___Reserved___|
                         |__Event_data__|
                         |_____SS_______|
                         |_____RSP______|
                         |_____FLAGS____|
                         |_____CS_______|
                         |_____IP_______|  <== ERETU stack frame
  64-byte cache line ==> |__Error_code__|
                         |______________|
                         |______________|
                         |______________|
                         |______________|
                         |______________|
                         |______________|
                         |______________|  <== RSP after step 1)
  64-byte cache line ==> |______________|  <== RSP after step 2)
                         |___Reserved___|
                         |__Event_data__|
                         |_____SS_______|
                         |_____RSP______|
                         |_____FLAGS____|
                         |_____CS_______|
                         |_____IP_______|  <== ERETS stack frame
  64-byte cache line ==> |__Error_code__|

    Xin
  

Patch

diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
index d975cacd060f..efe2bcd11273 100644
--- a/arch/x86/entry/entry_64_fred.S
+++ b/arch/x86/entry/entry_64_fred.S
@@ -5,8 +5,10 @@ 
  * The actual FRED entry points.
  */
 #include <linux/linkage.h>
-#include <asm/errno.h>
+#include <asm/asm.h>
 #include <asm/asm-offsets.h>
+#include <asm/errno.h>
+#include <asm/export.h>
 #include <asm/fred.h>
 
 #include "calling.h"
@@ -38,7 +40,9 @@  SYM_CODE_START_NOALIGN(fred_entrypoint_user)
 	call	fred_entry_from_user
 SYM_INNER_LABEL(fred_exit_user, SYM_L_GLOBAL)
 	FRED_EXIT
-	ERETU
+1:	ERETU
+
+	_ASM_EXTABLE_TYPE(1b, fred_entrypoint_user, EX_TYPE_ERETU)
 SYM_CODE_END(fred_entrypoint_user)
 
 .fill fred_entrypoint_kernel - ., 1, 0xcc
diff --git a/arch/x86/include/asm/extable_fixup_types.h b/arch/x86/include/asm/extable_fixup_types.h
index 991e31cfde94..1585c798a02f 100644
--- a/arch/x86/include/asm/extable_fixup_types.h
+++ b/arch/x86/include/asm/extable_fixup_types.h
@@ -64,6 +64,8 @@ 
 #define	EX_TYPE_UCOPY_LEN4		(EX_TYPE_UCOPY_LEN | EX_DATA_IMM(4))
 #define	EX_TYPE_UCOPY_LEN8		(EX_TYPE_UCOPY_LEN | EX_DATA_IMM(8))
 
-#define EX_TYPE_ZEROPAD			20 /* longword load with zeropad on fault */
+#define	EX_TYPE_ZEROPAD			20 /* longword load with zeropad on fault */
+
+#define	EX_TYPE_ERETU			21
 
 #endif
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 60814e110a54..a5d75b27a993 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -6,6 +6,7 @@ 
 #include <xen/xen.h>
 
 #include <asm/fpu/api.h>
+#include <asm/fred.h>
 #include <asm/sev.h>
 #include <asm/traps.h>
 #include <asm/kdebug.h>
@@ -195,6 +196,37 @@  static bool ex_handler_ucopy_len(const struct exception_table_entry *fixup,
 	return ex_handler_uaccess(fixup, regs, trapnr);
 }
 
+#ifdef CONFIG_X86_FRED
+static bool ex_handler_eretu(const struct exception_table_entry *fixup,
+			     struct pt_regs *regs, unsigned long error_code)
+{
+	struct pt_regs *uregs = (struct pt_regs *)(regs->sp - offsetof(struct pt_regs, ip));
+	unsigned short ss = uregs->ss;
+	unsigned short cs = uregs->cs;
+
+	/*
+	 * Move the NMI bit from the invalid stack frame, which caused ERETU
+	 * to fault, to the fault handler's stack frame, thus to unblock NMI
+	 * with the fault handler's ERETS instruction ASAP if NMI is blocked.
+	 */
+	regs->nmi = uregs->nmi;
+
+	fred_info(uregs)->edata = fred_event_data(regs);
+	uregs->ssx = regs->ssx;
+	uregs->ss = ss;
+	uregs->csx = regs->csx;
+	uregs->nmi = 0;	/* The NMI bit was moved away above */
+	uregs->current_stack_level = 0;
+	uregs->cs = cs;
+
+	/* Copy error code to uregs and adjust stack pointer accordingly */
+	uregs->orig_ax = error_code;
+	regs->sp -= 8;
+
+	return ex_handler_default(fixup, regs);
+}
+#endif
+
 int ex_get_fixup_type(unsigned long ip)
 {
 	const struct exception_table_entry *e = search_exception_tables(ip);
@@ -272,6 +304,10 @@  int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
 		return ex_handler_ucopy_len(e, regs, trapnr, reg, imm);
 	case EX_TYPE_ZEROPAD:
 		return ex_handler_zeropad(e, regs, fault_addr);
+#ifdef CONFIG_X86_FRED
+	case EX_TYPE_ERETU:
+		return ex_handler_eretu(e, regs, error_code);
+#endif
 	}
 	BUG();
 }