From: Roland McGrath <roland@redhat.com>
To: James Morris <jmorris@namei.org>
Cc: Chris Wright <chrisw@sous-sol.org>,
Oleg Nesterov <oleg@redhat.com>,
Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org,
Eric Paris <eparis@redhat.com>,
Stephen Smalley <sds@tycho.nsa.gov>,
David Howells <dhowells@redhat.com>
Subject: Re: [PATCH 3/3] ptrace: do not use task_lock() for attach
Date: Wed, 6 May 2009 18:17:16 -0700 (PDT) [thread overview]
Message-ID: <20090507011716.BA522FC39E@magilla.sf.frob.com> (raw)
In-Reply-To: James Morris's message of Thursday, 7 May 2009 09:48:29 +1000 <alpine.LRH.2.00.0905070946480.22586@tundra.namei.org>
> As far as I can tell, yes.
>
> (Added David Howells and security folk to the cc -- please make sure at
> least that the LSM list is cc'd when changing code which affects LSM
> modules).
Good catch, Chris and Oleg! This one is yet another dhowells blue plate
special, deeply subtle change buried inside the ginormous commit d84f4f9. ;-}
He even mentioned this one in the log:
(a) selinux_setprocattr() no longer does its check for whether the
current ptracer can access processes with the new SID inside the lock
that covers getting the ptracer's SID. Whilst this lock ensures that
the check is done with the ptracer pinned, the result is only valid
until the lock is released, so there's no point doing it inside the
lock.
Before d84f4f9, the extraction, avc check, and SID switch were all under
task_lock(). What David's comment ignores is that "the lock that covers
getting the ptracer's SID" (i.e. task_lock) is also the lock that excludes
ptrace attempts, with their security checks against the (old or new) SID.
i.e.:
if (!error)
tsec->sid = sid;
task_unlock(p);
That ensured that ptrace_attach/ptrace_traceme would be seen to atomically
check the bits that affect the SELinux ptrace controls and change the bits
that affect "if (tracer)".
Indeed, cred_exec_mutex is the equivalent lock for that post-d84f4f9.
Thanks,
Roland
next prev parent reply other threads:[~2009-05-07 1:44 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-05 22:47 [PATCH 3/3] ptrace: do not use task_lock() for attach Oleg Nesterov
2009-05-06 2:08 ` Roland McGrath
2009-05-06 8:00 ` [RFC PATCH 3/3a] ptrace: add _ptrace_may_access() Ingo Molnar
2009-05-06 20:32 ` Roland McGrath
2009-05-06 20:47 ` Christoph Hellwig
2009-05-06 21:09 ` Roland McGrath
2009-05-07 8:19 ` Ingo Molnar
2009-05-07 8:17 ` Ingo Molnar
2009-05-06 23:53 ` Oleg Nesterov
2009-05-07 0:21 ` Roland McGrath
2009-05-07 6:36 ` Oleg Nesterov
2009-05-07 8:20 ` Ingo Molnar
2009-05-07 8:31 ` Oleg Nesterov
2009-05-07 8:38 ` Ingo Molnar
2009-05-07 8:49 ` [patch] security: rename ptrace_may_access => ptrace_access_check Ingo Molnar
2009-05-07 9:19 ` Oleg Nesterov
2009-05-07 9:27 ` Ingo Molnar
2009-05-07 8:57 ` [RFC PATCH 3/3a] ptrace: add _ptrace_may_access() Chris Wright
2009-05-07 9:04 ` Ingo Molnar
2009-05-07 9:20 ` Chris Wright
2009-05-07 9:54 ` James Morris
2009-05-07 10:20 ` your mail Ingo Molnar
2009-05-07 11:37 ` security: rename ptrace_may_access => ptrace_access_check James Morris
2009-05-07 14:17 ` Ingo Molnar
2009-06-23 14:14 ` Oleg Nesterov
2009-06-23 17:49 ` Christoph Hellwig
2009-06-23 19:24 ` [PATCH 0/1] mm_for_maps: simplify, use ptrace_may_access() Oleg Nesterov
2009-06-23 19:25 ` [PATCH 1/1] " Oleg Nesterov
2009-06-24 3:06 ` Serge E. Hallyn
2009-06-24 14:21 ` James Morris
2009-06-24 9:25 ` Roland McGrath
2009-06-24 14:37 ` Oleg Nesterov
2009-06-24 1:08 ` security: rename ptrace_may_access => ptrace_access_check James Morris
2009-05-08 3:27 ` your mail Casey Schaufler
2009-06-24 14:19 ` security: rename ptrace_may_access => ptrace_access_check James Morris
2009-05-07 9:31 ` [RFC PATCH 3/3a] ptrace: add _ptrace_may_access() Ingo Molnar
2009-05-07 9:49 ` [patch 1/2] ptrace, security: rename ptrace_may_access => ptrace_access_check Ingo Molnar
2009-05-07 18:47 ` Roland McGrath
2009-05-07 19:55 ` Andrew Morton
2009-05-11 13:39 ` Ingo Molnar
2009-05-11 18:51 ` Andrew Morton
2009-05-15 1:10 ` Américo Wang
2009-05-15 19:34 ` Ingo Molnar
2009-05-07 9:50 ` [patch 2/2] ptrace: turn ptrace_access_check() into a retval function Ingo Molnar
2009-05-07 18:47 ` Roland McGrath
2009-05-06 22:46 ` [PATCH 3/3] ptrace: do not use task_lock() for attach Chris Wright
2009-05-06 23:13 ` Oleg Nesterov
2009-05-06 23:27 ` Chris Wright
2009-05-06 23:48 ` James Morris
2009-05-07 1:17 ` Roland McGrath [this message]
2009-05-08 12:18 ` David Howells
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=20090507011716.BA522FC39E@magilla.sf.frob.com \
--to=roland@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=chrisw@sous-sol.org \
--cc=dhowells@redhat.com \
--cc=eparis@redhat.com \
--cc=jmorris@namei.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=oleg@redhat.com \
--cc=sds@tycho.nsa.gov \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.