* [PATCH] Yama: allow access for the current ptrace parent
@ 2016-12-02 23:49 Kees Cook
2016-12-05 0:13 ` James Morris
0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2016-12-02 23:49 UTC (permalink / raw)
To: James Morris; +Cc: Serge E. Hallyn, linux-security-module, linux-kernel
From: Josh Stone <jistone@redhat.com>
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.
Signed-off-by: Josh Stone <jistone@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: James Morris <james.l.morris@oracle.com>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
James, can you pull this into your -next tree? I made a tiny fix to the
comment style, but it is otherwise identical to what Josh sent originally.
---
security/yama/yama_lsm.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index 0309f2111c70..968e5e0a3f81 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,18 @@ 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
+ * 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 +346,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.7.4
--
Kees Cook
Nexus Security
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Yama: allow access for the current ptrace parent
2016-12-02 23:49 [PATCH] Yama: allow access for the current ptrace parent Kees Cook
@ 2016-12-05 0:13 ` James Morris
0 siblings, 0 replies; 6+ messages in thread
From: James Morris @ 2016-12-05 0:13 UTC (permalink / raw)
To: Kees Cook
Cc: James Morris, Serge E. Hallyn, linux-security-module, linux-kernel
On Fri, 2 Dec 2016, Kees Cook wrote:
> James, can you pull this into your -next tree? I made a tiny fix to the
> comment style, but it is otherwise identical to what Josh sent originally.
Applied, thanks!
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Yama: allow access for the current ptrace parent
2016-12-05 19:13 ` Josh Stone
@ 2016-12-05 19:38 ` Kees Cook
0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2016-12-05 19:38 UTC (permalink / raw)
To: Josh Stone; +Cc: James Morris, LKML, Serge E. Hallyn, linux-security-module
On Mon, Dec 5, 2016 at 11:13 AM, Josh Stone <jistone@redhat.com> wrote:
> On 12/02/2016 03:27 PM, Kees Cook wrote:
>>> + /* 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.
>
> Thanks!
>
> I guess the tweak is that it should have an empty "/*" line?
>
> FWIW, checkpatch.pl doesn't warn about this -- perhaps it should?
> I only see the opposite check for NETWORKING_BLOCK_COMMENT_STYLE.
Hrm, I thought it did warn. But yeah, the networking subsystem uses
this style but everywhere else doesn't. :P Wheee. :)
-Kees
--
Kees Cook
Nexus Security
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Yama: allow access for the current ptrace parent
2016-12-02 23:27 ` Kees Cook
@ 2016-12-05 19:13 ` Josh Stone
2016-12-05 19:38 ` Kees Cook
0 siblings, 1 reply; 6+ messages in thread
From: Josh Stone @ 2016-12-05 19:13 UTC (permalink / raw)
To: Kees Cook, James Morris; +Cc: LKML, Serge E. Hallyn, linux-security-module
On 12/02/2016 03:27 PM, Kees Cook wrote:
>> + /* 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.
Thanks!
I guess the tweak is that it should have an empty "/*" line?
FWIW, checkpatch.pl doesn't warn about this -- perhaps it should?
I only see the opposite check for NETWORKING_BLOCK_COMMENT_STYLE.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Yama: allow access for the current ptrace parent
2016-12-01 1:24 Josh Stone
@ 2016-12-02 23:27 ` Kees Cook
2016-12-05 19:13 ` Josh Stone
0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2016-12-02 23:27 UTC (permalink / raw)
To: James Morris; +Cc: LKML, Josh Stone, Serge E. Hallyn, linux-security-module
On Wed, Nov 30, 2016 at 5:24 PM, Josh Stone <jistone@redhat.com> 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 <jistone@redhat.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: James Morris <james.l.morris@oracle.com>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Yama: allow access for the current ptrace parent
@ 2016-12-01 1:24 Josh Stone
2016-12-02 23:27 ` Kees Cook
0 siblings, 1 reply; 6+ messages in thread
From: Josh Stone @ 2016-12-01 1:24 UTC (permalink / raw)
To: linux-kernel
Cc: Josh Stone, Kees Cook, James Morris, Serge E. Hallyn,
linux-security-module
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.
Signed-off-by: Josh Stone <jistone@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: James Morris <james.l.morris@oracle.com>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
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
+ * 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-05 19:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-02 23:49 [PATCH] Yama: allow access for the current ptrace parent Kees Cook
2016-12-05 0:13 ` James Morris
-- strict thread matches above, loose matches on Subject: below --
2016-12-01 1:24 Josh Stone
2016-12-02 23:27 ` Kees Cook
2016-12-05 19:13 ` Josh Stone
2016-12-05 19:38 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).