mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-09-18 23:09:29 +02:00
seccomp, treewide: Rename and convert __secure_computing() to return boolean
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 <tglx@kernel.org> Suggested-by: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> Signed-off-by: Thomas Gleixner <tglx@kernel.org> Tested-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com> Reviewed-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com> Acked-by: Oleg Nesterov <oleg@redhat.com> Link: https://patch.msgid.link/20260707190254.230735780@kernel.org
This commit is contained in:
committed by
Thomas Gleixner
parent
8af25d0a2e
commit
7ba2ba7471
+1
-1
@@ -647,7 +647,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 it
|
||||
results in the system call being skipped immediately.
|
||||
- seccomp syscall wired up
|
||||
- if !HAVE_SPARSE_SYSCALL_NR, have SECCOMP_ARCH_NATIVE,
|
||||
|
||||
@@ -387,7 +387,7 @@ asmlinkage unsigned long syscall_trace_enter(void)
|
||||
* 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);
|
||||
|
||||
@@ -855,7 +855,7 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs)
|
||||
|
||||
/* Do seccomp after ptrace; syscall may have changed. */
|
||||
#ifdef CONFIG_HAVE_ARCH_SECCOMP_FILTER
|
||||
if (secure_computing() == -1)
|
||||
if (!seccomp_permit_syscall())
|
||||
return -1;
|
||||
#else
|
||||
/* XXX: remove this once OABI gets fixed */
|
||||
|
||||
@@ -2420,7 +2420,7 @@ int syscall_trace_enter(struct pt_regs *regs)
|
||||
}
|
||||
|
||||
/* Do the secure computing after ptrace; failures should be fast. */
|
||||
if (secure_computing() == -1)
|
||||
if (!seccomp_permit_syscall())
|
||||
return NO_SYSCALL;
|
||||
|
||||
if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
|
||||
|
||||
@@ -323,7 +323,7 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs)
|
||||
if (ptrace_report_syscall_entry(regs))
|
||||
return -1;
|
||||
|
||||
if (secure_computing() == -1)
|
||||
if (!seccomp_permit_syscall())
|
||||
return -1;
|
||||
|
||||
if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
|
||||
|
||||
@@ -281,7 +281,7 @@ asmlinkage int syscall_trace_enter(void)
|
||||
if (test_thread_flag(TIF_SYSCALL_TRACE))
|
||||
ret = ptrace_report_syscall_entry(task_pt_regs(current));
|
||||
|
||||
if (secure_computing() == -1)
|
||||
if (!seccomp_permit_syscall())
|
||||
return -1;
|
||||
|
||||
return ret;
|
||||
|
||||
@@ -1328,7 +1328,7 @@ asmlinkage long syscall_trace_enter(struct pt_regs *regs)
|
||||
return -1;
|
||||
}
|
||||
|
||||
if (secure_computing())
|
||||
if (!seccomp_permit_syscall())
|
||||
return -1;
|
||||
|
||||
if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
|
||||
|
||||
@@ -351,7 +351,7 @@ long do_syscall_trace_enter(struct pt_regs *regs)
|
||||
}
|
||||
|
||||
/* Do the secure computing check after ptrace. */
|
||||
if (secure_computing() == -1)
|
||||
if (!seccomp_permit_syscall())
|
||||
return -1;
|
||||
|
||||
#ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS
|
||||
|
||||
@@ -460,7 +460,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
|
||||
return -1;
|
||||
}
|
||||
|
||||
if (secure_computing() == -1)
|
||||
if (!seccomp_permit_syscall())
|
||||
return -1;
|
||||
|
||||
if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
|
||||
|
||||
@@ -27,7 +27,7 @@ void handle_syscall(struct uml_pt_regs *r)
|
||||
goto out;
|
||||
|
||||
/* Do the seccomp check after ptrace; failures should be fast. */
|
||||
if (secure_computing() == -1)
|
||||
if (!seccomp_permit_syscall())
|
||||
goto out;
|
||||
|
||||
syscall = UPT_SYSCALL_NR(r);
|
||||
|
||||
@@ -118,10 +118,10 @@ static bool write_ok_or_segv(unsigned long ptr, size_t size)
|
||||
|
||||
static bool __emulate_vsyscall(struct pt_regs *regs, unsigned long address)
|
||||
{
|
||||
unsigned long caller;
|
||||
int vsyscall_nr, syscall_nr, tmp;
|
||||
unsigned long caller, orig_dx;
|
||||
int vsyscall_nr, syscall_nr;
|
||||
bool skip;
|
||||
long ret;
|
||||
unsigned long orig_dx;
|
||||
|
||||
/* Confirm that the fault happened in 64-bit user mode */
|
||||
if (!user_64bit_mode(regs))
|
||||
@@ -197,16 +197,16 @@ static bool __emulate_vsyscall(struct pt_regs *regs, unsigned long address)
|
||||
*/
|
||||
regs->orig_ax = syscall_nr;
|
||||
regs->ax = -ENOSYS;
|
||||
tmp = secure_computing();
|
||||
if ((!tmp && regs->orig_ax != syscall_nr) || regs->ip != address) {
|
||||
skip = !seccomp_permit_syscall();
|
||||
if ((!skip && regs->orig_ax != syscall_nr) || regs->ip != address) {
|
||||
warn_bad_vsyscall(KERN_DEBUG, regs,
|
||||
"seccomp tried to change syscall nr or ip");
|
||||
force_exit_sig(SIGSYS);
|
||||
return true;
|
||||
}
|
||||
regs->orig_ax = -1;
|
||||
if (tmp)
|
||||
goto do_ret; /* skip requested */
|
||||
if (skip)
|
||||
goto do_ret;
|
||||
|
||||
/*
|
||||
* With a real vsyscall, page faults cause SIGSEGV.
|
||||
|
||||
@@ -553,8 +553,7 @@ int do_syscall_trace_enter(struct pt_regs *regs)
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (regs->syscall == NO_SYSCALL ||
|
||||
secure_computing() == -1) {
|
||||
if (regs->syscall == NO_SYSCALL || !seccomp_permit_syscall()) {
|
||||
do_syscall_trace_leave(regs);
|
||||
return 0;
|
||||
}
|
||||
|
||||
@@ -102,9 +102,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 (!__seccomp_permit_syscall())
|
||||
return -1L;
|
||||
}
|
||||
|
||||
/* Either of the above might have changed the syscall number */
|
||||
@@ -115,7 +114,7 @@ static __always_inline long syscall_trace_enter(struct pt_regs *regs, unsigned l
|
||||
|
||||
syscall_enter_audit(regs, syscall);
|
||||
|
||||
return ret ? : syscall;
|
||||
return syscall;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -138,7 +137,7 @@ static __always_inline long syscall_trace_enter(struct pt_regs *regs, unsigned l
|
||||
* It handles the following work items:
|
||||
*
|
||||
* 1) syscall_work flag dependent invocations of
|
||||
* ptrace_report_syscall_entry(), __secure_computing(), trace_sys_enter()
|
||||
* ptrace_report_syscall_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)
|
||||
|
||||
@@ -22,14 +22,14 @@
|
||||
#include <linux/atomic.h>
|
||||
#include <asm/seccomp.h>
|
||||
|
||||
extern int __secure_computing(void);
|
||||
extern bool __seccomp_permit_syscall(void);
|
||||
|
||||
#ifdef CONFIG_HAVE_ARCH_SECCOMP_FILTER
|
||||
static inline int secure_computing(void)
|
||||
static __always_inline bool seccomp_permit_syscall(void)
|
||||
{
|
||||
if (unlikely(test_syscall_work(SECCOMP)))
|
||||
return __secure_computing();
|
||||
return 0;
|
||||
return __seccomp_permit_syscall();
|
||||
return true;
|
||||
}
|
||||
#else
|
||||
extern void secure_computing_strict(int this_syscall);
|
||||
@@ -50,11 +50,11 @@ static inline int seccomp_mode(struct seccomp *s)
|
||||
struct seccomp_data;
|
||||
|
||||
#ifdef CONFIG_HAVE_ARCH_SECCOMP_FILTER
|
||||
static inline int secure_computing(void) { return 0; }
|
||||
static inline bool seccomp_permit_syscall(void) { return true; }
|
||||
#else
|
||||
static inline void secure_computing_strict(int this_syscall) { return; }
|
||||
#endif
|
||||
static inline int __secure_computing(void) { return 0; }
|
||||
static inline bool __seccomp_permit_syscall(void) { return true; }
|
||||
|
||||
static inline long prctl_get_seccomp(void)
|
||||
{
|
||||
|
||||
+17
-19
@@ -1100,12 +1100,13 @@ void secure_computing_strict(int this_syscall)
|
||||
else
|
||||
BUG();
|
||||
}
|
||||
int __secure_computing(void)
|
||||
|
||||
bool __seccomp_permit_syscall(void)
|
||||
{
|
||||
int this_syscall = syscall_get_nr(current, current_pt_regs());
|
||||
|
||||
secure_computing_strict(this_syscall);
|
||||
return 0;
|
||||
return true;
|
||||
}
|
||||
#else
|
||||
|
||||
@@ -1256,7 +1257,7 @@ out:
|
||||
return -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_syscall, const bool recheck_after_trace)
|
||||
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)) {
|
||||
@@ -1329,20 +1330,17 @@ static int __seccomp_filter(int this_syscall, const bool recheck_after_trace)
|
||||
* a reload of all registers. This does not goto skip since
|
||||
* a skip would have already been reported.
|
||||
*/
|
||||
if (__seccomp_filter(this_syscall, true))
|
||||
return -1;
|
||||
|
||||
return 0;
|
||||
return __seccomp_filter(this_syscall, 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 +1348,7 @@ static int __seccomp_filter(int this_syscall, const bool recheck_after_trace)
|
||||
* 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 +1365,46 @@ static int __seccomp_filter(int this_syscall, const bool recheck_after_trace)
|
||||
} 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();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user