[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Q: utrace && ptrace_check_attach()

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

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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]