entry: Rework syscall skip logic

Move it out of line and let it reread the syscall number on it's own. That makes the low level entry code denser and allows to move the reread to the call site of syscall_trace_enter() once the tracer is fixed up.

Signed-off-by Thomas Gleixner <[email protected]>

---
 include/linux/entry-common.h  |   14 +++-----------
 kernel/entry/syscall-common.c |   10 ++++++++++
 2 files changed, 13 insertions(+), 11 deletions(-)

include/linux/entry-common.h +1 -10

@@ -60,16 +60,7 @@ static __always_inline bool arch_ptrace_
 
 long trace_syscall_enter(struct pt_regs *regs, long syscall);
 void trace_syscall_exit(struct pt_regs *regs, long ret);
-
-static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
-{
-	if (unlikely(audit_context())) {
-		unsigned long args[6];
-
-		syscall_get_arguments(current, regs, args);
-		audit_syscall_entry(syscall, args[0], args[1], args[2], args[3]);
-	}
-}
+void syscall_enter_audit(struct pt_regs *regs);
 
 static __always_inline long syscall_trace_enter(struct pt_regs *regs, unsigned long work,
 						long syscall)

include/linux/entry-common.h +0 -0

 	return syscall;
 }

kernel/entry/syscall-common.c +10 -0

@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 
+#include <linux/audit.h>
 #include <linux/entry-common.h>
 
 #define CREATE_TRACE_POINTS

@@ -21,3 +22,12 @@ void trace_syscall_exit(struct pt_regs *
 {
 	trace_sys_exit(regs, ret);
 }
+
+void syscall_enter_audit(struct pt_regs *regs)
+{
+	long syscall = syscall_get_nr(current, regs);
+	unsigned long args[6];
+
+	syscall_get_arguments(current, regs, args);
+	__audit_syscall_entry(syscall, args[0], args[1], args[2], args[3]);
+}

Move it out of line and let it reread the syscall number on it's own. That makes the low level entry code denser and allows to move the reread to the call site of syscall_trace_enter() once the tracer is fixed up.

Signed-off-by Thomas Gleixner <[email protected]>

---
 include/linux/entry-common.h  |   14 +++----------- kernel/entry/syscall-common.c |   10 ++++++++++

 2 files changed, 13 insertions(+), 11 deletions(-)

include/linux/entry-common.h +0 -0

@@ -60,16 +60,7 @@ static __always_inline bool arch_ptrace_

 long trace_syscall_enter(struct pt_regs *regs, long syscall); void trace_syscall_exit(struct pt_regs *regs, long ret);

include/linux/entry-common.h +1 -10

-
-static inline void syscall_enter_audit(struct pt_regs *regs, long syscall)
-{
-	if (unlikely(audit_context())) {
-		unsigned long args[6];
-
-		syscall_get_arguments(current, regs, args);
-		audit_syscall_entry(syscall, args[0], args[1], args[2], args[3]);
-	}
-}
+void syscall_enter_audit(struct pt_regs *regs);

 static __always_inline long syscall_trace_enter(struct pt_regs *regs, unsigned long work, long syscall)

include/linux/entry-common.h +0 -0

@@ -111,7 +102,8 @@ static __always_inline long syscall_trac
  	if (unlikely(work & SYSCALL_WORK_SYSCALL_TRACEPOINT))
  		syscall = trace_syscall_enter(regs, syscall);

include/linux/entry-common.h +2 -1

-	syscall_enter_audit(regs, syscall);
+	if (unlikely(audit_context()))
+		syscall_enter_audit(regs);

  return syscall;
 }

kernel/entry/syscall-common.c +0 -0

@@ -1,5 +1,6 @@
  // SPDX-License-Identifier: GPL-2.0

+#include <linux/audit.h>
 #include <linux/entry-common.h>

 #define CREATE_TRACE_POINTS

kernel/entry/syscall-common.c +0 -0

@@ -21,3 +22,12 @@ void trace_syscall_exit(struct pt_regs *
  {
  	trace_sys_exit(regs, ret);

Reread the syscall number from pt_regs and stop returning the eventually modified syscall number.

That moves the reread to the end of the syscall_trace_enter() and prepares for moving it to the call site.

No functional change.

Signed-off-by Thomas Gleixner <[email protected]>

---
 include/linux/entry-common.h  |   10 ++++------
 kernel/entry/syscall-common.c |    9 ++-------
 2 files changed, 6 insertions(+), 13 deletions(-)

include/linux/entry-common.h +1 -4

@@ -58,7 +58,7 @@ static __always_inline bool arch_ptrace_
 }
 #endif
 
-long trace_syscall_enter(struct pt_regs *regs, long syscall);
+void trace_syscall_enter(struct pt_regs *regs);
 void trace_syscall_exit(struct pt_regs *regs, long ret);
 void syscall_enter_audit(struct pt_regs *regs);
 

@@ -96,16 +96,14 @@ static __always_inline long syscall_trac
 			return -1L;
 	}
 
