Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965828Ab2B1Qvr (ORCPT ); Tue, 28 Feb 2012 11:51:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:28384 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964782Ab2B1Qvn (ORCPT ); Tue, 28 Feb 2012 11:51:43 -0500 Date: Tue, 28 Feb 2012 17:43:35 +0100 From: Oleg Nesterov To: Will Drewry Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-doc@vger.kernel.org, kernel-hardening@lists.openwall.com, netdev@vger.kernel.org, x86@kernel.org, arnd@arndb.de, davem@davemloft.net, hpa@zytor.com, mingo@redhat.com, peterz@infradead.org, rdunlap@xenotime.net, mcgrathr@chromium.org, tglx@linutronix.de, luto@mit.edu, eparis@redhat.com, serge.hallyn@canonical.com, djm@mindrot.org, scarybeasts@gmail.com, indan@nul.nu, pmoore@redhat.com, akpm@linux-foundation.org, corbet@lwn.net, eric.dumazet@gmail.com, markus@chromium.org, coreyb@linux.vnet.ibm.com, keescook@chromium.org, Denys Vlasenko Subject: Re: [PATCH v11 10/12] ptrace,seccomp: Add PTRACE_SECCOMP support Message-ID: <20120228164335.GC3664@redhat.com> References: <1330140111-17201-1-git-send-email-wad@chromium.org> <1330140111-17201-10-git-send-email-wad@chromium.org> <20120227175407.GD10608@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3601 Lines: 85 On 02/27, Will Drewry wrote: > On Mon, Feb 27, 2012 at 11:54 AM, Oleg Nesterov wrote: > > On 02/24, Will Drewry wrote: > >> > >> ?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 > >> --- 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/