| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 26 May 2009 00:39:32 +0200
From: Oleg Nesterov <oleg@...hat.com>
To: Roland McGrath <roland@...hat.com>
Cc: Christoph Hellwig <hch@...radead.org>, Ingo Molnar <mingo@...e.hu>,
linux-kernel@...r.kernel.org
Subject: [RFC PATCH 8/X] ptrace: introduce ptrace_tracer() helper
On 05/25, Oleg Nesterov wrote:
>
> On 05/25, Oleg Nesterov wrote:
> >
> > Move task_struct->parent into ptrace_task->pt_tracer and change the users
> > accordingly.
> >
> > ...
> >
> > @@ -1773,8 +1779,8 @@ static int ptrace_signal(int signr, sigi
> > info->si_signo = signr;
> > info->si_errno = 0;
> > info->si_code = SI_USER;
> > - info->si_pid = task_pid_vnr(current->parent);
> > - info->si_uid = task_uid(current->parent);
> > + info->si_pid = task_pid_vnr(current->ptrace_task->pt_tracer);
> > + info->si_uid = task_uid(current->ptrace_task->pt_tracer);
>
> This change is wrong, will re-do. The task can be already untraced
> after ptrace_stop().
>
> But is the current code correct? If we are not traced any longer
> si_pid/si_uid are not necessary right either, we should calculate them
> before ptrace_stop(), no?
... and in theory we need rcu_read_lock() here.
I'd like to send the next patch for review, because I have questions.
It fixes the bug above, but of course the previous patch should be
fixed instead. I'll re-send the whole series (with renames), but first
I'd like to know what Roland thinks.
---------------------------------------------------------------------
[RFC PATCH 8/X] ptrace: introduce ptrace_tracer() helper
Introduce ptrace_tracer() (or suggest a better name) to simplify/cleanup
the code which needs the tracer and checks task_ptrace(). From now nobody
else uses ->pt_tracer except ptrace_link/ptrace_unlink.
Question. Note that ptrace_tracer() is equal to tracehook_tracer_task().
But I do not understand the future plans for tracehook_tracer_task().
Should we just use tracehook_tracer_task() ? If yes, how
ptrace_reparented() can use this helper?
include/linux/ptrace.h | 17 ++++++++++++-----
kernel/ptrace.c | 2 +-
kernel/signal.c | 28 +++++++++++++++++-----------
3 files changed, 30 insertions(+), 17 deletions(-)
--- PTRACE/include/linux/ptrace.h~8_PT_TRACER 2009-05-25 22:52:51.000000000 +0200
+++ PTRACE/include/linux/ptrace.h 2009-05-25 23:21:17.000000000 +0200
@@ -116,10 +116,17 @@ static inline int task_ptrace(struct tas
task->ptrace_task->pt_flags : 0;
}
-static inline int ptrace_reparented(struct task_struct *child)
+static inline struct task_struct *ptrace_tracer(struct task_struct *task)
{
- return unlikely(task_ptrace(child)) &&
- child->ptrace_task->pt_tracer != child->real_parent;
+ if (task->ptrace_task)
+ return task->ptrace_task->pt_tracer;
+ return NULL;
+}
+
+static inline bool ptrace_reparented(struct task_struct *child)
+{
+ struct task_struct *tracer = ptrace_tracer(child);
+ return unlikely(tracer) && tracer != child->real_parent;
}
static inline void ptrace_unlink(struct task_struct *child)
@@ -167,9 +174,9 @@ static inline void ptrace_init_task(stru
INIT_LIST_HEAD(&child->ptrace_entry);
INIT_LIST_HEAD(&child->ptraced);
- if (unlikely(child->ptrace_task) && task_ptrace(current))
+ if (unlikely(child->ptrace_task) && ptrace_tracer(current))
ptrace_link(child, task_ptrace(current),
- current->ptrace_task->pt_tracer);
+ ptrace_tracer(current));
}
/**
--- PTRACE/kernel/ptrace.c~8_PT_TRACER 2009-05-25 22:52:51.000000000 +0200
+++ PTRACE/kernel/ptrace.c 2009-05-25 23:24:57.000000000 +0200
@@ -108,7 +108,7 @@ int ptrace_check_attach(struct task_stru
* be changed by us so it's not changing right after this.
*/
read_lock(&tasklist_lock);
- if (task_ptrace(child) && child->ptrace_task->pt_tracer == current) {
+ if (ptrace_tracer(child) == current) {
ret = 0;
/*
* child->sighand can't be NULL, release_task()
--- PTRACE/kernel/signal.c~8_PT_TRACER 2009-05-25 22:52:51.000000000 +0200
+++ PTRACE/kernel/signal.c 2009-05-26 00:14:00.000000000 +0200
@@ -1411,9 +1411,8 @@ int do_notify_parent(struct task_struct
BUG_ON(!task_ptrace(tsk) &&
(tsk->group_leader != tsk || !thread_group_empty(tsk)));
- if (task_ptrace(tsk))
- parent = tsk->ptrace_task->pt_tracer;
- else
+ parent = ptrace_tracer(tsk);
+ if (likely(!parent))
parent = tsk->real_parent;
info.si_signo = sig;
@@ -1489,9 +1488,8 @@ static void do_notify_parent_cldstop(str
struct task_struct *parent;
struct sighand_struct *sighand;
- if (task_ptrace(tsk))
- parent = tsk->ptrace_task->pt_tracer;
- else {
+ parent = ptrace_tracer(tsk);
+ if (likely(!parent)) {
tsk = tsk->group_leader;
parent = tsk->real_parent;
}
@@ -1538,7 +1536,9 @@ static void do_notify_parent_cldstop(str
static inline int may_ptrace_stop(void)
{
- if (!likely(task_ptrace(current)))
+ struct task_struct *tracer = ptrace_tracer(current);
+
+ if (!likely(tracer))
return 0;
/*
* Are we in the middle of do_coredump?
@@ -1550,7 +1550,7 @@ static inline int may_ptrace_stop(void)
* is safe to enter schedule().
*/
if (unlikely(current->mm->core_state) &&
- unlikely(current->mm == current->ptrace_task->pt_tracer->mm))
+ unlikely(current->mm == tracer->mm))
return 0;
return 1;
@@ -1756,8 +1756,14 @@ static int do_signal_stop(int signr)
static int ptrace_signal(int signr, siginfo_t *info,
struct pt_regs *regs, void *cookie)
{
- if (!task_ptrace(current))
+ struct task_struct *tracer = ptrace_tracer(current);
+ pid_t pid;
+ uid_t uid;
+
+ if (!tracer)
return signr;
+ pid = task_pid_vnr(tracer);
+ uid = task_uid(tracer);
ptrace_signal_deliver(regs, cookie);
@@ -1779,8 +1785,8 @@ static int ptrace_signal(int signr, sigi
info->si_signo = signr;
info->si_errno = 0;
info->si_code = SI_USER;
- info->si_pid = task_pid_vnr(current->ptrace_task->pt_tracer);
- info->si_uid = task_uid(current->ptrace_task->pt_tracer);
+ info->si_pid = pid;
+ info->si_uid = uid;
}
/* If the (new) signal is now blocked, requeue it. */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists