First of all, I believe ptrace_check_attach() is buggy. It lacks
"child->parent == current" or "engine->data == current" check.
The bug is serious, sys_ptrace() does not do security checks unless
request == PTRACE_ATTACH. But this is trivial to fix.
Also. Without utrace, ptrace_check_attach() requires the child
is STOPPED or TRACED. With utrace, ptrace_check_attach() does
utrace_control(UTRACE_STOP) first. If the child is not stopped
we return -ESRCH correctly, but utrace_control() sets ENGINE_STOP
and utrace->report, and this can actually stop the child.
Yes, if ptracer does utrace_control(UTRACE_STOP), this doesn't
necessarily mean the child will stop. But, for example, suppose
that ptracer does sys_ptrace(WHATEVER) in a loop. In that case
it is possible that utrace_control(UTRACE_STOP) races with the
child doing utrace_resume()->start_callback(), and we can set
ENGINE_STOP right before start_callback() re-reads engine->flags
under "if (want & UTRACE_EVENT(QUIESCE))".
This is minor, but not good anyway. Unless I missed something.
And. From another thread,
utrace_prepare_examine verifies that the target is blocked already,
...
Note it does not in particular
require the target be stopped, though does require the caller's engine
used UTRACE_STOP.
Yes, but I am a bit worried about ptrace_check_attach() case...
For example, utrace_control(UTRACE_STOP)->utrace_do_stop() finds
the child inside utrace_report_jctl() path in TASK_STOPPED state,
sets utrace->stopped and returns true. After that utrace_report_jctl()
clears ->stopped and another engine's ->report_jctl does mutex_lock()
and sets ->state == TASK_INTERRUPTIBLE.
This means that ptrace_check_attach() can succeed while the child
can be waked up soon and do a lot more before it actually stops.
I don't see really bad implications though, just worried.
Oleg.