-	/* Either of the above might have changed the syscall number */
-	syscall = syscall_get_nr(current, regs);
-

include/linux/entry-common.h +0 -0

 }
 
 /**

kernel/entry/syscall-common.c +2 -7

@@ -8,14 +8,9 @@
 
 /* Out of line to prevent tracepoint code duplication */
 
-long trace_syscall_enter(struct pt_regs *regs, long syscall)
+void trace_syscall_enter(struct pt_regs *regs)
 {
-	trace_sys_enter(regs, syscall);
-	/*
-	 * Probes or BPF hooks in the tracepoint may have changed the
-	 * system call number. Reread it.
-	 */
-	return syscall_get_nr(current, regs);
+	trace_sys_enter(regs, syscall_get_nr(current, regs));
 }
 
 void trace_syscall_exit(struct pt_regs *regs, long ret)

Reread the syscall number from pt_regs and stop returning the eventually modified syscall number.

That moves the reread to the end of the syscall_trace_enter() and prepares for moving it to the call site.

No functional change.

Signed-off-by Thomas Gleixner <[email protected]>

---
 include/linux/entry-common.h  |   10 ++++------
 kernel/entry/syscall-common.c |    9 ++-------

 2 files changed, 6 insertions(+), 13 deletions(-)

include/linux/entry-common.h +0 -0

@@ -58,7 +58,7 @@ static __always_inline bool arch_ptrace_
  }
  #endif

include/linux/entry-common.h +1 -1

-long trace_syscall_enter(struct pt_regs *regs, long syscall);
+void trace_syscall_enter(struct pt_regs *regs);

 void trace_syscall_exit(struct pt_regs *regs, long ret); void syscall_enter_audit(struct pt_regs *regs);

include/linux/entry-common.h +0 -0

@@ -96,16 +96,14 @@ static __always_inline long syscall_trac
  			return -1L;
  	}

include/linux/entry-common.h +0 -3

-	/* Either of the above might have changed the syscall number */
-	syscall = syscall_get_nr(current, regs);
-

  if (unlikely(work & SYSCALL_WORK_SYSCALL_TRACEPOINT))

include/linux/entry-common.h +1 -1

-		syscall = trace_syscall_enter(regs, syscall);
+		trace_syscall_enter(regs);

  if (unlikely(audit_context()))
  syscall_enter_audit(regs);

include/linux/entry-common.h +1 -0

+	return syscall_get_nr(current, regs);

 }

 /**

kernel/entry/syscall-common.c +0 -0

@@ -8,14 +8,9 @@

 /* Out of line to prevent tracepoint code duplication */

kernel/entry/syscall-common.c +1 -1

-long trace_syscall_enter(struct pt_regs *regs, long syscall)
+void trace_syscall_enter(struct pt_regs *regs)

 {

kernel/entry/syscall-common.c +1 -6

-	trace_sys_enter(regs, syscall);
-	/*
-	 * Probes or BPF hooks in the tracepoint may have changed the
-	 * system call number. Reread it.
-	 */
-	return syscall_get_nr(current, regs);
+	trace_sys_enter(regs, syscall_get_nr(current, regs));

 }

 void trace_syscall_exit(struct pt_regs *regs, long ret)

From: Thomas Gleixner <[email protected]>

This prepares for changing the return types of syscall_enter_from_user_mode[_work]() to bool, which in turn separates the decision of invoking the syscall from the syscall number, which might have been changed in the call by ptrace, seccomp, tracing.

Signed-off-by Thomas Gleixner <[email protected]>

---
 include/linux/entry-common.h |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

include/linux/entry-common.h +4 -5

@@ -72,7 +72,7 @@ static __always_inline long syscall_trac
 	 */
 	if (work & SYSCALL_WORK_SYSCALL_USER_DISPATCH) {
 		if (syscall_user_dispatch(regs))
-			return -1L;
+			return false;
 	}
 
 	/*

@@ -87,13 +87,13 @@ static __always_inline long syscall_trac
 	if (work & (SYSCALL_WORK_SYSCALL_TRACE | SYSCALL_WORK_SYSCALL_EMU)) {
 		if (!arch_ptrace_report_syscall_permit_entry(regs) ||
 		    (work & SYSCALL_WORK_SYSCALL_EMU))
-			return -1L;
+			return false;
 	}
 
 	/* Do seccomp after ptrace, to catch any tracer changes. */
 	if (work & SYSCALL_WORK_SECCOMP) {
 		if (!__seccomp_permit_syscall())
-			return -1L;
+			return false;
 	}
 
 	if (unlikely(work & SYSCALL_WORK_SYSCALL_TRACEPOINT))

