Re: [RFC] entry: Untangle the return value of syscall_enter_from_user_mode from syscall NR

On Thu, Jul 02, 2026 at 01:24:57PM +0200, Thomas Gleixner wrote:

On Wed, Jul 01 2026 at 19:42, Michal Suchánek wrote:

quoted

The return value of syscall_enter_from_user_mode is used both for the
adjusted syscall number and the indicator that a syscall should be
skipped.

As seccomp can be invoked on any syscall, including invalid ones this
somewhat undermines seccomp.

While the seccomp variants that terminate the process do not need to
care about this for the filter that sets the syscall return value this
disctinction is required.

You completely fail to explain why and what actual problem you are
trying to solve. At least I can't figure it out from the above word
salad.

syscall_enter_from_user_mode returns the new syscall number after doing
something arbitrarry with it, including running seccomp.

Wehn the syscall is already handled, eg. by seccomp filtering it returns
-1 as the new syscall number. -1 is an invalid syscall number but it can
still be filtered by seccomp. When the syscall number was -1 to start
with it's not possible to determine if the syscall was fileterd from the
return value. s390 returns the filtered state in a flag it sets on the
regs structure, avoiding this problem.

However, the API should be specified in a way that does not require
everyone implementing such flag.

quoted

Pass the syscall number as a pointer to the inline entry functions, and
use the return value exclusively for the indication that the syscall is
already handled.

This should avoid the need for the s390 PIF_SYSCALL_RET_SET which is the
workaround for exactly this deficiency.

If this is desirable the patch could be split into some series that
adjusts the code flow where needed so that the final change is mostly
mechanical.

That's not a matter of desire. That's mandatory.

So long as it's desirable to implement an API change in this direction,
it's not clear to me so far.

quoted

-	instrumentation_begin();
-	if (!invoke_syscall(regs, nr) && nr != -1)
-	 	result_reg(regs) = __sys_ni_syscall(regs);
-	instrumentation_end();
+	/* Skip syscall when -1 is returned */
+	if (!syscall_enter_from_user_mode(regs, &nr)) {

Seriously?

If we go and separate the syscall number from the return value, then the
return value 0 means success and anything else fail. Which in other
words is a boolean. So instead of tastelessly adding a completely
nonsensical comment about -1 here, syscall_enter_from_user_mode() wants
to have the return value type bool with a proper boolean logic: true =
success, false = abort.

We have that very same API down to __secure_computing() which returns
boolean represented as -1 and 0 values. That does not mean it's not
tasteless.

quoted

@@ -168,8 +168,7 @@ __visible noinstr void do_int80_emulation(struct pt_regs *regs)
 	nr = syscall_32_enter(regs);
 
 	local_irq_enable();
-	nr = syscall_enter_from_user_mode_work(regs, nr);
-	do_syscall_32_irqs_on(regs, nr);
+	syscall_enter_from_user_mode_work(regs, &nr);

How exactly is this ever going to invoke a valid syscall?

That's one of the problems with giant all-in-one patch, things like this
easily slip in. However, it is in cluded mostly for illustration, I
don't expect anyone to merge this as-is.

quoted

+	if (!syscall_enter_from_user_mode_work(regs, &nr)) {
+		nr &= GENMASK(31, 0);
+		do_syscall_32_irqs_on(regs, nr);

  do_syscall_32_irqs_on(regs, (int)nr);

would be too simple, right?

Also way less explicit.

Thanks

Michal