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.