Thread (53 messages) 53 messages, 9 authors, 2012-02-29

Re: [PATCH v11 10/12] ptrace,seccomp: Add PTRACE_SECCOMP support

From: Oleg Nesterov <oleg@redhat.com>
Date: 2012-02-28 16:51:47
Also in: linux-arch, lkml

On 02/27, Will Drewry wrote:
On Mon, Feb 27, 2012 at 11:54 AM, Oleg Nesterov [off-list ref] wrote:
quoted
On 02/24, Will Drewry wrote:
quoted
 arch/Kconfig              |    1 +
 include/linux/ptrace.h    |    7 +++++--
 include/linux/seccomp.h   |    4 +++-
 include/linux/tracehook.h |    6 ++++++
 kernel/ptrace.c           |    4 ++++
 kernel/seccomp.c          |   18 ++++++++++++++++++
FYI, this conflicts with the changes -mm tree.

The changes in ptrace.* confict with Denys's
"ptrace: simplify PTRACE_foo constants and PTRACE_SETOPTIONS code"

The change in tracehook.h conflicts with
"ptrace: the killed tracee should not enter the syscall"
What's the best way to reconcile this in this day and age?
Of course I'd prefer if you make this change on top of Denys's patch ;)

Besides, if you agree with PTRACE_EVENT_SECCOMP/PTRACE_O_SECCOMP you
need only one trivial change in ptrace.h.
I don't see
these in kernel-next yet and I can't tell if there is a public -mm
anywhere anymore.
Strange... I didn't check, but every patch in
http://marc.info/?l=linux-mm-commits has this note:

	The -mm tree is included into linux-next and is updated
	there every 3-4 working days
quoted
quoted
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -354,6 +354,24 @@ int __secure_computing_int(int this_syscall)
                      seccomp_send_sigsys(this_syscall, reason_code);
                      return -1;
              }
+             case SECCOMP_RET_TRACE: {
+                     int ret;
+                     struct pt_regs *regs = task_pt_regs(current);
+                     if (!(test_tsk_thread_flag(current, TIF_SYSCALL_TRACE)) ||
+                         !(current->ptrace & PT_TRACE_SECCOMP))
+                             return -1;
+                     /*
+                      * PT_TRACE_SECCOMP and seccomp.trace indicate whether
+                      * tracehook_report_syscall_entry needs to signal the
+                      * tracer.  This avoids race conditions in hand off and
+                      * the requirement for TIF_SYSCALL_TRACE ensures that
+                      * we are in the syscall slow path.
+                      */
+                     current->seccomp.trace = 1;
+                     ret = tracehook_report_syscall_entry(regs);
+                     current->seccomp.trace = 0;
+                     return ret;
To be honest, this interface looks a bit strange to me...

Once again, sorry if this was already discussed. But perhaps it would
be better to introduce PTRACE_EVENT_SECCOMP/PTRACE_O_SECCOMP instead?

SECCOMP_RET_TRACE: could simply do ptrace_event(PTRACE_EVENT_SECCOMP)
unconditionaly. The tracer can set the option and do PTRACE_CONT if it
doesn't want the system call notifications.
Works for me - this also gets rid of the extra int for brief state
tracking. I'll switch over to that in the next rev.
Great. In this case this patch becomes really trivial. Just 2 defines
in ptrace.h and the unconditional ptrace_event() under SECCOMP_RET_TRACE.

But probably you should check fatal_signal_pending(current) after
ptrace_event() returns, ptrace_event() returns void.

Oleg.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help