Messages in this thread | | | Date | Tue, 17 Nov 2015 13:57:10 -0500 | From | Tejun Heo <> | Subject | Re: 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
| |