entry: Consolidate and rework syscall entry handling
Status from this message: The following commit has been merged into the core/entry branch of tip:
Get series with b4
b4 am [email protected]Versions newest first
Patches in v1 Applied
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/18 | powerpc: Move stack randomization after syscall_enter_from_user_mode()arch/ | +1 | 5 replies | |
| 2/18 | randomize_kstack: Provide add_random_kstack_offset_irqsoff()include/ | +19 | 4 replies | |
| 3/18 | entry: Provide [syscall_]enter_from_user_mode_randomize_stack()include/ | +56 | 7 replies | |
| 4/18 | loongarch/syscall: Use syscall_enter_from_user_mode_randomize_stack()arch/ | +1 | 4 replies | |
| 5/18 | powerpc/syscall: Use syscall_enter_from_user_mode_randomize_stack()arch/ | +1 | 4 replies | |
| 6/18 | riscv/syscall: Use syscall_enter_from_user_mode_randomize_stack()arch/ | +1 | 5 replies | |
| 7/18 | s390/syscall: Use enter_from_user_mode_randomize_stack()arch/ | +2 | 6 replies | |
| 8/18 | x86/syscall: Use [syscall_]enter_from_user_mode_randomize_stack()arch/ arch/ arch/ | +6 | 4 replies | |
| 9/18 | entry: Remove syscall_enter_from_user_mode()Documentation/ include/ include/ | +17 | 4 replies | |
| 10/18 | entry: Use syscall number instead of rereading itinclude/ | +5 | 5 replies | |
| 11/18 | seccomp, treewide: Rename and convert __secure_computing() to return booleanarch/ arch/ arch/ +11 more | +45 | 6 replies | |
| 12/18 | ptrace, treewide: Rename ptrace_report_syscall_entry() to ptrace_report_syscall_permit_entry()arch/ arch/ arch/ +17 more | +40 | 7 replies | |
| 13/18 | entry: Make trace_syscall_enter() return type boolinclude/ kernel/ | +9 | 11 replies | |
| 14/18 | entry: Make return type of syscall_trace_enter() boolinclude/ | +15 | 1 reply | |
| 15/18 | x86/entry: Make syscall functions staticarch/ arch/ arch/ | +7 | 2 replies | |
| 16/18 | x86/entry: Get rid of the sys_ni_syscall() indirectionarch/ arch/ | +3 | 17 replies | |
| 17/18 | x86/entry: Simplify the syscall number logicarch/ arch/ arch/ | +26 | 0 replies | |
| 18/18 | entry, treewide: Make syscall_enter_from_user_mode[_work]() indicate syscall executionDocumentation/ arch/ arch/ +5 more | +53 | 3 replies |
Reviews trailers per patch
16 of 18 patches have a Reviewed-by or Acked-by of their own; the series has one for all of them.
| Patch | Jinjie Ruan | Sven Schnelle | Shrikanth Hegde | Oleg Nesterov | Radu Rendec | Geert Uytterhoeven | Philippe Mathieu-Daudé | Kees Cook | Mukesh Kumar Chaurasiya (IBM) | Magnus Lindholm | Guo Ren | Sebastian Andrzej Siewior |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Series | T-b | A-b T-b | ||||||||||
| 1/18 | R-b | R-b | R-b | R-b | R-b | |||||||
| 2/18 | R-b | R-b | R-b | |||||||||
| 3/18 | R-b | R-b | R-b | R-b | ||||||||
| 4/18 | R-b | R-b | R-b | R-b | ||||||||
| 5/18 | R-b | R-b | R-b | R-b | ||||||||
| 6/18 | R-b | R-b | R-b | R-b | R-b | |||||||
| 7/18 | R-b | R-b | R-b | R-b | R-b | |||||||
| 8/18 | R-b | R-b | R-b | R-b | ||||||||
| 9/18 | R-b | R-b | ||||||||||
| 10/18 | R-b | R-b | R-b | R-b | ||||||||
| 11/18 | R-b | A-b | R-b | |||||||||
| 12/18 | R-b | A-b | R-b | A-b | R-b | |||||||
| 13/18 | ||||||||||||
| 14/18 | R-b | |||||||||||
| 15/18 | R-b | R-b | ||||||||||
| 16/18 | R-b | T-b | ||||||||||
| 17/18 | ||||||||||||
| 18/18 | R-b |
Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes. Scroll the table sideways for all 12 reviewers.
Cover letter [patch 00/18] entry: Consolidate and rework syscall entry handling
Sorry for the long CC list, but this is a treewide change.
Michal recently posted a RFC patch to separate the potential syscall number modifications in syscall_enter_user_mode_work() from the information whether the syscall should be processed and the return value modified:
https://lore.kernel.org/lkml/[email protected]
The existing logic is:
arch_syscall()
regs->result = -ENOSYS; syscallnr = syscall_enter_from_user_mode(regs, syscall); if (syscallnr != -1L) regs->result = invoke_syscall(regs, syscall;
syscall_enter_from_user_mode() invokes ptrace, seccomp and tracing/BPF/Probes. All of them can modify the syscall number.
ptrace and seccomp explicitly set the syscall number to -1L to indicate that the syscall invocation needs to be skipped and the result has not to be modified as it might have been modified by ptrace or seccomp. The tracer/BPF/Probes mechanism can modify the syscall number as well and relies implicitly on the -1L logic.
This can obviously not be differentiated from a syscall invocation where userspace provided -1 as syscall number.
The general agreement of the discussion was that the current mechanism, while functionally correct is non-intuitive and something like Michals proposal would make that code clearer and easier to handle on the architecture side:
arch_syscall()
regs->result = -ENOSYS; if (syscall_enter_from_user_mode(regs, &syscall)) regs->result = invoke_syscall(regs, syscall;
That discussion made me look deeper into the related code and as usual there were a lot of other things to discover.
1) Stack randomization
add_random_kstack_offset() can only be invoked after enter_from_user_mode() established proper state as it calls into instrumentable code.
PowerPC got that wrong and the other architectures either invoke it after enter_from_user_mode() or after syscall_enter_from_user_mode().
The latter is suboptimal as the randomization takes place after all the user mode entry work. Aside of that add_random_kstack_offset() uses get/put_cpu_var(), which makes it usable in preemptible code, but when invoked in the interrupt disabled region that's pointless overhead.
2) As discussed in the above thread just changing the function signature of syscall_enter_from_user_mode[_work]() so they take a pointer argument for the syscall and then return 0 on success is not really intuitive either. Aside of that this breaks the implicit assumption of the tracer when setting the syscall number to -1.
3) The x86 entry code has some historically accumulated oddities
The following series addresses this by:
1) Providing new [syscall_]enter_from_user_mode() variants, which include stack randomization and utilize a new add_random_kstack_offset_irqsoff() variant, which avoids the get/put_cpu_var() overhead and converting all usage sites over
2) Picking up Jinjie's seccomp patch from:
https://lore.kernel.org/lkml/[email protected]
and addressing the feedback (renaming the seccomp functions)
3) Making the ptrace and tracer related functions return a boolean value to indicate syscall permission
4) Addressing the x86 oddities
5) Converting the tree over to the new scheme
With that all architectures using the generic syscall entry code follow the same scheme, apply stack randomization at the correct and earliest possible place and skip syscall processing depending on the boolean return value of syscall_enter_from_user_mode[_work]().
There should be no functional changes, at least there are none intended.
The resulting text size for the syscall entry code on x8664 is slightly smaller than before these changes.
Testing syscall heavy workloads and micro benchmarks shows a small performance gain for the general rework, but the last patch, which changes the logic to be more understandable has no measurable impact in either direction.
The series applies on Linus tree and is also available from git:
git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git entry-rework-v1
Thanks,
tglx
--- Documentation/core-api/entry.rst | 33 +++++--- arch/alpha/kernel/ptrace.c | 4 - arch/arc/kernel/ptrace.c | 2 arch/arm/kernel/ptrace.c | 4 - arch/arm64/kernel/ptrace.c | 4 - arch/csky/kernel/ptrace.c | 4 - arch/hexagon/kernel/traps.c | 2 arch/loongarch/kernel/syscall.c | 17 +--- arch/m68k/kernel/ptrace.c | 4 - arch/microblaze/kernel/ptrace.c | 2 arch/mips/kernel/ptrace.c | 4 - arch/nios2/kernel/ptrace.c | 2 arch/openrisc/kernel/ptrace.c | 2 arch/parisc/kernel/ptrace.c | 12 +-- arch/powerpc/kernel/syscall.c | 5 - arch/riscv/kernel/traps.c | 14 +-- arch/s390/kernel/syscall.c | 11 +- arch/sh/kernel/ptrace_32.c | 4 - arch/sparc/kernel/ptrace_32.c | 2 arch/sparc/kernel/ptrace_64.c | 2 arch/um/kernel/ptrace.c | 2 arch/um/kernel/skas/syscall.c | 2 arch/x86/entry/syscall_32.c | 70 +++++++------------ arch/x86/entry/syscall_64.c | 61 ++++++---------- arch/x86/entry/vsyscall/vsyscall_64.c | 14 +-- arch/x86/include/asm/entry-common.h | 1 arch/x86/include/asm/syscall.h | 10 -- arch/xtensa/kernel/ptrace.c | 5 - include/asm-generic/syscall.h | 4 - include/linux/entry-common.h | 125 ++++++++++++++++++++-------------- include/linux/irq-entry-common.h | 6 - include/linux/ptrace.h | 13 +-- include/linux/randomize_kstack.h | 19 +++++ include/linux/seccomp.h | 12 +-- kernel/entry/syscall-common.c | 7 + kernel/seccomp.c | 35 ++++----- 36 files changed, 264 insertions(+), 256 deletions(-)