Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753945AbcLBX1u (ORCPT ); Fri, 2 Dec 2016 18:27:50 -0500 Received: from mail-io0-f178.google.com ([209.85.223.178]:34552 "EHLO mail-io0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750903AbcLBX1s (ORCPT ); Fri, 2 Dec 2016 18:27:48 -0500 MIME-Version: 1.0 In-Reply-To: <20161201012401.17367-1-jistone@redhat.com> References: <20161201012401.17367-1-jistone@redhat.com> From: Kees Cook Date: Fri, 2 Dec 2016 15:27:46 -0800 X-Google-Sender-Auth: HnRjki9ypNH_G-qItvazbcFABp8 Message-ID: Subject: Re: [PATCH] Yama: allow access for the current ptrace parent To: James Morris Cc: LKML , Josh Stone , "Serge E. Hallyn" , linux-security-module Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2873 Lines: 79 On Wed, Nov 30, 2016 at 5:24 PM, Josh Stone wrote: > Under ptrace_scope=1, it's possible to have a tracee that is already > ptrace-attached, but is no longer a direct descendant. For instance, a > forking daemon will be re-parented to init, losing its ancestry to the > tracer that launched it. > > The tracer can continue using ptrace in that state, but it will be > denied other accesses that check PTRACE_MODE_ATTACH, like process_vm_rw > and various procfs files. There's no reason to prevent such access for > a tracer that already has ptrace control anyway. > > This patch adds a case to ptracer_exception_found to allow access for > any task in the same thread group as the current ptrace parent. Nice catch, thanks! > Signed-off-by: Josh Stone > Cc: Kees Cook > Cc: James Morris > Cc: "Serge E. Hallyn" > Cc: linux-security-module@vger.kernel.org > --- > security/yama/yama_lsm.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c > index 0309f2111c70..da67a6e07a60 100644 > --- a/security/yama/yama_lsm.c > +++ b/security/yama/yama_lsm.c > @@ -309,7 +309,7 @@ static int task_is_descendant(struct task_struct *parent, > * @tracer: the task_struct of the process attempting ptrace > * @tracee: the task_struct of the process to be ptraced > * > - * Returns 1 if tracer has is ptracer exception ancestor for tracee. > + * Returns 1 if tracer has a ptracer exception ancestor for tracee. > */ > static int ptracer_exception_found(struct task_struct *tracer, > struct task_struct *tracee) > @@ -320,6 +320,17 @@ static int ptracer_exception_found(struct task_struct *tracer, > bool found = false; > > rcu_read_lock(); > + > + /* If there's already an active tracing relationship, then make an I'll adjust the comment style here and add it to my tree for -next. > + * exception for the sake of other accesses, like process_vm_rw. > + */ > + parent = ptrace_parent(tracee); > + if (parent != NULL && same_thread_group(parent, tracer)) { > + rc = 1; > + goto unlock; > + } > + > + /* Look for a PR_SET_PTRACER relationship. */ > if (!thread_group_leader(tracee)) > tracee = rcu_dereference(tracee->group_leader); > list_for_each_entry_rcu(relation, &ptracer_relations, node) { > @@ -334,6 +345,8 @@ static int ptracer_exception_found(struct task_struct *tracer, > > if (found && (parent == NULL || task_is_descendant(parent, tracer))) > rc = 1; > + > +unlock: > rcu_read_unlock(); > > return rc; > -- > 2.9.3 > Thanks! -Kees -- Kees Cook Nexus Security