lkml.org 
[lkml]   [2015]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: ptrace() hangs on attempt to seize/attach stopped & frozen task
    Hey, Oleg.

    On Tue, Nov 17, 2015 at 08:34:19PM +0100, Oleg Nesterov wrote:
    > On 11/16, Tejun Heo wrote:
    > >
    > > *** WARNING: THE ATTACHED DOCUMENT(S) CONTAIN MACROS ***
    > > *** MACROS MAY CONTAIN MALICIOUS CODE ***
    > > *** Open only if you can verify and trust the sender ***
    > > *** Please contact infosec@redhat.com if you have questions or concerns **
    >
    > Hmm, infosec@redhat.com doesn't like you. But I dared to open and nothing
    > happened so far. although perhaps you already own my machine.

    lol no idea what's going on there but dude you gotta clean up the
    browsing history.

    > > Hmmm... It's nasty tho. We're breaking a guaranteed userland behavior
    >
    > Perhaps you are right, but I am wondering if it was ever guaranteed.
    >
    > What actually annoys me is that now I am almost sure that it was me
    > who asked you to hide this from user-space, and today I see no reason
    > for this hack.
    >
    > > I'd be a lot more comfortable stating
    > > that cgroup freezer is currently broken rather than diddling with
    > > subtle ptrace semantics.
    >
    > OK, lets keep this JOBCTL_TRAPPING_BIT.
    >
    > But still I would like to know what Pedro thinks...
    >
    > Anyway, wait_on_bit(TASK_UNINTERRUPTIBLE) doesn't look good. Do you
    > see any problem with the change below? Yes, the comment is not clear,
    > it should be updated, the tracee can clear this bit too.
    >
    > And perhaps we can change get_task_state() until freezer gets another state,
    >
    > --- x/fs/proc/array.c
    > +++ x/fs/proc/array.c
    > @@ -126,6 +126,9 @@ static inline const char *get_task_state
    > {
    > unsigned int state = (tsk->state | tsk->exit_state) & TASK_REPORT;
    >
    > + if (tsk->flags & PF_FROZEN)
    > + return "D (frozen)";
    > +
    > BUILD_BUG_ON(1 + ilog2(TASK_REPORT) != ARRAY_SIZE(task_state_array)-1);
    >
    > return task_state_array[fls(state)];

    Hmm... the only nit is that we'll eventually want to share "T
    (stopped)" or do "T (frozen)" and switching down the road could be a
    bit confusing. It shouldn't be a big deal tho. I think I'm mostly
    reluctant to accomodate the broken behavior of cgroup freezer.

    > --- x/kernel/ptrace.c
    > +++ x/kernel/ptrace.c
    > @@ -364,8 +364,13 @@ unlock_creds:
    > mutex_unlock(&task->signal->cred_guard_mutex);
    > out:
    > if (!retval) {
    > - wait_on_bit(&task->jobctl, JOBCTL_TRAPPING_BIT,
    > - TASK_UNINTERRUPTIBLE);
    > + if (wait_on_bit(&task->jobctl, JOBCTL_TRAPPING_BIT,
    > + TASK_KILLABLE))
    > + /*
    > + * We will clear JOBCTL_TRAPPING in __ptrace_unlink(),
    > + * until then nobody can trace this task anyway.
    > + */
    > + retval = -EINTR;

    Yeah, this looks good to me.

    Thanks.

    --
    tejun


    \
     
     \ /
      Last update: 2015-11-17 20:21    [W:3.520 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site