On 7/8/2026 3:06 AM, Thomas Gleixner wrote:
> From: Jinjie Ruan <[email protected]>
> 
> The return value of __secure_computing() currently uses 0 to indicate
> that a system call should be allowed, and -1 to indicate that it should
> be blocked/killed. This 0/-1 pattern is non-intuitive for a security
> check function and makes the control flow at the call sites less readable.
> 
> Furthermore, any potential future changes to these return values would
> require a high-risk, error-prone audit of all its users across different
> architectures.
> 
> Sanitize this logic by converting the return type of __secure_computing()
> to a proper boolean, where 'true' explicitly means 'allow' and 'false'
> means 'fail/deny'.
> 
> Update all the two dozen or so call sites across the tree to align with
> this new boolean semantic. No functional changes are intended, as the
> callers still return -1 to the lower-level assembly entry code upon
> seccomp denial.
> 
> Rename the function to __seccomp_permit_syscall() so that the purpose is
> entirely clear.
> 
> [ tglx: Rename the function ]
> 
> Suggested-by: Thomas Gleixner <[email protected]>
> Suggested-by: Mark Rutland <[email protected]>
> Signed-off-by: Jinjie Ruan <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Oleg Nesterov <[email protected]>
> Cc: Richard Henderson <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Guo Ren <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Helge Deller <[email protected]>
> Cc: Yoshinori Sato <[email protected]>
> Cc: Richard Weinberger <[email protected]>
> Cc: Chris Zankel <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
>  arch/alpha/kernel/ptrace.c            |    2 -
>  arch/arm/kernel/ptrace.c              |    2 -
>  arch/arm64/kernel/ptrace.c            |    2 -
>  arch/csky/kernel/ptrace.c             |    2 -
>  arch/m68k/kernel/ptrace.c             |    2 -
>  arch/mips/kernel/ptrace.c             |    2 -
>  arch/parisc/kernel/ptrace.c           |    2 -
>  arch/sh/kernel/ptrace_32.c            |    2 -
>  arch/um/kernel/skas/syscall.c         |    2 -
>  arch/x86/entry/vsyscall/vsyscall_64.c |   14 ++++++-------
>  arch/xtensa/kernel/ptrace.c           |    3 --
>  include/linux/entry-common.h          |    9 +++-----
>  include/linux/seccomp.h               |   12 +++++------
>  kernel/seccomp.c                      |   35 
> +++++++++++++++++-----------------
>  14 files changed, 45 insertions(+), 46 deletions(-)

As Ada pointed out, the description of secure_computing in arch/Kconfig
need to be updated, a possible suggestion:

--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -636,8 +636,8 @@ config HAVE_ARCH_SECCOMP_FILTER
          - syscall_rollback()
          - syscall_set_return_value()
          - SIGSYS siginfo_t support
-         - secure_computing is called from a ptrace_event()-safe context
-         - secure_computing return value is checked and a return value
of -1
+         - seccomp_permits_syscall is called from a ptrace_event()-safe
context
+         - seccomp_permits_syscall return value is checked and if false


Link:
https://lore.kernel.org/all/[email protected]/

> --- a/arch/alpha/kernel/ptrace.c
> +++ b/arch/alpha/kernel/ptrace.c
> @@ -387,7 +387,7 @@ asmlinkage unsigned long syscall_trace_e
>        * If this fails, seccomp may already have set up the return value
>        * (e.g. SECCOMP_RET_ERRNO / TRACE).
>        */
> -     if (secure_computing() == -1) {
> +     if (!seccomp_permit_syscall()) {
>               if (regs->r19 == 0 && regs->r0 == (unsigned long)-1)
>                       syscall_set_return_value(current, regs, -ENOSYS, 0);
>               syscall_set_nr(current, regs, -1);

[...]

> -static int __seccomp_filter(int this_syscall, const bool recheck_after_trace)
> +static bool __seccomp_filter(int this_syscall, const bool 
> recheck_after_trace)
>  {
>       u32 filter_ret, action;
>       struct seccomp_data sd;
> @@ -1294,7 +1295,7 @@ static int __seccomp_filter(int this_sys
>       case SECCOMP_RET_TRACE:
>               /* We've been put in this state by the ptracer already. */
>               if (recheck_after_trace)
> -                     return 0;
> +                     return true;
>  
>               /* ENOSYS these calls if there is no tracer attached. */
>               if (!ptrace_event_enabled(current, PTRACE_EVENT_SECCOMP)) {
> @@ -1330,19 +1331,19 @@ static int __seccomp_filter(int this_sys
>                * a skip would have already been reported.
>                */
>               if (__seccomp_filter(this_syscall, true))
> -                     return -1;
> +                     return false;

The return value of __seccomp_filter is checked in the wrong way, check
-1 should be replaced with check false, maybe:

-               if (__seccomp_filter(this_syscall, true))
-                       return -1;
+               if (!__seccomp_filter(this_syscall, true))
+                       return false;

otherwise,

LGTM
Reviewed-by: Jinjie Ruan <[email protected]>

>  
> -             return 0;
> +             return true;
>  
>       case SECCOMP_RET_USER_NOTIF:
>               if (seccomp_do_user_notification(this_syscall, match, &sd))
>                       goto skip;
>  
> -             return 0;
> +             return true;
>  
>       case SECCOMP_RET_LOG:
>               seccomp_log(this_syscall, 0, action, true);
> -             return 0;
> +             return true;
>  
>       case SECCOMP_RET_ALLOW:
>               /*
> @@ -1350,7 +1351,7 @@ static int __seccomp_filter(int this_sys
>                * this action since SECCOMP_RET_ALLOW is the starting
>                * state in seccomp_run_filters().
>                */
> -             return 0;
> +             return true;
>  
>       case SECCOMP_RET_KILL_THREAD:
>       case SECCOMP_RET_KILL_PROCESS:
> @@ -1367,46 +1368,46 @@ static int __seccomp_filter(int this_sys
>               } else {
>                       do_exit(SIGSYS);
>               }
> -             return -1; /* skip the syscall go directly to signal handling */
> +             return false; /* skip the syscall go directly to signal 
> handling */
>       }
>  
>       unreachable();
>  
>  skip:
>       seccomp_log(this_syscall, 0, action, match ? match->log : false);
> -     return -1;
> +     return false;
>  }
>  #else
> -static int __seccomp_filter(int this_syscall, const bool recheck_after_trace)
> +static bool __seccomp_filter(int this_syscall, const bool 
> recheck_after_trace)
>  {
>       BUG();
>  
> -     return -1;
> +     return false;
>  }
>  #endif
>  
> -int __secure_computing(void)
> +bool __seccomp_permit_syscall(void)
>  {
>       int mode = current->seccomp.mode;
>       int this_syscall;
>  
>       if (IS_ENABLED(CONFIG_CHECKPOINT_RESTORE) &&
>           unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
> -             return 0;
> +             return true;
>  
>       this_syscall = syscall_get_nr(current, current_pt_regs());
>  
>       switch (mode) {
>       case SECCOMP_MODE_STRICT:
>               __secure_computing_strict(this_syscall);  /* may call do_exit */
> -             return 0;
> +             return true;
>       case SECCOMP_MODE_FILTER:
>               return __seccomp_filter(this_syscall, false);
>       /* Surviving SECCOMP_RET_KILL_* must be proactively impossible. */
>       case SECCOMP_MODE_DEAD:
>               WARN_ON_ONCE(1);
>               do_exit(SIGKILL);
> -             return -1;
> +             return false;
>       default:
>               BUG();
>       }
> 


Reply via email to