@@ -102,8 +102,7 @@ static __always_inline long syscall_trac
 	if (unlikely(audit_context()))
 		syscall_enter_audit(regs);
 
-	/* Either of the above might have changed the syscall number */
-	return syscall_get_nr(current, regs);
+	return true;
 }
 
 /**

@@ -133,8 +132,13 @@ static __always_inline long syscall_ente
 {
 	unsigned long work = READ_ONCE(current_thread_info()->syscall_work);

include/linux/entry-common.h +0 -0

 	return syscall;
 }
 

From: Thomas Gleixner <[email protected]>

This prepares for changing the return types of syscall_enter_from_user_mode[_work]() to bool, which in turn separates the decision of invoking the syscall from the syscall number, which might have been changed in the call by ptrace, seccomp, tracing.

Signed-off-by Thomas Gleixner <[email protected]>

---
 include/linux/entry-common.h |   18 +++++++++++-------

 1 file changed, 11 insertions(+), 7 deletions(-)

include/linux/entry-common.h +1 -1

@@ -72,7 +72,7 @@ static __always_inline long syscall_trac
  	 */
  	if (work & SYSCALL_WORK_SYSCALL_USER_DISPATCH) {
  		if (syscall_user_dispatch(regs))
-			return -1L;
+			return false;
  	}

  /*

include/linux/entry-common.h +0 -0

@@ -87,13 +87,13 @@ static __always_inline long syscall_trac
  	if (work & (SYSCALL_WORK_SYSCALL_TRACE | SYSCALL_WORK_SYSCALL_EMU)) {

  if (!arch_ptrace_report_syscall_permit_entry(regs) || (work & SYSCALL_WORK_SYSCALL_EMU))

include/linux/entry-common.h +1 -1

-			return -1L;
+			return false;

  }

  /* Do seccomp after ptrace, to catch any tracer changes. */ if (work & SYSCALL_WORK_SECCOMP) {
  if (!__seccomp_permit_syscall())

include/linux/entry-common.h +1 -1

-			return -1L;
+			return false;

  }

  if (unlikely(work & SYSCALL_WORK_SYSCALL_TRACEPOINT))

include/linux/entry-common.h +0 -0

@@ -102,8 +102,7 @@ static __always_inline long syscall_trac
  	if (unlikely(audit_context()))
  		syscall_enter_audit(regs);

include/linux/entry-common.h +1 -2

