All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/1] x86/fred: Fix INT80 emulation for FRED
@ 2024-04-17 17:47 Xin Li (Intel)
  2024-04-18  8:59 ` [tip: x86/urgent] " tip-bot2 for Xin Li (Intel)
  0 siblings, 1 reply; 2+ messages in thread
From: Xin Li (Intel) @ 2024-04-17 17:47 UTC (permalink / raw
  To: linux-kernel; +Cc: luto, tglx, mingo, bp, dave.hansen, x86, hpa

Add a FRED-specific INT80 handler fred_int80_emulation():

1) As INT instructions and hardware interrupts are separate event
   types, FRED does not preclude the use of vector 0x80 for external
   interrupts. As a result the FRED setup code does *NOT* reserve
   vector 0x80 and calling int80_is_external() is not merely
   suboptimal but actively incorrect: it could cause a system call
   to be incorrectly ignored.

2) fred_int80_emulation(), only called for handling vector 0x80 of
   event type EVENT_TYPE_SWINT, will NEVER be called to handle any
   external interrupt (event type EVENT_TYPE_EXTINT).

3) FRED has separate entry flows depending on if the event came from
   user space or kernel space, and because kernel does not use INT
   insns, the FRED kernel entry handler fred_entry_from_kernel()
   falls through to fred_bad_type() if the event type is
   EVENT_TYPE_SWINT, i.e., INT insns. So if the kernel is handling
   an INT insn, it can only be from a user level application.

4) int80_emulation() does a CLEAR_BRANCH_HISTORY, which is
   IDT-specific. While FRED will likely take a different approach if
   it is ever needed: it *probably* belongs in either fred_intx()/
   fred_other() or asm_fred_entrypoint_user(), depending on if this
   ought to be done for all entries from userspace or only system
   calls.

5) int $0x80 is the FAST path for 32-bit system calls under FRED.

A dedicated FRED INT80 handler duplicates quite a bit of the code in
do_int80_emulation(), but it avoids sprinkling more tests and seems
more readable.

Fixes: 55617fb991df ("x86/entry: Do not allow external 0x80 interrupts")

Suggested-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Signed-off-by: Xin Li (Intel) <xin@zytor.com>
---

Changes since v3:
* Make it more clear that why the FRED kernel entry handler
  fred_entry_from_kernel() falls through to fred_bad_type() if the event
  type is EVENT_TYPE_SWINT, i.e., INT insns (Borislav Petkov).
* Fix the comment about CLEAR_BRANCH_HISTORY (Nikolay Borisov).

Changes since v2:
* Add comments explaining the reasons why a FRED-specific INT80 handler
  is required to the head comment of fred_int80_emulation(), not just
  the change log (H. Peter Anvin).
* Incorporate extra clarifications from H. Peter Anvin.
* Fix a few typos and wordings (H. Peter Anvin).
* Add a maintainer tip to the change log and head comment: unify common
  stuff later, i.e., after the code settles (Borislav Petkov).

Change since v1:
* Prefer a FRED-specific INT80 handler instead of sprinkling more tests
  around (Borislav Petkov).
---
 arch/x86/entry/common.c     | 71 +++++++++++++++++++++++++++++++++++++
 arch/x86/entry/entry_fred.c |  2 +-
 2 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 6de50b80702e..700acda99cc8 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -255,6 +255,77 @@ __visible noinstr void do_int80_emulation(struct pt_regs *regs)
 	instrumentation_end();
 	syscall_exit_to_user_mode(regs);
 }
+
+#ifdef CONFIG_X86_FRED
+/*
+ * A FRED-specific INT80 handler fred_int80_emulation() is required:
+ *
+ * 1) As INT instructions and hardware interrupts are separate event
+ *    types, FRED does not preclude the use of vector 0x80 for external
+ *    interrupts. As a result the FRED setup code does *NOT* reserve
+ *    vector 0x80 and calling int80_is_external() is not merely
+ *    suboptimal but actively incorrect: it could cause a system call
+ *    to be incorrectly ignored.
+ *
+ * 2) fred_int80_emulation(), only called for handling vector 0x80 of
+ *    event type EVENT_TYPE_SWINT, will NEVER be called to handle any
+ *    external interrupt (event type EVENT_TYPE_EXTINT).
+ *
+ * 3) FRED has separate entry flows depending on if the event came from
+ *    user space or kernel space, and because kernel does not use INT
+ *    insns, the FRED kernel entry handler fred_entry_from_kernel()
+ *    falls through to fred_bad_type() if the event type is
+ *    EVENT_TYPE_SWINT, i.e., INT insns. So if the kernel is handling
+ *    an INT insn, it can only be from a user level application.
+ *
+ * 4) int80_emulation() does a CLEAR_BRANCH_HISTORY, which is
+ *    IDT-specific. While FRED will likely take a different approach if
+ *    it is ever needed: it *probably* belongs in either fred_intx()/
+ *    fred_other() or asm_fred_entrypoint_user(), depending on if this
+ *    ought to be done for all entries from userspace or only system
+ *    calls.
+ *
+ * 5) int $0x80 is the FAST path for 32-bit system calls under FRED.
+ *
+ * A dedicated FRED INT80 handler duplicates quite a bit of the code in
+ * do_int80_emulation(), but it avoids sprinkling more tests and seems
+ * more readable. Just remember that we can always unify common stuff
+ * later if it turns out that it won't diverge anymore, i.e., after the
+ * FRED code settles.
+ */
+DEFINE_FREDENTRY_RAW(int80_emulation)
+{
+	int nr;
+
+	enter_from_user_mode(regs);
+
+	instrumentation_begin();
+	add_random_kstack_offset();
+
+	/*
+	 * FRED pushed 0 into regs::orig_ax and regs::ax contains the
+	 * syscall number.
+	 *
+	 * User tracing code (ptrace or signal handlers) might assume
+	 * that the regs::orig_ax contains a 32-bit number on invoking
+	 * a 32-bit syscall.
+	 *
+	 * Establish the syscall convention by saving the 32bit truncated
+	 * syscall number in regs::orig_ax and by invalidating regs::ax.
+	 */
+	regs->orig_ax = regs->ax & GENMASK(31, 0);
+	regs->ax = -ENOSYS;
+
+	nr = syscall_32_enter(regs);
+
+	local_irq_enable();
+	nr = syscall_enter_from_user_mode_work(regs, nr);
+	do_syscall_32_irqs_on(regs, nr);
+
+	instrumentation_end();
+	syscall_exit_to_user_mode(regs);
+}
+#endif
 #else /* CONFIG_IA32_EMULATION */
 
 /* Handles int $0x80 on a 32bit kernel */
diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
index ac120cbdaaf2..9fa18b8c7f26 100644
--- a/arch/x86/entry/entry_fred.c
+++ b/arch/x86/entry/entry_fred.c
@@ -66,7 +66,7 @@ static noinstr void fred_intx(struct pt_regs *regs)
 	/* INT80 */
 	case IA32_SYSCALL_VECTOR:
 		if (ia32_enabled())
-			return int80_emulation(regs);
+			return fred_int80_emulation(regs);
 		fallthrough;
 #endif
 

base-commit: 1e0fd81e4f32a8a383c05d27a672d742b45c1088
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [tip: x86/urgent] x86/fred: Fix INT80 emulation for FRED
  2024-04-17 17:47 [PATCH v4 1/1] x86/fred: Fix INT80 emulation for FRED Xin Li (Intel)
@ 2024-04-18  8:59 ` tip-bot2 for Xin Li (Intel)
  0 siblings, 0 replies; 2+ messages in thread
