Re: [patch 16/18] x86/entry: Get rid of the sys_ni_syscall() indirection

[H. Peter Anvin]

On October 22, 2026 2:34:08 PM PDT, Thomas Gleixner <tglx@xxxxxxxxxx> wrote:
>On Thu, Oct 22 2026 at 21:15, Thomas Gleixner wrote:
>> On Thu, Oct 22 2026 at 16:24, Sebastian Andrzej Siewior wrote:
>>> On 2026-07-07 21:07:01 [+0200], Thomas Gleixner wrote:
>>> |[RUN]           Checking out of range system calls
>>> |[FAIL]                  x32 syscall 1:-64 returned -9999, but it should have returned -ENOSYS
>>> |[FAIL]              x32 syscalls 1:-64..-2 had 63 failure
>>> |[OK]                x32 syscall 1:-1 returned MODIFIED_BY_PTRACE as expected
>>> |[FAIL]                  x64 syscall 1:1073741760 returned -9999, but it should have returned -ENOSYS
>>> …
>>>
>>> and so on… The test has
>>>
>>> | #define MODIFIED_BY_PTRACE      -9999
>>>
>>> and the ptracer modifies the return code. However not always. Not sure
>>> if this was indeed a pointless exercise or not.
>>
>> Hrmm. That's weird. Let me have a look.
>
>So I clearly failed to decode the overloading of the -1L return value
>correctly and then failed to run the stupid test.
>
>TBH, this whole thing was a horrid and inconsistent mess.
>
>The logic in the original code was:
>
>    When the entry work code returned an invalid syscall number then the
>    return value in regs->ax was set to -ENOSYS, except when the
>    returned syscall number was -1. Syscall number "-1" indicated that
>    the syscall has to be aborted.
>
>     regs->ax = -ENOSYS;
>     syscall = enter_work(regs, syscall);
>     if (syscall_in_range(syscall))
>        regs->ax = do_syscall(syscall);
>     else if (syscall != -1)
>        regs->ax = -ENOSYS;
>
>So if the ptrace check in enter_work() overwrote regs->ax and the
>syscall number (either as supplied by user space or rewritten) was out
>of range and not -1 then the x86 entry code overwrote regs->ax to
>-ENOSYS again. IOW, there is no distinction between a check induced
>abort and a syscall with syscall number -1.
>
>The rework changed the return value to boolean, which is true when the
>syscall should be invoked and false when one of the checks requests to
>abort the syscall. In that case the return code does not touch the
>return value in regs->ax.
>
>If the eventually modified syscall number is out of range then the code
>relies on the fact that regs->ax was preset with -ENOSYS, i.e.:
>
>     regs->ax = -ENOSYS;
>     // True indicates to proceed
>     if (enter_work(regs, &syscall)) {
>         if (syscall_in_range(syscall))
>            regs->ax = do_syscall(syscall);
>     }
>
>That seems to be equivalent, but it is not when ptrace() modifies
>the return value in regs->ax and also rewrites the syscall number to an
>invalid value or leaves the user supplied invalid value intact and does
>_NOT_ abort the syscall.
>
>So the reworked code does not overwrite the tracer modified return value
>with -ENOSYS in that case and returns whatever the tracer decided to put
>into it.
>
>TBH, that makes a lot more sense than the original behavior. If the
>tracer decides to fiddle with the default return value then that's what
>the tracer intended.
>
>That makes the behavior consistent vs. other abort scenarios:
>
>    ptrace()
>      regs->ax = MAGIC;
>      return true;            // Allow syscall
>
>    seccomp()
>      return false;          // Abort syscall
>
>So when the tracer modified the return value and seccomp aborts the
>syscall then the tracer induced value is preserved. That was the case in
>the original code as well because seccomp() indicated the abort by
>returning -1 as the modified syscall number. Similar effects when ftrace
>(BPF, kprobe) decided to overwrite the syscall number).
>
>Bringing the original behaviour back is trivial enough, but I'm not
>convinced that it makes any sense and is worth it. At least this
>confines the x32 specific insanity of treating -1 special to x32...
>
>Thanks,
>
>        tglx
>---
>--- a/arch/x86/entry/syscall_32.c
>+++ b/arch/x86/entry/syscall_32.c
>@@ -77,6 +77,8 @@ static __always_inline void do_syscall_3
> 	if (likely(nr < IA32_NR_syscalls)) {
> 		nr = array_index_nospec(nr, IA32_NR_syscalls);
> 		regs->ax = ia32_sys_call(regs, (unsigned int)nr);
>+	} else {
>+		regs->ax = -ENOSYS;
> 	}
> }
> 
>--- a/arch/x86/entry/syscall_64.c
>+++ b/arch/x86/entry/syscall_64.c
>@@ -64,15 +64,24 @@ static __always_inline bool do_syscall_x
> 	return false;
> }
> 
>-static __always_inline void do_syscall_x32(struct pt_regs *regs, unsigned long nr)
>+static __always_inline bool do_syscall_x32(struct pt_regs *regs, unsigned long nr)
> {
>+	if (!IS_ENABLED(CONFIG_X86_X32_ABI))
>+		return false;
>+
>+	/* Preserve the historic insanity of treating -1 special on x32 */
>+	if (nr == ULONG_MAX)
>+		return true;
>+
> 	/* Adjust the starting offset of the table */
> 	nr -= __X32_SYSCALL_BIT;
> 
>-	if (IS_ENABLED(CONFIG_X86_X32_ABI) && likely(nr < X32_NR_syscalls)) {
>+	if (likely(nr < X32_NR_syscalls)) {
> 		nr = array_index_nospec(nr, X32_NR_syscalls);
> 		regs->ax = x32_sys_call(regs, (unsigned int)nr);
>+		return true;
> 	}
>+	return false;
> }
> 
> /* Returns true to return using SYSRET, or false to use IRET */
>@@ -81,8 +90,10 @@ static __always_inline void do_syscall_x
> 	if (likely(syscall_enter_from_user_mode_randomize_stack(regs, &nr))) {
> 		instrumentation_begin();
> 
>-		if (!do_syscall_x64(regs, nr))
>-			do_syscall_x32(regs, nr);
>+		if (unlikely(!do_syscall_x64(regs, nr))) {
>+			if (!do_syscall_x32(regs, nr))
>+				regs->ax = -ENOSYS;
>+		}
> 
> 		instrumentation_end();
> 	}
>
>
>
>
>
>
>
>
>
>
>

It was never limited to x32...