-	/* Either of the above might have changed the syscall number */
-	return syscall_get_nr(current, regs);
+	return true;

 }

 /**

include/linux/entry-common.h +0 -0

@@ -133,8 +132,13 @@ static __always_inline long syscall_ente
  {
  	unsigned long work = READ_ONCE(current_thread_info()->syscall_work);

include/linux/entry-common.h +2 -0

+		syscall = syscall_get_nr(current, regs);
+	}

  return syscall;
 }

The return values of syscall_enter_from_user_mode[_work]() are non-intuitive. Both functions return the syscall number which should be invoked by the architecture specific syscall entry code. The returned number can be:

- the unmodified syscall number which was handed in by the caller

- a modified syscall number (ptrace, seccomp, trace/probe/bpf)

That has an additional twist. If the return value is -1L then the caller is not allowed to modify the return value as that indicates that the modifying entity requests to abort the syscall and set the return value already. That can obviously not be differentiated from a syscall which handed in -1 as syscall number.

The most trivial way to deal with that is:

    set_return_value(regs, -ENOSYS);
    nr = syscall_enter_from_user_mode(regs, nr);
    if (valid(nr))
    	handle_syscall(regs, nr);

That's what LOONGARCH, RISCV, and X86 do. But PowerPC and S390 do not preset the return value, so when user space hands in -1 and there is nothing setting the return value in the entry work code, then the syscall is skipped but the return value is whatever random data has been in the return value register.

Change the return values of syscall_enter_from_user_mode[_work]() to boolean and return false, when either ptrace or seccomp request to skip the

Hello,

Quoted 22 lines · Thomas Gleixner

The return values of syscall_enter_from_user_mode[_work]() are non-intuitive. Both functions return the syscall number which should be invoked by the architecture specific syscall entry code. The returned number can be:

- the unmodified syscall number which was handed in by the caller

- a modified syscall number (ptrace, seccomp, trace/probe/bpf)

That has an additional twist. If the return value is -1L then the caller is not allowed to modify the return value as that indicates that the modifying entity requests to abort the syscall and set the return value already. That can obviously not be differentiated from a syscall which handed in -1 as syscall number.

The most trivial way to deal with that is:

    set_return_value(regs, -ENOSYS);
    nr = syscall_enter_from_user_mode(regs, nr);
    if (valid(nr))
    	handle_syscall(regs, nr);

Quoted 5 lines

That's what LOONGARCH, RISCV, and X86 do. But PowerPC and S390 do not preset the return value, so when user space hands in -1 and there is nothing setting the return value in the entry work code, then the syscall is skipped but the return value is whatever random data has been in the return value register.

The reason why PowerPC and S390 do not preset the return value is that the return value uses the same register as the syscall number. There are apparently other architectures on which the return value overlaps with the arguments which also do not preset the return value for that reason. If they would use the generic entry the same problem would arise.

Quoted 2 lines

Change the return values of syscall_enter_from_user_mode[_work]() to boolean and return false, when either ptrace or seccomp request to skip the

There is a difference between seccomp and ptrace.

When seccomp indicates to skip the syscall it has also set the syscall return value.

However, when the syscall number is -1 and the return value is not preset that does not indicate anything.

The return value can still hold garbage. ptrace does not have the ability to indicate that a syscall is to be skipped, at least on the entry trace. It needs to be skipped based on the syscall number being invalid.

Quoted 22 lines · Thomas Gleixner

The return values of syscall_enter_from_user_mode[_work]() are non-intuitive. Both functions return the syscall number which should be invoked by the architecture specific syscall entry code. The returned number can be:

- the unmodified syscall number which was handed in by the caller

- a modified syscall number (ptrace, seccomp, trace/probe/bpf)

That has an additional twist. If the return value is -1L then the caller is not allowed to modify the return value as that indicates that the modifying entity requests to abort the syscall and set the return value already. That can obviously not be differentiated from a syscall which handed in -1 as syscall number.

The most trivial way to deal with that is:

    set_return_value(regs, -ENOSYS);
    nr = syscall_enter_from_user_mode(regs, nr);
    if (valid(nr))
    	handle_syscall(regs, nr);

Quoted 5 lines

That's what LOONGARCH, RISCV, and X86 do. But PowerPC and S390 do not preset the return value, so when user space hands in -1 and there is nothing setting the return value in the entry work code, then the syscall is skipped but the return value is whatever random data has been in the return value register.

The reason why PowerPC and S390 do not preset the return value is that the return value uses the same register as the syscall number. There are apparently other architectures on which the return value overlaps with the arguments which also do not preset the return value for that reason. If they would use the generic entry the same problem would arise.

Quoted 2 lines

Change the return values of syscall_enter_from_user_mode[_work]() to boolean and return false, when either ptrace or seccomp request to skip the

There is a difference between seccomp and ptrace.

When seccomp indicates to skip the syscall it has also set the syscall return value.

However, when the syscall number is -1 and the return value is not preset that does not indicate anything.

The return value can still hold garbage. ptrace does not have the ability to indicate that a syscall is to be skipped, at least on the entry trace. It needs to be skipped based on the syscall number being invalid.

Quoted 29 lines · Michal Suchánek

Quoted 22 lines · Thomas Gleixner

The return values of syscall_enter_from_user_mode[_work]() are non-intuitive. Both functions return the syscall number which should be invoked by the architecture specific syscall entry code. The returned number can be:

- the unmodified syscall number which was handed in by the caller

- a modified syscall number (ptrace, seccomp, trace/probe/bpf)

That has an additional twist. If the return value is -1L then the caller is not allowed to modify the return value as that indicates that the modifying entity requests to abort the syscall and set the return value already. That can obviously not be differentiated from a syscall which handed in -1 as syscall number.

The most trivial way to deal with that is:

    set_return_value(regs, -ENOSYS);
    nr = syscall_enter_from_user_mode(regs, nr);
    if (valid(nr))
    	handle_syscall(regs, nr);

Quoted 5 lines

That's what LOONGARCH, RISCV, and X86 do. But PowerPC and S390 do not preset the return value, so when user space hands in -1 and there is nothing setting the return value in the entry work code, then the syscall is skipped but the return value is whatever random data has been in the return value register.

Quoted 5 lines

The reason why PowerPC and S390 do not preset the return value is that the return value uses the same register as the syscall number. There are apparently other architectures on which the return value overlaps with the arguments which also do not preset the return value for that reason. If they would use the generic entry the same problem would arise.

That's an implementation choice of PPC/S390 as I explained before, which could trivially be solved by having an explicit pt_regs->return_val member,

Quoted 8 lines

Quoted 2 lines

Change the return values of syscall_enter_from_user_mode[_work]() to boolean and return false, when either ptrace or seccomp request to skip the

There is a difference between seccomp and ptrace.

When seccomp indicates to skip the syscall it has also set the syscall return value.

Quoted 2 lines

However, when the syscall number is -1 and the return value is not preset that does not indicate anything.

I agree it's an invalid syscall, but the current generic entry code made the rightful assumption that returning -1L as the syscall number either results in -ENOSYS or in the value which was set by one of the entry mechanisms as that code originated from the x86 implementation.

It's not the fault of that code that PPC and S390 converted their stuff over without paying attention to that detail.

Quoted 5 lines

The return value can still hold garbage. ptrace does not have the ability to indicate that a syscall is to be skipped, at least on the entry trace. It needs to be skipped based on the syscall number being invalid.

That's what I explained you before and you told me I'm all wrong.

But that's moot as this latest version does not care anymore. The architectures whixh preset the return value are correct under all circumstances and PPC/S390 can keep their own world view.

Quoted 36 lines · Thomas Gleixner

Quoted 34 lines · Michal Suchánek

Quoted 22 lines · Thomas Gleixner

The return values of syscall_enter_from_user_mode[_work]() are non-intuitive. Both functions return the syscall number which should be invoked by the architecture specific syscall entry code. The returned number can be:

- the unmodified syscall number which was handed in by the caller

- a modified syscall number (ptrace, seccomp, trace/probe/bpf)

That has an additional twist. If the return value is -1L then the caller is not allowed to modify the return value as that indicates that the modifying entity requests to abort the syscall and set the return value already. That can obviously not be differentiated from a syscall which handed in -1 as syscall number.

The most trivial way to deal with that is:

    set_return_value(regs, -ENOSYS);
    nr = syscall_enter_from_user_mode(regs, nr);
    if (valid(nr))
    	handle_syscall(regs, nr);

Quoted 5 lines

That's what LOONGARCH, RISCV, and X86 do. But PowerPC and S390 do not preset the return value, so when user space hands in -1 and there is nothing setting the return value in the entry work code, then the syscall is skipped but the return value is whatever random data has been in the return value register.

The reason why PowerPC and S390 do not preset the return value is that the return value uses the same register as the syscall number. There are apparently other architectures on which the return value overlaps with the arguments which also do not preset the return value for that reason. If they would use the generic entry the same problem would arise.

Quoted 3 lines

That's an implementation choice of PPC/S390 as I explained before, which could trivially be solved by having an explicit pt_regs->return_val member,

The return values of syscall_enter_from_user_mode[_work]() are non-intuitive. Both functions return the syscall number which should be invoked by the architecture specific syscall entry code. The returned number can be:

  - the unmodified syscall number which was handed in by the caller

  - a modified syscall number (ptrace, seccomp, trace/probe/bpf)

That has an additional twist. If the return value is -1L then the caller is not allowed to modify the return value as that indicates that the modifying entity requests to abort the syscall and set the return value already. That can obviously not be differentiated from a syscall which handed in -1 as syscall number.

The most trivial way to deal with that is:

    set_return_value(regs, -ENOSYS);
    nr = syscall_enter_from_user_mode(regs, nr);
    if (valid(nr))
     handle_syscall(regs, nr);

That's what LOONGARCH, RISCV, and X86 do. But PowerPC and S390 do not preset the return value, so when user space hands in -1 and there is nothing setting the return value in the entry work code, then the syscall is skipped but the return value is whatever random data has been in the return value register.

Change the return values of syscall_enter_from_user_mode[_work]() to boolean and return false, when either ptrace or seccomp request to skip the syscall. If they return true, update the syscall number as it might have been changed.

That results in slightly different behaviour of the architectures versus tracing.

If the syscall tracepoint has probe/BPF attached, those might set the syscall number to -1 and also set the return value. PowerPC and S390 will then overwrite that value with -ENOSYS. The other architectures will just ignore it like any other invalid syscall and use the modified one.

Originally-by Michal Suchánek <[email protected]>

Signed-off-by Thomas Gleixner <[email protected]>

---
V2: Change the return logic so Power and S390 can insist on being special.
---
 Documentation/core-api/entry.rst |   45 ++++++++++++++++++++++++++++++--------- arch/loongarch/kernel/syscall.c  |   14 ++++++------ arch/powerpc/kernel/syscall.c    |    3 +- arch/riscv/kernel/traps.c        |   11 ++++----- arch/s390/kernel/syscall.c       |    7 ++++-- arch/x86/entry/syscall_32.c      |   25 ++++++++++----------- arch/x86/entry/syscall_64.c      |   12 +++++----- include/linux/entry-common.h     |   32 +++++++++++++--------------

 8 files changed, 88 insertions(+), 61 deletions(-)

Documentation/core-api/entry.rst +0 -0

@@ -58,26 +58,51 @@ state transitions must run with interrup

 Syscalls
 --------

Documentation/core-api/entry.rst +5 -4

-Syscall-entry code starts in assembly code and calls out into low-level C code
-after establishing low-level architecture-specific state and stack frames. This
-low-level C code must not be instrumented. A typical syscall handling function
-invoked from low-level assembly code looks like this:
+Syscall-entry code starts in assembly code and calls out into low-level C
+code after establishing low-level architecture-specific state and stack
+frames. This low-level C code must not be instrumented. The recommended
+syscall handling function invoked from low-level assembly code looks like
+this:

 .. code-block:: c

-  noinstr void syscall(struct pt_regs *regs, int nr) +  noinstr void syscall(struct pt_regs *regs, long nr) {
  arch_syscall_enter(regs);

Documentation/core-api/entry.rst +9 -1

-	nr = syscall_enter_from_user_mode_randomize_stack(regs, nr);
+	result_reg(regs) = -ENOSYS;
+	if (syscall_enter_from_user_mode_randomize_stack(regs, &nr)) {
+		instrumentation_begin();
+		if (valid(nr)
+			result_reg(regs) = invoke_syscall(regs, nr);
+		instrumentation_end();
+	}
+	syscall_exit_to_user_mode(regs);
+  }

Documentation/core-api/entry.rst +4 -4

-	instrumentation_begin();
-	if (!invoke_syscall(regs, nr) && nr != -1)
-	 	result_reg(regs) = __sys_ni_syscall(regs);
-	instrumentation_end();
+This is the most resilent variant as it has always a guaranteed valid
+return code. The alternative variant is:
+
+.. code-block:: c

+  noinstr void syscall(struct pt_regs *regs, long nr) +  {

Documentation/core-api/entry.rst +9 -0

+	arch_syscall_enter(regs);
+	if (syscall_enter_from_user_mode_randomize_stack(regs, &nr)) {
+		instrumentation_begin();
+		if (valid(nr)
+			result_reg(regs) = invoke_syscall(regs, nr);
+		else
+			result_reg(regs) = -ENOSYS;
+		instrumentation_end();
+	}

  syscall_exit_to_user_mode(regs);
   }

Documentation/core-api/entry.rst +5 -0

+That works for most situations except when a probe/BPF attached to the
+syscall tracepoint sets an invalid syscall number e.g. -1 and also modifies
+the result register. So this variant will obviously overwrite the modified
+result with -ENOSYS.
+

 syscall_enter_from_user_mode_randomize_stack() first invokes enter_from_user_mode_randomize_stack() which establishes state in the following order:

arch/loongarch/kernel/syscall.c +0 -0

@@ -57,8 +57,8 @@ typedef long (*sys_call_fn)(unsigned lon

 void noinstr __no_stack_protector do_syscall(struct pt_regs *regs) {

arch/loongarch/kernel/syscall.c +0 -1

-	unsigned long nr;

  sys_call_fn syscall_fn;

arch/loongarch/kernel/syscall.c +1 -0

+	unsigned long nr;

  nr = regs->regs[11];
  /* Set for syscall restarting */

arch/loongarch/kernel/syscall.c +0 -0

@@ -69,12 +69,12 @@ void noinstr __no_stack_protector do_sys
  	regs->orig_a0 = regs->regs[4];
  	regs->regs[4] = -ENOSYS;

arch/loongarch/kernel/syscall.c +6 -6

-	nr = syscall_enter_from_user_mode_randomize_stack(regs, nr);
-
-	if (nr < NR_syscalls) {
-		syscall_fn = sys_call_table[array_index_nospec(nr, NR_syscalls)];
-		regs->regs[4] = syscall_fn(regs->orig_a0, regs->regs[5], regs->regs[6],
-					   regs->regs[7], regs->regs[8], regs->regs[9]);
+	if (likely(syscall_enter_from_user_mode_randomize_stack(regs, &nr))) {
+		if (nr < NR_syscalls) {
+			syscall_fn = sys_call_table[array_index_nospec(nr, NR_syscalls)];
+			regs->regs[4] = syscall_fn(regs->orig_a0, regs->regs[5], regs->regs[6],
+						   regs->regs[7], regs->regs[8], regs->regs[9]);
+		}

  }

  syscall_exit_to_user_mode(regs);

arch/powerpc/kernel/syscall.c +0 -0

@@ -18,7 +18,8 @@ notrace long system_call_exception(struc
  	long ret;
  	syscall_fn f;

arch/powerpc/kernel/syscall.c +2 -1

-	r0 = syscall_enter_from_user_mode_randomize_stack(regs, r0);
+	if (unlikely(!syscall_enter_from_user_mode_randomize_stack(regs, &r0)))
+		return syscall_get_error(current, regs);

  if (unlikely(r0 >= NR_syscalls)) {
  if (unlikely(trap_is_unsupported_scv(regs))) {

arch/riscv/kernel/traps.c +0 -0

@@ -332,13 +332,12 @@ void do_trap_ecall_u(struct pt_regs *reg

  riscv_v_vstate_discard(regs);

arch/riscv/kernel/traps.c +5 -5

-		syscall = syscall_enter_from_user_mode_randomize_stack(regs, syscall);
-
-		if (syscall >= 0 && syscall < NR_syscalls) {
-			syscall = array_index_nospec(syscall, NR_syscalls);
-			syscall_handler(regs, syscall);
+		if (likely(syscall_enter_from_user_mode_randomize_stack(regs, &syscall))) {
+			if (syscall >= 0 && syscall < NR_syscalls) {
+				syscall = array_index_nospec(syscall, NR_syscalls);
+				syscall_handler(regs, syscall);
+			}

  }
-
  syscall_exit_to_user_mode(regs);
  } else {
  irqentry_state_t state = irqentry_nmi_enter(regs);

arch/s390/kernel/syscall.c +1 -0

@@ -96,6 +96,7 @@ SYSCALL_DEFINE0(ni_syscall)
  void noinstr __do_syscall(struct pt_regs *regs, int per_trap)
  {
  	unsigned long nr;
+	bool permit;

enter_from_user_mode_randomize_stack(regs);

arch/s390/kernel/syscall.c +4 -2

@@ -121,7 +122,9 @@ void noinstr __do_syscall(struct pt_regs
  		regs->psw.addr = current->restart_block.arch_data;
  		current->restart_block.arch_data = 1;
  	}
-	nr = syscall_enter_from_user_mode_work(regs, nr);
+
+	permit = syscall_enter_from_user_mode_work(regs, &nr);
+
  	/*
  	 * In the s390 ptrace ABI, both the syscall number and the return value
  	 * use gpr2. However, userspace puts the syscall number either in the

@@ -129,7 +132,7 @@ void noinstr __do_syscall(struct pt_regs
  	 * work, the ptrace code sets PIF_SYSCALL_RET_SET, which is checked here
  	 * and if set, the syscall will be skipped.
  	 */
-	if (unlikely(test_and_clear_pt_regs_flag(regs, PIF_SYSCALL_RET_SET)))
+	if (unlikely(test_and_clear_pt_regs_flag(regs, PIF_SYSCALL_RET_SET) || !permit))
  		goto out;
  	regs->gprs[2] = -ENOSYS;
  	if (likely(nr < NR_syscalls)) {

arch/x86/entry/syscall_32.c +0 -0

@@ -161,8 +161,9 @@ static __always_inline bool int80_is_ext
  	nr = syscall_32_enter(regs);

  local_irq_enable();

arch/x86/entry/syscall_32.c +3 -2

-	nr = syscall_enter_from_user_mode_work(regs, nr);
-	do_syscall_32_irqs_on(regs, nr);
+
+	if (likely(syscall_enter_from_user_mode_work(regs, &nr)))
+		do_syscall_32_irqs_on(regs, nr);

  instrumentation_end();
  syscall_exit_to_user_mode(regs);

arch/x86/entry/syscall_32.c +0 -0

@@ -223,8 +224,8 @@ DEFINE_FREDENTRY_RAW(int80_emulation)
  	nr = syscall_32_enter(regs);

  local_irq_enable();

arch/x86/entry/syscall_32.c +2 -2

-	nr = syscall_enter_from_user_mode_work(regs, nr);
-	do_syscall_32_irqs_on(regs, nr);
+	if (likely(syscall_enter_from_user_mode_work(regs, &nr)))
+		do_syscall_32_irqs_on(regs, nr);

  instrumentation_end();
  syscall_exit_to_user_mode(regs);

arch/x86/entry/syscall_32.c +2 -3

@@ -243,13 +244,13 @@ DEFINE_FREDENTRY_RAW(int80_emulation)
  	 * orig_ax, the int return value truncates it. This matches
  	 * the semantics of syscall_get_nr().
  	 */
-	nr = syscall_enter_from_user_mode_randomize_stack(regs, nr);
-
-	instrumentation_begin();
+	if (likely(syscall_enter_from_user_mode_randomize_stack(regs, &nr))) {
+		instrumentation_begin();

arch/x86/entry/syscall_32.c +1 -1

-	do_syscall_32_irqs_on(regs, nr);
+		do_syscall_32_irqs_on(regs, nr);

arch/x86/entry/syscall_32.c +2 -1

-	instrumentation_end();
+		instrumentation_end();
+	}

  syscall_exit_to_user_mode(regs);
 }
 #endif /* !CONFIG_IA32_EMULATION */

arch/x86/entry/syscall_32.c +0 -0

@@ -286,10 +287,8 @@ static noinstr bool __do_fast_syscall_32
  		return false;
  	}

arch/x86/entry/syscall_32.c +2 -4

-	nr = syscall_enter_from_user_mode_work(regs, nr);
-
-	/* Now this is just like a normal syscall. */
-	do_syscall_32_irqs_on(regs, nr);
+	if (likely(syscall_enter_from_user_mode_work(regs, &nr)))
+		do_syscall_32_irqs_on(regs, nr);

  instrumentation_end();
  syscall_exit_to_user_mode(regs);

arch/x86/entry/syscall_64.c +2 -1

@@ -78,14 +78,14 @@ static __always_inline void do_syscall_x
  /* Returns true to return using SYSRET, or false to use IRET */
  __visible noinstr bool do_syscall_64(struct pt_regs *regs, long nr)
  {
-	nr = syscall_enter_from_user_mode_randomize_stack(regs, nr);
+	if (likely(syscall_enter_from_user_mode_randomize_stack(regs, &nr))) {
+		instrumentation_begin();

arch/x86/entry/syscall_64.c +2 -1

-	instrumentation_begin();
+		if (!do_syscall_x64(regs, nr))
+			do_syscall_x32(regs, nr);

arch/x86/entry/syscall_64.c +2 -4

-	if (!do_syscall_x64(regs, nr))
-		do_syscall_x32(regs, nr);
-
-	instrumentation_end();
+		instrumentation_end();
+	}

  syscall_exit_to_user_mode(regs);

  /*

include/linux/entry-common.h +8 -9

@@ -111,16 +111,15 @@ static __always_inline long syscall_trac
   * @regs:	Pointer to currents pt_regs
   * @syscall:	The syscall number
   *
- * Invoked from architecture specific syscall entry code with interrupts
- * enabled after invoking enter_from_user_mode(), enabling interrupts and
- * extra architecture specific work.
+ * Invoked from architecture specific syscall entry code with interrupts enabled
+ * after invoking enter_from_user_mode(), enabling interrupts and extra
+ * architecture specific work with the syscall return value preset to -ENOSYS.
   *
- * Returns: The original or a modified syscall number
+ * Returns: True if the syscall should be invoked, False otherwise.
   *
- * If the returned syscall number is -1 then the syscall should be
- * skipped. In this case the caller may invoke syscall_set_error() or
- * syscall_set_return_value() first.  If neither of those are called and -1
- * is returned, then the syscall will fail with ENOSYS.
+ * If the return value is false, the caller must skip the syscall and leave the
+ * syscall return value unmodified as it might have been set by one of the entry
+ * work functions.
   *
   * It handles the following work items:
   *

@@ -128,19 +127,20 @@ static __always_inline long syscall_trac
   *     ptrace_report_syscall_permit_entry(), __seccomp_permit_syscall(), trace_sys_enter()
   *  2) Invocation of audit_syscall_entry()
   */
-static __always_inline long syscall_enter_from_user_mode_work(struct pt_regs *regs, long syscall)
+static __always_inline bool syscall_enter_from_user_mode_work(struct pt_regs *regs, long *syscall)
  {
  	unsigned long work = READ_ONCE(current_thread_info()->syscall_work);

include/linux/entry-common.h +2 -3

-	if (work & SYSCALL_WORK_ENTER) {
-		if (!syscall_trace_enter(regs, work, syscall))
-			return -1L;
+	if (!(work & SYSCALL_WORK_ENTER))
+		return true;

include/linux/entry-common.h +2 -3

-		/* Reread the syscall number as it might have been modified */
-		syscall = syscall_get_nr(current, regs);
-	}
+	if (unlikely(!syscall_trace_enter(regs, work, *syscall)))
+		return false;

include/linux/entry-common.h +4 -1

-	return syscall;
+	/* Reread the syscall number as it might have been modified */
+	*syscall = syscall_get_nr(current, regs);
+
+	return true;

 }

 /**

This are the reworked leftovers of the larger entry logic rework series, which can be found here:

https://lore.kernel.org/lkml/[email protected]

The undisputed part of the series has been merged into:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core/entry

After a fruitless discussion about the most resilent approach, I decided to give up wasting my time and reworked the series so it caters to the desire of powerpc and s390 to be special.

That results in almost identical behavior except for the case where tracing/probe/BPF sets the syscall number to -1 (or any other out of range value) and also sets the syscall return value to something different than -ENOSYS.

PowerPC and S390 will overwrite that value with -ENOSYS.

Loongarch, RISC-V and x86 will not overwrite it because those architectures preset the return value to -ENOSYS and skip out of range syscalls completely. Loongarch and RISC-V always did the out of range skip. X86 gained that in the already applied rework, which means that the final patch which splits the skip decision from the syscall number does not longer change x86 behavior for the above case.

Documentation is also updated and describes the two implementation variants and the subtle difference in the resulting behavior.

The series applies on top of:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core/entry

and is also available from git:

git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git entry/rework