From: tip-bot2 for Xin Li (Intel) @ 2024-04-18  8:59 UTC (permalink / raw
  To: linux-tip-commits
  Cc: H. Peter Anvin (Intel), Xin Li (Intel), Borislav Petkov (AMD),
	x86, linux-kernel

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     32f5f73b79ffdef215e2e1bcb6ad74387c0f925c
Gitweb:        https://git.kernel.org/tip/32f5f73b79ffdef215e2e1bcb6ad74387c0f925c
Author:        Xin Li (Intel) <xin@zytor.com>
AuthorDate:    Wed, 17 Apr 2024 10:47:31 -07:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Thu, 18 Apr 2024 10:37:11 +02:00

x86/fred: Fix INT80 emulation for FRED

Add a FRED-specific INT80 handler and document why it differs from the
current one. Eventually, the common bits will be unified once FRED hw is
available and it turns out that no further changes are needed but for
now, keep the handlers separate for everyone's sanity's sake.

  [ bp: Zap duplicated commit message, massage. ]

Fixes: 55617fb991df ("x86/entry: Do not allow external 0x80 interrupts")
Suggested-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Signed-off-by: Xin Li (Intel) <xin@zytor.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://lore.kernel.org/r/20240417174731.4189592-1-xin@zytor.com
---
 arch/x86/entry/common.c     | 65 ++++++++++++++++++++++++++++++++++++-
 arch/x86/entry/entry_fred.c |  2 +-
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 6de50b8..51cc9c7 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -255,6 +255,71 @@ __visible noinstr void do_int80_emulation(struct pt_regs *regs)
 	instrumentation_end();
 	syscall_exit_to_user_mode(regs);
 }
