On 04/20, Roland McGrath wrote:
>
> > Afaics ptrace_attach() needs this lock only to pin ->mm, no other other
> > reasons. ptrace_traceme() doesn't need it at all.
>
> I'm pretty sure that ->mm check is only meant to exclude kernel threads.
> It should check PF_KTHREAD now,
Yes.
But __ptrace_may_access()->get_dumpable(task->mm) is not safe without
task_lock(). This is easy.
> > The comment above tracehook_unsafe_exec() says "Called with task_lock()",
> > this is wrong, check_unsafe_exec() doesn't take task_lock().
>
> This changed upstream without keeping tracehook.h up to date.
> Sigh. Please send a tracehook.h comment fix patch upstream.
OK.
> > Perhaps it makes sense to do a little cleanup first, introduce
>
> The clean-up should get rid of PT_DTRACE entirely.
Agreed. But this needs another patch...
Roland, I have to apologize for delay again. The first patch (move ->ptrace
in struct ->ptrace_task) is not ready. Will do my best to send tomorrow.
I was distracted by other problems, and then I found that this (trivial) patch
becomes really huge. There are 116 places which use task->ptrace directly,
most in arch/.
So I am going to add
static inline int ptrace_set_flag(struct task_struct *task, unsigned
flag)
{
task->ptrace |= flag;
}
static inline void ptrace_clear_flag(struct task_struct *task, unsigned
flag)
{
task->ptrace &= ~flag;
}
and convert the code to use task_ptrace(), ptrace_set_flag(),
ptrace_clear_flag().
This change can be sent upstream right now. In that case the actual change will
be
very small.
Do you agree with these helpers? Perhaps even ptrace_test_flag() makes sense...
Oleg.
