> Not sure I understand... This is behaviour change, no?
No, I'm not talking about any change in the user-observable semantics.
> Suppose a tracee is running, but for some reason it is somewhere
> inside start_callback(). utrace_control(UTRACE_STOP) can change ->flags
> and provoke report->action = UTRACE_STOP. In this case the tracee will
> stop.
And then you check whether it's really in a proper ptrace stop, see that it
isn't, and use UTRACE_RESUME.
> IOW. The tracee is running, say, it does "for (;;) ;". If the tracer
> does
> for (;;)
> ptrace(PTRACE_CONT);
>
> it can stop the tracee.
No.
> As I said, I think this is minor. But I never know which behaviour
> changes are important.
That one would be quite wrong, but it's not what I said.
> I don't think we have security issues, ptrace_check_attach() is only
> called when we already ptrace the task.
It is called to verify that we already ptrace the task.
That's the point.
> It is a bit racy, we should
> check child->parent after ptrace_lookup_engine(), but this is trivial.
As I said, as long as we verify that we are the attached ptracer before
calling utrace_control, there is no security issue.
> (and in fact this race doesn't exist because of lock_kernl(), but
> sys_ptrace()->lock_kernel() must die).
Yes, pretend it weren't there.
> We already have the indicator - get_stop_event(). Except it doesn't work
> for jctl stops. ptrace_check_attach() needs something like:
>
> if (!ptrace_event_pending()) {
> do not change engine->flags,
> do not set utrace->report,
> just s/TASK_STOPPED/TASK_TRACED/ or fail
> }
utrace_control(,,UTRACE_STOP) does s/TASK_STOPPED/TASK_TRACED/. That's
what I was just saying. If it returns -EINPROGRESS, then it wasn't really
in TASK_STOPPED, so you can do UTRACE_RESUME and then fail with -ESRCH.
(In that race, you could get a report_quiesce or UTRACE_REPORT_SIGNAL,
and those just need to know to do nothing if (!ptrace_event_pending()).)
That alone succeeds when it shouldn't, if there is another engine keeping
it in TASK_TRACED already so that utrace_control returns zero. To rule
that out we might use report_jctl to do some more bookkeeping. But that
doesn't cover it being in TASK_TRACED already when you PTRACE_ATTACH. I
wonder if we could make ->state = TASK_STOPPED|TASK_TRACED work to keep
task_is_stopped() as a meaningful test while in UTRACE_STOP.
Thanks,
Roland
