lkml.org 
[lkml]   [2011]   [May]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH ptrace] ptrace: fix signal->wait_chldexit usage in task_clear_group_stop_trapping()
    Hello,

    On Sun, May 08, 2011 at 05:34:35PM +0200, Oleg Nesterov wrote:
    > But this is subjective too, and I agree that the future patches can
    > change the current trivial contract. So:
    >
    > Reviewed-by: Oleg Nesterov <oleg@redhat.com>
    >
    > I'll add this fix to my tree.

    Great, thanks.

    > > > Hmm. This is minor and off-topic, but perhaps it makes sense to move
    > > > the code which sets/waits GROUP_STOP_TRAPPING from ptrace_attach() to
    > > > the separate function, it can be called outside of tasklist_lock and
    > > > cred_guard_mutex.
    > >
    > > I'm confused. It should be set while siglock is held. Which place
    > > are you suggesting?
    >
    > Of course, we should take siglock. I meant, we could probably make
    >
    > static void ptrace_s_stopped_traced(struct task_struct *task)
    > {
    > bool wait_trap = false;
    >
    > spin_lock(&task->sighand->siglock);
    > /*
    > * If the task is already STOPPED, set GROUP_STOP_PENDING and
    > * TRAPPING, and kick it so that it transits to TRACED. TRAPPING
    > * will be cleared if the child completes the transition or any
    > * event which clears the group stop states happens. We'll wait
    > * for the transition to complete before returning from this
    > * function.
    > *
    > * This hides STOPPED -> RUNNING -> TRACED transition from the
    > * attaching thread but a different thread in the same group can
    > * still observe the transient RUNNING state. IOW, if another
    > * thread's WNOHANG wait(2) on the stopped tracee races against
    > * ATTACH, the wait(2) may fail due to the transient RUNNING.
    > *
    > * The following task_is_stopped() test is safe as both transitions
    > * in and out of STOPPED are protected by siglock.
    > */
    > if (task_is_stopped(task)) {
    > task->group_stop |= GROUP_STOP_PENDING | GROUP_STOP_TRAPPING;
    > signal_wake_up(task, 1);
    > wait_trap = true;
    > }
    > spin_unlock(&task->sighand->siglock);
    >
    > if (wait_trap)
    > wait_event(current->signal->wait_chldexit,
    > !(task->group_stop & GROUP_STOP_TRAPPING));
    > return retval;
    > }
    >
    > called by ptrace_attach() at the end.

    Ah, okay. Yeah, that might be a good idea but in the just posted
    patch series, I implemented a different helper and moved the wait back
    into ptrace_check_attach(), so if it needs further cleanup, it would
    be great if we could do it on top of the series.

    Thanks.

    --
    tejun


    \
     
     \ /
      Last update: 2011-05-08 17:59    [W:0.038 / U:92.300 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site