> Except... personally I dislike ->flags, it is not greppable.
Sure. I don't care what the field names are. I just use short names in
examples off the cuff when it keeps the example code lines from getting too
long.
> Yes, except release_task() doesn't work. I think it should be freed
> by __put_task_struct(). Proc can read ->ptrace_child and race with
> release_task().
Right.
> Now the questions. First of all, what does task_lock() currently mean
> from the ptrace pov ?
It was originally used to synchronize ->ptrace changes. Except where it
wasn't. Then the tasklist_lock+task_lock dance was added to synchronize
with exit.c code without losing whatever task_lock was presumed to be
doing. I think you are right that write_lock on tasklist_lock is the only
meaningful lock for ->ptrace at this point.
> 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, and reparent_to_kthreadd() already handles
the case of ptrace_attach() getting in before daemonize() runs.
> 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.
I believe cred_exec_mutex now serves that function.
> The only place which believes task_lock() can help is sys_execve() on
> some arches, the code clears PT_DTRACE under task_lock(). Why, and what
> PT_DTRACE actually means?
It is obsolete. Most uses were long ago replaced with per-arch things like
TIF_SINGLESTEP. This flag ought to die entirely. The common occurrences of:
current->ptrace &= ~PT_DTRACE;
in arch execve code is ancient boilerplate that does nothing useful.
AFAICT the only place PT_DTRACE is still used meaningfully at all
is in UML.
> arch/m68k/kernel/traps.c:trap_c() does current->ptrace |= PT_DTRACE.
I see no m68k code that checks the flag.
> Perhaps it makes sense to do a little cleanup first, introduce
The clean-up should get rid of PT_DTRACE entirely.
> The last question. ptrace_attach() does
>
> task->ptrace |= PT_PTRACED;
>
> Why can't we use the plain "=" instead of "|=" ? This looks as if ->ptrace
> can be nonzero even if the task is not traced. But I assume this is not
> possible?
I don't think it is possible. The use of |= here matches this check:
/* the same process cannot be attached many times */
if (task->ptrace & PT_PTRACED)
goto bad;
If you make it =, make the check if (task->ptrace) to match.
> And any code which does "if (p->ptrace & PT_TRACED)" could just
> do "if (p->ptrace)", right?
I believe so. It might have been otherwise in the distant past,
perhaps when PT_DTRACE was meaningful.
Thanks,
Roland
