lkml.org 
[lkml]   [2022]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 05/16] ptrace: Remove dead code from __ptrace_detach
    Sorry for delay.

    On 05/18, Eric W. Biederman wrote:
    >
    > Ever since commit 28d838cc4dfe ("Fix ptrace self-attach rule") it has
    > been impossible to attach another thread in the same thread group.
    >
    > Remove the code from __ptrace_detach that was trying to support
    > detaching from a thread in the same thread group.

    may be I am totally confused, but I think you misunderstood this code
    and thus this patch is very wrong.

    The same_thread_group() check does NOT try to check if debugger and
    tracee is in the same thread group, this is indeed impossible.

    We need this check to know if the tracee was ptrace_reparented() before
    __ptrace_unlink() or not.


    > -static int ignoring_children(struct sighand_struct *sigh)
    > -{
    > - int ret;
    > - spin_lock(&sigh->siglock);
    > - ret = (sigh->action[SIGCHLD-1].sa.sa_handler == SIG_IGN) ||
    > - (sigh->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT);
    > - spin_unlock(&sigh->siglock);
    > - return ret;
    > -}

    ...

    > @@ -565,14 +552,9 @@ static bool __ptrace_detach(struct task_struct *tracer, struct task_struct *p)
    >
    > dead = !thread_group_leader(p);
    >
    > - if (!dead && thread_group_empty(p)) {
    > - if (!same_thread_group(p->real_parent, tracer))
    > - dead = do_notify_parent(p, p->exit_signal);
    > - else if (ignoring_children(tracer->sighand)) {
    > - __wake_up_parent(p, tracer);
    > - dead = true;
    > - }
    > - }

    So the code above does:

    - if !same_thread_group(p->real_parent, tracer), then the tracee was
    ptrace_reparented(), and now we need to notify its natural parent
    to let it know it has a zombie child.

    - otherwise, the tracee is our natural child, and it is actually dead.
    however, since we are going to reap this task, we need to wake up our
    sub-threads possibly sleeping on ->wait_chldexit wait_queue_head_t.

    See?

    > + if (!dead && thread_group_empty(p))
    > + dead = do_notify_parent(p, p->exit_signal);

    No, this looks wrong. Or I missed something?

    Oleg.

    \
     
     \ /
      Last update: 2022-05-24 13:45    [W:6.517 / U:0.160 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site