[lkml]   [2012]   [Jan]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: ptrace fixes for 3.2
    Hi Tejun,

    On 01/03, Tejun Heo wrote:
    > On Tue, Jan 03, 2012 at 04:44:04PM +0100, Oleg Nesterov wrote:
    > > It fails because ->real_parent sees its child in EXIT_DEAD state
    > > while the tracer is going to change the state back to EXIT_ZOMBIE
    > > in wait_task_zombie().
    > Argh.... EXIT_ZOMBIE -> DEAD -> ZOMBIE dancing in wait_task_zombie()
    > is just nasty. Didn't realize it was doing that. :(

    We both missed this ;)

    > > The offending commit is 823b018e which moved the EXIT_DEAD check,
    > > but in fact we should not blame it. The original code was not
    > > correct as well because it didn't take ptrace_reparented() into
    > > account and because we can't really trust ->ptrace.
    > >
    > > This patch adds the additional check to close this particular
    > > race but it doesn't solve the whole problem. We simply can't
    > > rely on ->ptrace in this case, it can be cleared if the tracer
    > > is multithreaded by the exiting ->parent.
    > I'm not following this part. Can you please explain it in a bit more
    > detail?

    Before 823b018e the code was:

    if (!ptrace && p->ptrace) {
    wo->notask_error = 0;
    return 0;

    if (p->exit_state == EXIT_DEAD)
    return 0;

    There are 2 problems:

    1. it is not correct to clear ->notask_error unless
    this child is ptrace_reparented(). Nobody will
    wakeup us if EXIT_DEAD was set by our sub-thread.

    2. We can not rely on ->ptrace to detect this case.

    Suppose that the tracer is multithreaded, it has
    two threads T1 and T2, T1 traces our child.

    - T2 does do_wait(WEXITED), sets EXIT_DEAD, drops

    - T1 exits and does __ptrace_detach(), this means
    __ptrace_unlink() and nothing more.

    - Now, real_parent does do_wait() and sees the
    EXIT_DEAD child but ->ptrace = 0.

    - finally T2 sets EXIT_DEAD but it is too late,

    The patch doesn't solve the 2nd (btw very old) problem. Fortunately
    this race is very unlikely.

    > > I think we should kill EXIT_DEAD altogether, we should always
    > > remove the soon-to-be-reaped child from ->children or at least
    > > we should never do the DEAD->ZOMBIE transition. But this is too
    > > complex for 3.2.
    > Agreed. Removing the reverse transition shouldn't be too difficult
    > and can be done without affecting fast non-ptrace path. ie. if the
    > child is ptraced, drop readlock, grab writelock, recheck, buffer
    > states to copy out to userland, detach and transit to DEAD if
    > necessary.


    > > Also, I think wait_consider_task() needs more fixes. I do not
    > > think we should clear ->notask_error without WEXITED in this
    > > case, but this is what we do in the EXIT_ZOMBIE case.
    > Hmmm... I'm not sure about that. Why do you think so?

    I am not sure too. But why do_wait() should sleep if it is called
    without WEXITED (lets ignore WCONTINUED) and the child is ZOMBIE?
    I think it should return -ECHILD, like it does if the child is not

    IOW. Suppose we have a single EXIT_ZOMBIE child. If it is not traced,
    do_wait(WSTOPPED) returns -ECHILD. If the child is traced (by another
    process) do_wait() sleeps until detach just to return the same error.
    This looks a bit strange.

    > Acked-by: Tejun Heo <>

    Great, thanks.


     \ /
      Last update: 2012-01-03 18:17    [W:0.036 / U:0.600 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site