Skip to content

x86: Various SYSENTER/SYSEXIT/#DB fixes and cleanups

Andy Lutomirski lkml 2 versions – New

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest12 patches 0 repliesNew
  2. v10 of 10 patches 5 repliesApplied

Changes in v2 from the cover letter

- s/Sysenter/SYSENTER in two places (Borislav)
- int main() -> int main(void) (Borislav)
- Fix some "SYENTER" typoed
- Add the last two patches (11 was a missed cleanup and 12 is a respin)
- Patch 12 differs from the old version in that Ingo's comments are hopefully addressed.

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/12selftests/x86: In syscall_nt, test NT|TF as welltools/testing/selftests/x86/syscall_nt.c+49 -80 replies
2/12x86/entry/compat: In SYSENTER, sink AC clearing below the existing FLAGS testarch/x86/entry/entry_64_compat.S+3 -50 replies
3/12x86/entry/32: Filter NT and speed up AC filtering in SYSENTERarch/x86/entry/entry_32.S+22 -10 replies
4/12x86/entry/32: Restore FLAGS on SYSEXITarch/x86/entry/entry_32.S+9 -00 replies
5/12x86/traps: Clear TIF_BLOCKSTEP on all debug exceptionsarch/x86/kernel/traps.c+7 -50 replies
6/12x86/traps: Clear DR6 early in do_debug and improve the commentarch/x86/kernel/traps.c+12 -30 replies
7/12x86/entry: Vastly simplify SYSENTER TF handlingarch/x86/entry/entry_32.S arch/x86/entry/entry_64_compat.S arch/x86/include/asm/proto.h +1 more+94 -240 replies
8/12x86/entry: Only allocate space for SYSENTER_stack if neededarch/x86/include/asm/processor.h+2 -00 replies
9/12x86/entry/32: Simplify and fix up the SYSENTER stack #DB/NMI fixuparch/x86/entry/entry_32.S arch/x86/kernel/asm-offsets_32.c+56 -630 replies
10/12x86/entry/32: Add and check a stack canary for the SYSENTER stackarch/x86/include/asm/processor.h arch/x86/kernel/process.c arch/x86/kernel/traps.c+13 -10 replies
11/12x86/entry: Remove TIF_SINGLESTEP entry workarch/x86/entry/common.c arch/x86/include/asm/thread_info.h+1 -110 replies
12/12x86/entry: Improve system call entry commentsarch/x86/entry/entry_32.S arch/x86/entry/entry_64.S arch/x86/entry/entry_64_compat.S+128 -280 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v2 yet.

Read the v2 thread

Cover letter [PATCH v2 00/12] x86: Various SYSENTER/SYSEXIT/#DB fixes and cleanups

hpa asked me to get rid of the ASM_CLAC at the beginning of the SYSENTER path. Little did he know...

This series makes the observed behavior of SYSENTER wrt flags the same for all sane flags and kernel bitnesses. That is, SYSENTER preserves flags now unless you do a syscall that explicitly changes flags, and the HW flags that the syscall executes with are sanitized. This includes NT, TF, AC and all arithmetic flags. Prior to this series, 32-bit kernels clobbered TF and the arithmetic flags and behaved highly erratically if NT was set. (If IF is cleared by evil userspace when SYSENTER starts, IF will be set again on return. There's nothing the kernel can do about this -- SYSENTER inherently forgets the state of IF.)

This series speeds up SYSENTER on all kernels by a surprisingly large amount on Skylake because it eliminates an unconditional CLAC.

While SYSENTER used to handle TF correctly as far as I can tell on 64-bit kernels, the means by which it did so was heavily tangled up in the ptrace single-step logic. It now works just like all the other kernel entries except insofar as do_debug has a simple special case for it. Relatedly, the bizarre and poorly explained old fixup in do_debug is now hidden behind a WARN_ON_ONCE in preparation for deleting it at some point.

The code that fixed up NMI and #DB early in SYSENTER in 32-bit kernels used to be both terrifying and incorrect. (It doesn't appear to have been exploitably bad, but the reason for that is subtle, and the code was certainy more fragile than it deserved to me.) We still need a special fixup, but it's much simpler now.

While I was doing all this, I also noticed that DR6 and BTF handling in do_debug was a bit off. Two of the patches in here try to fix it up.

Have fun!

tl;dr: Cleanups and sanity fixes here, but no security fixes, and I don't think anything needs to be backported or put in x86/urgent.

This series applies to the result of merging tip:x86/asm and tip:x86/urgent. I've been testing on a somewhat bastardized base, because tip currently doesn't work on my laptop in 32-bit mode. (That bug is fixed in Linus' tree.)

Changes from v1:
- s/Sysenter/SYSENTER in two places (Borislav)
- int main() -> int main(void) (Borislav)
- Fix some "SYENTER" typoed
- Add the last two patches (11 was a missed cleanup and 12 is a respin)
- Patch 12 differs from the old version in that Ingo's comments are hopefully addressed.

Andy Lutomirski (12):
selftests/x86: In syscall_nt, test NT|TF as well x86/entry/compat: In SYSENTER, sink AC clearing below the existing
FLAGS test
x86/entry/32: Filter NT and speed up AC filtering in SYSENTER x86/entry/32: Restore FLAGS on SYSEXIT
x86/traps: Clear TIF_BLOCKSTEP on all debug exceptions x86/traps: Clear DR6 early in do_debug and improve the comment x86/entry: Vastly simplify SYSENTER TF handling x86/entry: Only allocate space for SYSENTER_stack if needed x86/entry/32: Simplify and fix up the SYSENTER stack #DB/NMI fixup x86/entry/32: Add and check a stack canary for the SYSENTER stack x86/entry: Remove TIF_SINGLESTEP entry work
x86/entry: Improve system call entry comments

 arch/x86/entry/common.c                  |  10 --
 arch/x86/entry/entry_32.S                | 243 +++++++++++++++++++++----------
 arch/x86/entry/entry_64.S                |  10 ++
 arch/x86/entry/entry_64_compat.S         | 100 +++++++++----
 arch/x86/include/asm/processor.h         |   5 +-
 arch/x86/include/asm/proto.h             |  15 +-
 arch/x86/include/asm/thread_info.h       |   2 +-
 arch/x86/kernel/asm-offsets_32.c         |   5 +
 arch/x86/kernel/process.c                |   3 +
 arch/x86/kernel/traps.c                  |  87 ++++++++---
 tools/testing/selftests/x86/syscall_nt.c |  57 +++++++-
 11 files changed, 392 insertions(+), 145 deletions(-)
Signature · 1 line
2.5.0