Hi Jinjie,
On 29/06/2026 14:05, Jinjie Ruan wrote:
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.
Would it be relevant to mention that this fixes the unsound return value of
`syscall_trace_enter()` in generic entry, which motivated the patch
initially[0] ?
Suggested-by: Thomas Gleixner <[email protected]>
Signed-off-by: Jinjie Ruan <[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 | 2 +-
arch/xtensa/kernel/ptrace.c | 3 +--
include/linux/entry-common.h | 7 +++---
include/linux/seccomp.h | 10 ++++----
kernel/seccomp.c | 34 +++++++++++++--------------
14 files changed, 36 insertions(+), 38 deletions(-)
This is missing an update to the Kconfig documentation, a possible
suggestion :
diff --git a/arch/Kconfig b/arch/Kconfig
index fa7507ac8e13..9e3d40088afb 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -637,7 +637,7 @@ config HAVE_ARCH_SECCOMP_FILTER
- 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
+ - secure_computing return value is checked and if false
results in the system call being skipped immediately.
- seccomp syscall wired up
- if !HAVE_SPARSE_SYSCALL_NR, have SECCOMP_ARCH_NATIVE,
[...]
diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
index 416a3352261f..3f66320e46d3 100644
--- a/include/linux/entry-common.h
+++ b/include/linux/entry-common.h
@@ -100,9 +100,8 @@ static __always_inline long syscall_trace_enter(struct
pt_regs *regs, unsigned l
/* Do seccomp after ptrace, to catch any tracer changes. */
if (work & SYSCALL_WORK_SECCOMP) {
- ret = __secure_computing();
- if (ret == -1L)
- return ret;
+ if (!__secure_computing())
+ return -1L;
}
/* Either of the above might have changed the syscall number */
@@ -113,7 +112,7 @@ static __always_inline long syscall_trace_enter(struct
pt_regs *regs, unsigned l
syscall_enter_audit(regs, syscall);
- return ret ? : syscall;
+ return syscall;
}
[...]
Otherwise this feels like a more appropriate change with regards to
"safeguarding against new `secure_computing()` return value" !
With the updated Kconfig :
Reviewed-by: Ada Couprie Diaz <[email protected]>
Thanks,
Ada
[0]:
https://lore.kernel.org/r/[email protected]
