lkml.org 
[lkml]   [2022]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 6/9] signal: Always call do_notify_parent_cldstop with siglock held
    "Eric W. Biederman" <ebiederm@xmission.com> writes:

    > Oleg Nesterov <oleg@redhat.com> writes:
    >
    >> On 04/26, Eric W. Biederman wrote:
    >>>
    >>> @@ -2164,7 +2166,9 @@ static void do_notify_parent_cldstop(struct task_struct *tsk,
    >>> }
    >>>
    >>> sighand = parent->sighand;
    >>> - spin_lock_irqsave(&sighand->siglock, flags);
    >>> + lock = tsk->sighand != sighand;
    >>> + if (lock)
    >>> + spin_lock_nested(&sighand->siglock, SINGLE_DEPTH_NESTING);
    >>
    >> But why is it safe?
    >>
    >> Suppose we have two tasks, they both trace each other, both call
    >> ptrace_stop() at the same time. Of course this is ugly, they both
    >> will block.
    >>
    >> But with this patch in this case we have the trivial ABBA deadlock,
    >> no?
    >
    > I was thinking in terms of the process tree (which is fine).
    >
    > The ptrace parental relationship definitely has the potential to be a
    > graph with cycles. Which as you point out is not fine.
    >
    >
    > The result is very nice and I don't want to give it up. I suspect
    > something ptrace cycles are always a problem and can simply be
    > forbidden. That is going to take some analsysis and some additional
    > code in ptrace_attach.
    >
    > I will go look at that.


    Hmm. If we have the following process tree.

    A
    \
    B
    \
    C

    Process A, B, and C are all in the same process group.
    Process A and B are setup to receive SIGCHILD when
    their process stops.

    Process C traces process A.

    When a sigstop is delivered to the group we can have:

    Process B takes siglock(B) siglock(A) to notify the real_parent
    Process C takes siglock(C) siglock(B) to notify the real_parent
    Process A takes siglock(A) siglock(C) to notify the tracer

    If they all take their local lock at the same time there is
    a deadlock.

    I don't think the restriction that you can never ptrace anyone
    up the process tree is going to fly. So it looks like I am back to the
    drawing board for this one.

    Eric







    \
     
     \ /
      Last update: 2022-04-27 16:50    [W:4.305 / U:0.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site