Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1265756 > unrolled thread
| Started by | Andrey Ryabinin <aryabinin@virtuozzo.com> |
|---|---|
| First post | 2015-11-09 16:20 +0100 |
| Last post | 2015-11-10 20:30 +0100 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
ptrace() hangs on attempt to seize/attach stopped & frozen task Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-11-09 16:20 +0100
Re: ptrace() hangs on attempt to seize/attach stopped & frozen task Oleg Nesterov <oleg@redhat.com> - 2015-11-09 19:00 +0100
Re: ptrace() hangs on attempt to seize/attach stopped & frozen task Tejun Heo <tj@kernel.org> - 2015-11-09 19:10 +0100
Re: ptrace() hangs on attempt to seize/attach stopped & frozen task Oleg Nesterov <oleg@redhat.com> - 2015-11-10 20:30 +0100
Re: ptrace() hangs on attempt to seize/attach stopped & frozen task Oleg Nesterov <oleg@redhat.com> - 2015-11-10 20:30 +0100
| From | Andrey Ryabinin <aryabinin@virtuozzo.com> |
|---|---|
| Date | 2015-11-09 16:20 +0100 |
| Subject | ptrace() hangs on attempt to seize/attach stopped & frozen task |
| Message-ID | <qsWoF-89v-3@gated-at.bofh.it> |
Hi,
So, the ptrace() hangs if we try to attach to stopped task from freezing cgroup.
It seems this was introduced by 5d8f72b55c2756("freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()").
See below for the exact scenario and small script to reproduce this.
Tracee: Tracer:
static bool do_signal_stop(int signr)
__set_current_state(TASK_STOPPED);
freezable_schedule();
freezer_do_not_count();
schedule(); /* waiting for wake up */
ptrace_attach()
if (task_is_stopped(task) &&
task_set_jobctl_pending(task, JOBCTL_TRAP_STOP | JOBCTL_TRAPPING))
signal_wake_up_state(task, __TASK_STOPPED);
/* woken up by ptrace_attach() */
freezer_count();
__refrigerator()
/* And here we will hang, because tracee is now frozen in __refrigerator() */
wait_on_bit(&task->jobctl, JOBCTL_TRAPPING_BIT,
TASK_UNINTERRUPTIBLE);
Reproduecer:
-----------
#!/bin/bash
sleep 100 &
pid=$!
kill -SIGSTOP $pid
mkdir -p /sys/fs/cgroup/freezer/test
echo $pid > /sys/fs/cgroup/freezer/test/tasks
echo FROZEN > /sys/fs/cgroup/freezer/test/freezer.state
echo '#include <stdlib.h>
#include <sys/ptrace.h>
int main(int argc, char **argv)
{
int pid = atoi(argv[1]);
return ptrace(PTRACE_SEIZE, pid, NULL, 0);
}
' | gcc -x c -
./a.out $pid
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Oleg Nesterov <oleg@redhat.com> |
|---|---|
| Date | 2015-11-09 19:00 +0100 |
| Message-ID | <qsYTw-18Q-9@gated-at.bofh.it> |
| In reply to | #1265756 |
Hi,
On 11/09, Andrey Ryabinin wrote:
>
> Hi,
>
> So, the ptrace() hangs if we try to attach to stopped task from freezing cgroup.
> It seems this was introduced by 5d8f72b55c2756("freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()").
quite possible...
> See below for the exact scenario and small script to reproduce this.
>
>
> Tracee: Tracer:
> static bool do_signal_stop(int signr)
> __set_current_state(TASK_STOPPED);
> freezable_schedule();
> freezer_do_not_count();
> schedule(); /* waiting for wake up */
>
> ptrace_attach()
> if (task_is_stopped(task) &&
> task_set_jobctl_pending(task, JOBCTL_TRAP_STOP | JOBCTL_TRAPPING))
> signal_wake_up_state(task, __TASK_STOPPED);
>
> /* woken up by ptrace_attach() */
> freezer_count();
> __refrigerator()
> /* And here we will hang, because tracee is now frozen in __refrigerator() */
> wait_on_bit(&task->jobctl, JOBCTL_TRAPPING_BIT,
> TASK_UNINTERRUPTIBLE);
Thanks. All I can say I never liked this wait_on_bit() ;)
I need to think, but *at first glance* we can move this wait-for-stopped-
traced-transition into do_wait() path, and this way clear_jobctl_trapping()
can use __wake_up_parent(). Perhaps we just need to modify task_stopped_code()
to take JOBCTL_TRAPPING into account...
Sure, debugger will block in sys_wait() after PTRACE_ATTACH/SEIZE. But this
does not really differ from the case when the tracee was already frozen;
SIGSTOP sent by ATTACH or PTRACE_INTERRUPT, so debugger will equally block
in do_wait() until the tracee is unfrozen.
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <tj@kernel.org> |
|---|---|
| Date | 2015-11-09 19:10 +0100 |
| Message-ID | <qsZ3e-1rz-45@gated-at.bofh.it> |
| In reply to | #1265906 |
Hello, Oleg. > Thanks. All I can say I never liked this wait_on_bit() ;) > > I need to think, but *at first glance* we can move this wait-for-stopped- > traced-transition into do_wait() path, and this way clear_jobctl_trapping() > can use __wake_up_parent(). Perhaps we just need to modify task_stopped_code() > to take JOBCTL_TRAPPING into account... > > Sure, debugger will block in sys_wait() after PTRACE_ATTACH/SEIZE. But this > does not really differ from the case when the tracee was already frozen; > SIGSTOP sent by ATTACH or PTRACE_INTERRUPT, so debugger will equally block > in do_wait() until the tracee is unfrozen. We simply need to reimplement cgroup freezer so that its userland visible state is well defined (most likely jobctl stop). Right now, it's allowing userland to trigger "stuck somewhere in the kernel" condition, so interactions with frozen tasks are naturally broken. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Oleg Nesterov <oleg@redhat.com> |
|---|---|
| Date | 2015-11-10 20:30 +0100 |
| Message-ID | <qtmM9-MK-17@gated-at.bofh.it> |
| In reply to | #1265919 |
Hi Tejun,
On 11/09, Tejun Heo wrote:
>
> Hello, Oleg.
>
> > Thanks. All I can say I never liked this wait_on_bit() ;)
> >
> > I need to think, but *at first glance* we can move this wait-for-stopped-
> > traced-transition into do_wait() path, and this way clear_jobctl_trapping()
> > can use __wake_up_parent(). Perhaps we just need to modify task_stopped_code()
> > to take JOBCTL_TRAPPING into account...
It seems that we can simply kill JOBCTL_TRAPPING, no other changes are
needed.
But afaics task_stopped_code()->task_is_stopped_or_traced() looks confusing,
it should not see a TASK_STOPPED task if ptrace, task_is_traced() makes more
sense. This is off-topic.
> > Sure, debugger will block in sys_wait() after PTRACE_ATTACH/SEIZE. But this
> > does not really differ from the case when the tracee was already frozen;
> > SIGSTOP sent by ATTACH or PTRACE_INTERRUPT, so debugger will equally block
> > in do_wait() until the tracee is unfrozen.
>
> We simply need to reimplement cgroup freezer so that its userland
> visible state is well defined (most likely jobctl stop). Right now,
> it's allowing userland to trigger "stuck somewhere in the kernel"
> condition, so interactions with frozen tasks are naturally broken.
I agree, the freezer is not perfect, and it needs changes.
Still I think this needs a fix in ptrace code. At least we should not
wait in TASK_UNINTERRUPTIBLE state.
And perhaps we can simply remove this logic? I forgot why do we hide this
STOPPED -> RUNNING -> TRACED transition from the attaching thread. But the
vague feeling tells me that we discussed this before and perhaps it was me
who suggested to avoid the user-visible change when you introduced this
transition...
Anyway, now I do not understand why do we want to hide it. Lets consider
the following "test-case",
void test(int pid)
{
kill(pid, SIGSTOP);
waitpid(pid, NULL, WSTOPPED);
ptrace(PTRACE_ATTACH-OR-PTRACE_SEIZE, pid, 0,0);
assert(ptrace(PTRACE_DETACH, pid, 0,0) == 0);
}
Yes, it will fail if we remove JOBCTL_TRAPPING. But it can equally fail
if SIGCONT comes before ATTACH, so perhaps we do not really care?
Jan, Pedro, do you think the patch below can break gdb somehow? With this
patch you can never assume that waitpid(WNOHANG) or ptrace(WHATEVER) will
succeed right after PTRACE_ATTACH/PTRACE_SEIZE, even if you know that the
tracee was TASK_STOPPED before attach.
Tejun, do you see any reason to keep JOBCTL_TRAPPING?
Oleg.
--- x/kernel/ptrace.c
+++ x/kernel/ptrace.c
@@ -338,21 +338,13 @@ static int ptrace_attach(struct task_struct *task, long request,
* If the task is already STOPPED, set JOBCTL_TRAP_STOP 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.
+ * event which clears the group stop states happens.
*
* 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_set_jobctl_pending(task, JOBCTL_TRAP_STOP | JOBCTL_TRAPPING))
+ task_set_jobctl_pending(task, JOBCTL_TRAP_STOP))
signal_wake_up_state(task, __TASK_STOPPED);
spin_unlock(&task->sighand->siglock);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Oleg Nesterov <oleg@redhat.com> |
|---|---|
| Date | 2015-11-10 20:30 +0100 |
| Message-ID | <qtmM9-MK-3@gated-at.bofh.it> |
| In reply to | #1265906 |
On 11/09, Oleg Nesterov wrote:
>
> Hi,
>
> On 11/09, Andrey Ryabinin wrote:
> >
> > Hi,
> >
> > So, the ptrace() hangs if we try to attach to stopped task from freezing cgroup.
> > It seems this was introduced by 5d8f72b55c2756("freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()").
>
> quite possible...
Not really, this was broken before that commit. Note that try_to_freeze()
was called under "relock:" label, so we had the same problem: the tracee
doesn't do do_jobctl_trap() after do_signal_stop() without try_to_freeze()
in between. Not that it matters, but still.
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web