Received-SPF: pass (google.com: domain of linux-kernel+bounces-149007-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: "Xin Li (Intel)" <xin@zytor.com>
To: linux-kernel@vger.kernel.org
Cc: luto@kernel.org, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com
Subject: [PATCH v4 1/1] x86/fred: Fix INT80 emulation for FRED
Date: Wed, 17 Apr 2024 10:47:31 -0700
Message-ID: <20240417174731.4189592-1-xin@zytor.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

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