From: Will Drewry <wad@chromium.org>
To: linux-kernel@vger.kernel.org
Cc: mcgrathr@google.com, hpa@zytor.com, indan@nul.nu,
netdev@parisplace.org, linux-security-module@vger.kernel.org,
kernel-hardening@lists.openwall.com, mingo@redhat.com,
oleg@redhat.com, peterz@infradead.org, rdunlap@xenotime.net,
tglx@linutronix.de, luto@mit.edu, serge.hallyn@canonical.com,
pmoore@redhat.com, akpm@linux-foundation.org, corbet@lwn.net,
markus@chromium.org, coreyb@linux.vnet.ibm.com,
keescook@chromium.org, viro@zeniv.linux.org.uk,
jmorris@namei.org, Will Drewry <wad@chromium.org>
Subject: [RFC PATCH 3/3] arch/*: move secure_computing after trace
Date: Thu, 24 May 2012 11:08:01 -0500 [thread overview]
Message-ID: <1337875681-20717-4-git-send-email-wad@chromium.org> (raw)
In-Reply-To: <1337875681-20717-1-git-send-email-wad@chromium.org>
At present, any ptracer can bypass the seccomp system call
policy by changing the system call after secure_computing has
been called. This change ensures that secure_computing
occurs after all other userspace applications may have changed the
system call value.
(Note, ARM was the most affected in this patch as the call was moved
into the syscall_trace path and reordering after ptrace was
non-trivial.)
This change is wildly untested.
Signed-off-by: Will Drewry <wad@chromium.org>
---
arch/arm/kernel/entry-common.S | 7 +------
arch/arm/kernel/ptrace.c | 42 +++++++++++++++++++--------------------
arch/microblaze/kernel/ptrace.c | 4 ++--
arch/mips/kernel/ptrace.c | 16 ++++++---------
arch/powerpc/kernel/ptrace.c | 5 +++--
arch/s390/kernel/ptrace.c | 6 +++---
arch/sh/kernel/ptrace_32.c | 5 +++--
arch/sh/kernel/ptrace_64.c | 5 +++--
arch/sparc/kernel/ptrace_64.c | 7 ++++---
9 files changed, 46 insertions(+), 51 deletions(-)
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index 7bd2d3c..57a5bde 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -416,12 +416,7 @@ ENTRY(vector_swi)
#ifdef CONFIG_SECCOMP
tst r10, #_TIF_SECCOMP
- beq 1f
- mov r0, scno
- bl __secure_computing
- add r0, sp, #S_R0 + S_OFF @ pointer to regs
- ldmia r0, {r0 - r3} @ have to reload r0 - r3
-1:
+ bne __sys_trace @ are we in seccomp mode?
#endif
tst r10, #_TIF_SYSCALL_WORK @ are we tracing syscalls?
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 14e3826..1654071 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -909,32 +909,32 @@ long arch_ptrace(struct task_struct *child, long request,
asmlinkage int syscall_trace(int why, struct pt_regs *regs, int scno)
{
- unsigned long ip;
-
- if (why)
- audit_syscall_exit(regs);
- else
- audit_syscall_entry(AUDIT_ARCH_ARM, scno, regs->ARM_r0,
- regs->ARM_r1, regs->ARM_r2, regs->ARM_r3);
-
- if (!test_thread_flag(TIF_SYSCALL_TRACE))
- return scno;
-
- current_thread_info()->syscall = scno;
-
/*
* IP is used to denote syscall entry/exit:
* IP = 0 -> entry, =1 -> exit
*/
- ip = regs->ARM_ip;
- regs->ARM_ip = why;
+ unsigned long ip = regs->ARM_ip;
- if (why)
- tracehook_report_syscall_exit(regs, 0);
- else if (tracehook_report_syscall_entry(regs))
- current_thread_info()->syscall = -1;
-
- regs->ARM_ip = ip;
+ current_thread_info()->syscall = scno;
+ if (why) { /* exit */
+ if (test_thread_flag(TIF_SYSCALL_TRACE)) {
+ regs->ARM_ip = why;
+ tracehook_report_syscall_exit(regs, 0);
+ regs->ARM_ip = ip;
+ }
+ audit_syscall_exit(regs);
+ } else { /* entry */
+ if (test_thread_flag(TIF_SYSCALL_TRACE)) {
+ regs->ARM_ip = why;
+ if (tracehook_report_syscall_entry(regs))
+ current_thread_info()->syscall = -1;
+ regs->ARM_ip = ip;
+ }
+ /* Call secure_computing after any userspace changes are done */
+ secure_computing_strict(current_thread_info()->syscall);
+ audit_syscall_entry(AUDIT_ARCH_ARM, scno, regs->ARM_r0,
+ regs->ARM_r1, regs->ARM_r2, regs->ARM_r3);
+ }
return current_thread_info()->syscall;
}
diff --git a/arch/microblaze/kernel/ptrace.c b/arch/microblaze/kernel/ptrace.c
index ab1b9db..9a917a7 100644
--- a/arch/microblaze/kernel/ptrace.c
+++ b/arch/microblaze/kernel/ptrace.c
@@ -136,8 +136,6 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
{
long ret = 0;
- secure_computing_strict(regs->r12);
-
if (test_thread_flag(TIF_SYSCALL_TRACE) &&
tracehook_report_syscall_entry(regs))
/*
@@ -147,6 +145,8 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
*/
ret = -1L;
+ secure_computing_strict(regs->r12);
+
audit_syscall_entry(EM_MICROBLAZE, regs->r12, regs->r5, regs->r6,
regs->r7, regs->r8);
diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
index 4812c6d..2b1f5f3 100644
--- a/arch/mips/kernel/ptrace.c
+++ b/arch/mips/kernel/ptrace.c
@@ -534,19 +534,16 @@ static inline int audit_arch(void)
*/
asmlinkage void syscall_trace_enter(struct pt_regs *regs)
{
- /* do the secure computing check first */
- secure_computing_strict(regs->regs[2]);
-
- if (!(current->ptrace & PT_PTRACED))
- goto out;
-
- if (!test_thread_flag(TIF_SYSCALL_TRACE))
- goto out;
-
+ if ((current->ptrace & PT_PTRACED) &&
+ test_thread_flag(TIF_SYSCALL_TRACE)) {
/* The 0x80 provides a way for the tracing parent to distinguish
between a syscall stop and SIGTRAP delivery */
ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD) ?
0x80 : 0));
+ }
+
+ /* do the secure computing check after the system calls are final. */
+ secure_computing_strict(regs->regs[2]);
/*
* this isn't the same as continuing with a signal, but it will do
@@ -558,7 +555,6 @@ asmlinkage void syscall_trace_enter(struct pt_regs *regs)
current->exit_code = 0;
}
-out:
audit_syscall_entry(audit_arch(), regs->regs[2],
regs->regs[4], regs->regs[5],
regs->regs[6], regs->regs[7]);
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index dd5e214..4b725ed 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -1710,8 +1710,6 @@ long do_syscall_trace_enter(struct pt_regs *regs)
{
long ret = 0;
- secure_computing_strict(regs->gpr[0]);
-
if (test_thread_flag(TIF_SYSCALL_TRACE) &&
tracehook_report_syscall_entry(regs))
/*
@@ -1721,6 +1719,9 @@ long do_syscall_trace_enter(struct pt_regs *regs)
*/
ret = -1L;
+ if (!ret)
+ secure_computing_strict(regs->gpr[0]);
+
if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
trace_sys_enter(regs, regs->gpr[0]);
diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
index 4993e68..b95c0ac 100644
--- a/arch/s390/kernel/ptrace.c
+++ b/arch/s390/kernel/ptrace.c
@@ -718,9 +718,6 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
{
long ret = 0;
- /* Do the secure computing check first. */
- secure_computing_strict(regs->gprs[2]);
-
/*
* The sysc_tracesys code in entry.S stored the system
* call number to gprs[2].
@@ -737,6 +734,9 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
ret = -1;
}
+ /* Do the secure computing check with a final syscall set. */
+ secure_computing_strict(regs->gprs[2]);
+
if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
trace_sys_enter(regs, regs->gprs[2]);
diff --git a/arch/sh/kernel/ptrace_32.c b/arch/sh/kernel/ptrace_32.c
index 81f999a..2500ce1 100644
--- a/arch/sh/kernel/ptrace_32.c
+++ b/arch/sh/kernel/ptrace_32.c
@@ -503,8 +503,6 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
{
long ret = 0;
- secure_computing_strict(regs->regs[0]);
-
if (test_thread_flag(TIF_SYSCALL_TRACE) &&
tracehook_report_syscall_entry(regs))
/*
@@ -514,6 +512,9 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
*/
ret = -1L;
+ if (!ret)
+ secure_computing_strict(regs->regs[0]);
+
if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
trace_sys_enter(regs, regs->regs[0]);
diff --git a/arch/sh/kernel/ptrace_64.c b/arch/sh/kernel/ptrace_64.c
index af90339..cc030e0 100644
--- a/arch/sh/kernel/ptrace_64.c
+++ b/arch/sh/kernel/ptrace_64.c
@@ -522,8 +522,6 @@ asmlinkage long long do_syscall_trace_enter(struct pt_regs *regs)
{
long long ret = 0;
- secure_computing_strict(regs->regs[9]);
-
if (test_thread_flag(TIF_SYSCALL_TRACE) &&
tracehook_report_syscall_entry(regs))
/*
@@ -533,6 +531,9 @@ asmlinkage long long do_syscall_trace_enter(struct pt_regs *regs)
*/
ret = -1LL;
+ if (!ret)
+ secure_computing_strict(regs->regs[9]);
+
if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
trace_sys_enter(regs, regs->regs[9]);
diff --git a/arch/sparc/kernel/ptrace_64.c b/arch/sparc/kernel/ptrace_64.c
index 484daba..31fff51 100644
--- a/arch/sparc/kernel/ptrace_64.c
+++ b/arch/sparc/kernel/ptrace_64.c
@@ -1061,12 +1061,13 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs)
{
int ret = 0;
- /* do the secure computing check first */
- secure_computing_strict(regs->u_regs[UREG_G1]);
-
if (test_thread_flag(TIF_SYSCALL_TRACE))
ret = tracehook_report_syscall_entry(regs);
+ /* do the secure computing check with the final syscall */
+ if (!ret)
+ secure_computing_strict(regs->u_regs[UREG_G1]);
+
if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
trace_sys_enter(regs, regs->u_regs[UREG_G1]);
--
1.7.9.5
next prev parent reply other threads:[~2012-05-24 16:09 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-21 18:21 seccomp and ptrace. what is the correct order? Eric Paris
2012-05-21 18:25 ` Roland McGrath
2012-05-21 19:20 ` Indan Zupancic
2012-05-22 16:23 ` Will Drewry
2012-05-22 16:26 ` Will Drewry
2012-05-22 17:39 ` Al Viro
2012-05-22 20:26 ` Will Drewry
2012-05-22 20:34 ` H. Peter Anvin
2012-05-22 20:48 ` Will Drewry
2012-05-22 21:07 ` Al Viro
2012-05-22 21:17 ` Roland McGrath
2012-05-22 21:18 ` H. Peter Anvin
2012-05-22 22:20 ` Al Viro
2012-05-22 21:09 ` H. Peter Anvin
2012-05-22 21:14 ` Will Drewry
2012-05-22 21:37 ` H. Peter Anvin
2012-05-24 16:07 ` [RFC PATCH 0/3] move the secure_computing call Will Drewry
2012-05-24 16:07 ` [RFC PATCH 1/3] seccomp: Don't allow tracers to abuse RET_TRACE Will Drewry
2012-05-24 17:54 ` Indan Zupancic
2012-05-24 18:24 ` Will Drewry
2012-05-24 20:17 ` Indan Zupancic
2012-05-24 16:08 ` [RFC PATCH 2/3] arch/x86: move secure_computing after ptrace Will Drewry
2012-05-24 16:08 ` Will Drewry [this message]
2012-05-24 16:13 ` [RFC PATCH 0/3] move the secure_computing call H. Peter Anvin
2012-05-24 18:07 ` Roland McGrath
2012-05-24 18:27 ` Indan Zupancic
2012-05-24 18:45 ` H. Peter Anvin
2012-05-24 19:39 ` Indan Zupancic
2012-05-24 22:00 ` Andrew Morton
2012-05-25 1:55 ` Will Drewry
2012-05-24 23:40 ` James Morris
2012-05-24 23:43 ` Andrew Lutomirski
2012-05-24 23:56 ` H. Peter Anvin
2012-05-25 0:26 ` Andrew Lutomirski
2012-05-25 0:38 ` H. Peter Anvin
2012-05-25 0:55 ` Andrew Lutomirski
2012-05-21 18:47 ` seccomp and ptrace. what is the correct order? richard -rw- weinberger
2012-05-21 19:13 ` H. Peter Anvin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1337875681-20717-4-git-send-email-wad@chromium.org \
--to=wad@chromium.org \
--cc=akpm@linux-foundation.org \
--cc=corbet@lwn.net \
--cc=coreyb@linux.vnet.ibm.com \
--cc=hpa@zytor.com \
--cc=indan@nul.nu \
--cc=jmorris@namei.org \
--cc=keescook@chromium.org \
--cc=kernel-hardening@lists.openwall.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=luto@mit.edu \
--cc=markus@chromium.org \
--cc=mcgrathr@google.com \
--cc=mingo@redhat.com \
--cc=netdev@parisplace.org \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=pmoore@redhat.com \
--cc=rdunlap@xenotime.net \
--cc=serge.hallyn@canonical.com \
--cc=tglx@linutronix.de \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®