+
+#ifdef CONFIG_X86_FRED
+/*
+ * A FRED-specific INT80 handler is warranted for the follwing reasons:
+ *
+ * 1) As INT instructions and hardware interrupts are separate event
+ *    types, FRED does not preclude the use of vector 0x80 for external
+ *    interrupts. As a result, the FRED setup code does not reserve
+ *    vector 0x80 and calling int80_is_external() is not merely
+ *    suboptimal but actively incorrect: it could cause a system call
+ *    to be incorrectly ignored.
+ *
+ * 2) It is called only for handling vector 0x80 of event type
+ *    EVENT_TYPE_SWINT and will never be called to handle any external
+ *    interrupt (event type EVENT_TYPE_EXTINT).
+ *
+ * 3) FRED has separate entry flows depending on if the event came from
+ *    user space or kernel space, and because the kernel does not use
+ *    INT insns, the FRED kernel entry handler fred_entry_from_kernel()
+ *    falls through to fred_bad_type() if the event type is
+ *    EVENT_TYPE_SWINT, i.e., INT insns. So if the kernel is handling
+ *    an INT insn, it can only be from a user level.
+ *
+ * 4) int80_emulation() does a CLEAR_BRANCH_HISTORY. While FRED will
+ *    likely take a different approach if it is ever needed: it
+ *    probably belongs in either fred_intx()/ fred_other() or
+ *    asm_fred_entrypoint_user(), depending on if this ought to be done
+ *    for all entries from userspace or only system
+ *    calls.
+ *
+ * 5) INT $0x80 is the fast path for 32-bit system calls under FRED.
+ */
+DEFINE_FREDENTRY_RAW(int80_emulation)
+{
+	int nr;
+
+	enter_from_user_mode(regs);
+
+	instrumentation_begin();
+	add_random_kstack_offset();
+
+	/*
+	 * FRED pushed 0 into regs::orig_ax and regs::ax contains the
+	 * syscall number.
+	 *
+	 * User tracing code (ptrace or signal handlers) might assume
+	 * that the regs::orig_ax contains a 32-bit number on invoking
+	 * a 32-bit syscall.
+	 *
+	 * Establish the syscall convention by saving the 32bit truncated
+	 * syscall number in regs::orig_ax and by invalidating regs::ax.
+	 */
+	regs->orig_ax = regs->ax & GENMASK(31, 0);
+	regs->ax = -ENOSYS;
+
+	nr = syscall_32_enter(regs);
+
+	local_irq_enable();
+	nr = syscall_enter_from_user_mode_work(regs, nr);
+	do_syscall_32_irqs_on(regs, nr);
+
+	instrumentation_end();
+	syscall_exit_to_user_mode(regs);
+}
+#endif
 #else /* CONFIG_IA32_EMULATION */
 
 /* Handles int $0x80 on a 32bit kernel */
diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
index ac120cb..9fa18b8 100644
--- a/arch/x86/entry/entry_fred.c
+++ b/arch/x86/entry/entry_fred.c
@@ -66,7 +66,7 @@ static noinstr void fred_intx(struct pt_regs *regs)
 	/* INT80 */
 	case IA32_SYSCALL_VECTOR:
 		if (ia32_enabled())
-			return int80_emulation(regs);
+			return fred_int80_emulation(regs);
 		fallthrough;
 #endif
 

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-04-18  8:59 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-17 17:47 [PATCH v4 1/1] x86/fred: Fix INT80 emulation for FRED Xin Li (Intel)
2024-04-18  8:59 ` [tip: x86/urgent] " tip-bot2 for Xin Li (Intel)

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.