Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1260644 > unrolled thread

WARNING in task_participate_group_stop

Back to article view | Back to linux.kernel

Contents

  WARNING in task_participate_group_stop Dmitry Vyukov <dvyukov@google.com> - 2015-11-02 14:30 +0100
    Re: WARNING in task_participate_group_stop Oleg Nesterov <oleg@redhat.com> - 2015-11-02 15:20 +0100
      Re: WARNING in task_participate_group_stop Dmitry Vyukov <dvyukov@google.com> - 2015-11-02 15:30 +0100
        Re: WARNING in task_participate_group_stop Oleg Nesterov <oleg@redhat.com> - 2015-11-02 15:40 +0100
      Re: WARNING in task_participate_group_stop Oleg Nesterov <oleg@redhat.com> - 2015-11-02 16:50 +0100
        Re: WARNING in task_participate_group_stop Oleg Nesterov <oleg@redhat.com> - 2015-11-02 18:20 +0100
          [PATCH 1/1] signal: kill the obsolete SIGNAL_UNKILLABLE check in  complete_signal() Oleg Nesterov <oleg@redhat.com> - 2015-11-04 19:30 +0100
            Re: [PATCH 1/1] signal: kill the obsolete SIGNAL_UNKILLABLE check  in complete_signal() Andrew Morton <akpm@linux-foundation.org> - 2015-11-05 02:30 +0100
              Re: [PATCH 1/1] signal: kill the obsolete SIGNAL_UNKILLABLE check  in complete_signal() Oleg Nesterov <oleg@redhat.com> - 2015-11-05 16:20 +0100
                Re: [PATCH 1/1] signal: kill the obsolete SIGNAL_UNKILLABLE check  in complete_signal() Oleg Nesterov <oleg@redhat.com> - 2015-11-05 18:10 +0100
          [PATCH 0/1] (Was: WARNING in task_participate_group_stop) Oleg Nesterov <oleg@redhat.com> - 2015-11-04 19:30 +0100

#1260644 — WARNING in task_participate_group_stop

Hello,

I've hit the following WARNING on 6a13feb9c82803e2b815eca72fa7a9f5561d7861 (4.3)

------------[ cut here ]------------
WARNING: CPU: 1 PID: 1 at kernel/signal.c:334
task_participate_group_stop+0x157/0x1d0()
Modules linked in:
CPU: 1 PID: 1 Comm: init Not tainted 4.3.0 #48
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 ffffffff82e40280 ffff88003eb0fae0 ffffffff819efe55 0000000000000000
 ffff88003eb0fb20 ffffffff810ec871 ffffffff8110f4d7 ffff88003eb00000
 ffff88003eb20000 0000000000000000 ffff88003eb0fbf8 ffff88003eb20000
Call Trace:
 [<ffffffff810eca35>] warn_slowpath_null+0x15/0x20 kernel/panic.c:480
 [<ffffffff8110f4d7>] task_participate_group_stop+0x157/0x1d0
kernel/signal.c:334
 [<ffffffff81113587>] do_signal_stop+0x1e7/0x6e0 kernel/signal.c:2060
 [<ffffffff81116ab7>] get_signal+0x387/0x11b0 kernel/signal.c:2316
 [<ffffffff8100cf0d>] do_signal+0x8d/0x19e0 arch/x86/kernel/signal.c:707
 [<ffffffff81005d8d>] prepare_exit_to_usermode+0x11d/0x170
arch/x86/entry/common.c:251
 [<ffffffff81005e83>] syscall_return_slowpath+0xa3/0x2b0
arch/x86/entry/common.c:317
 [<ffffffff82d4f6a7>] int_ret_from_sys_call+0x25/0x8f
arch/x86/entry/entry_64.S:281
---[ end trace f6697fd630b7c361 ]---


The reproducer is (needs to be run as root):

// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <sys/ptrace.h>
#include <unistd.h>

int main()
{
    int pid = 1;
    ptrace(PTRACE_ATTACH, pid, 0, 0);
    ptrace(PTRACE_SETOPTIONS, pid, 0, PTRACE_O_EXITKILL);
    sleep(1);
    return 0;
}

Yes, it is weird and it kills init right afterwards. But I wasn't able
to figure out what's the root cause (why task does not have
JOBCTL_STOP_PENDING) and maybe the same WARNING can be triggered
without root and/or with other than init process. So still posting it
here.

Can somebody more knowledgeable in ptrace please take a look at the root cause?

Thanks
--
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]

#1260672

Hi Dmitry,

On 11/02, Dmitry Vyukov wrote:
>
> WARNING: CPU: 1 PID: 1 at kernel/signal.c:334
> task_participate_group_stop+0x157/0x1d0()
> Modules linked in:
> CPU: 1 PID: 1 Comm: init Not tainted 4.3.0 #48
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>  ffffffff82e40280 ffff88003eb0fae0 ffffffff819efe55 0000000000000000
>  ffff88003eb0fb20 ffffffff810ec871 ffffffff8110f4d7 ffff88003eb00000
>  ffff88003eb20000 0000000000000000 ffff88003eb0fbf8 ffff88003eb20000
> Call Trace:
>  [<ffffffff810eca35>] warn_slowpath_null+0x15/0x20 kernel/panic.c:480
>  [<ffffffff8110f4d7>] task_participate_group_stop+0x157/0x1d0
> kernel/signal.c:334
>  [<ffffffff81113587>] do_signal_stop+0x1e7/0x6e0 kernel/signal.c:2060
>  [<ffffffff81116ab7>] get_signal+0x387/0x11b0 kernel/signal.c:2316
>  [<ffffffff8100cf0d>] do_signal+0x8d/0x19e0 arch/x86/kernel/signal.c:707
>  [<ffffffff81005d8d>] prepare_exit_to_usermode+0x11d/0x170
> arch/x86/entry/common.c:251
>  [<ffffffff81005e83>] syscall_return_slowpath+0xa3/0x2b0
> arch/x86/entry/common.c:317
>  [<ffffffff82d4f6a7>] int_ret_from_sys_call+0x25/0x8f
> arch/x86/entry/entry_64.S:281
> ---[ end trace f6697fd630b7c361 ]---
>
>
> The reproducer is (needs to be run as root):
>
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include <sys/ptrace.h>
> #include <unistd.h>
>
> int main()
> {
>     int pid = 1;
>     ptrace(PTRACE_ATTACH, pid, 0, 0);
>     ptrace(PTRACE_SETOPTIONS, pid, 0, PTRACE_O_EXITKILL);
>     sleep(1);
>     return 0;
> }

Thanks.

Can't reproduce, but at first glance the problem looks clear...

> Yes, it is weird and it kills init right afterwards.

Could you confirm that this WARN_ON() happens _after_ the reproducer exits?

> But I wasn't able
> to figure out what's the root cause (why task does not have
> JOBCTL_STOP_PENDING) and maybe the same WARNING can be triggered
> without root and/or with other than init process. So still posting it
> here.

Yes I think you are right. SIGSTOP can race with SIGKILL which (unlike SIGCONT)
doesn't clear JOBCTL_STOP_DEQUEUED/PENDING/etc.

This is mostly fine, the task won't block in TASK_STOPPED if SIGKILL is pending,
but still is not right and leads to the warning above: JOBCTL_STOP_PENDING was not
set because do_signal_stop()->task_set_jobctl_pending() checks fatal_signal_pending().

Probably the patch below should fix the problem, but I'd like to think more before
I send the fix.

Oleg.

--- x/kernel/signal.c
+++ x/kernel/signal.c
@@ -2002,7 +2002,7 @@ static bool do_signal_stop(int signr)
 		WARN_ON_ONCE(signr & ~JOBCTL_STOP_SIGMASK);
 
 		if (!likely(current->jobctl & JOBCTL_STOP_DEQUEUED) ||
-		    unlikely(signal_group_exit(sig)))
+		    unlikely(fatal_signal_pending(current)))
 			return false;
 		/*
 		 * There is no group stop already in progress.  We must

--
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]

#1260675

On Mon, Nov 2, 2015 at 4:13 PM, Oleg Nesterov <oleg@redhat.com> wrote:
> Hi Dmitry,
>
> On 11/02, Dmitry Vyukov wrote:
>>
>> WARNING: CPU: 1 PID: 1 at kernel/signal.c:334
>> task_participate_group_stop+0x157/0x1d0()
>> Modules linked in:
>> CPU: 1 PID: 1 Comm: init Not tainted 4.3.0 #48
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>  ffffffff82e40280 ffff88003eb0fae0 ffffffff819efe55 0000000000000000
>>  ffff88003eb0fb20 ffffffff810ec871 ffffffff8110f4d7 ffff88003eb00000
>>  ffff88003eb20000 0000000000000000 ffff88003eb0fbf8 ffff88003eb20000
>> Call Trace:
>>  [<ffffffff810eca35>] warn_slowpath_null+0x15/0x20 kernel/panic.c:480
>>  [<ffffffff8110f4d7>] task_participate_group_stop+0x157/0x1d0
>> kernel/signal.c:334
>>  [<ffffffff81113587>] do_signal_stop+0x1e7/0x6e0 kernel/signal.c:2060
>>  [<ffffffff81116ab7>] get_signal+0x387/0x11b0 kernel/signal.c:2316
>>  [<ffffffff8100cf0d>] do_signal+0x8d/0x19e0 arch/x86/kernel/signal.c:707
>>  [<ffffffff81005d8d>] prepare_exit_to_usermode+0x11d/0x170
>> arch/x86/entry/common.c:251
>>  [<ffffffff81005e83>] syscall_return_slowpath+0xa3/0x2b0
>> arch/x86/entry/common.c:317
>>  [<ffffffff82d4f6a7>] int_ret_from_sys_call+0x25/0x8f
>> arch/x86/entry/entry_64.S:281
>> ---[ end trace f6697fd630b7c361 ]---
>>
>>
>> The reproducer is (needs to be run as root):
>>
>> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>> #include <sys/ptrace.h>
>> #include <unistd.h>
>>
>> int main()
>> {
>>     int pid = 1;
>>     ptrace(PTRACE_ATTACH, pid, 0, 0);
>>     ptrace(PTRACE_SETOPTIONS, pid, 0, PTRACE_O_EXITKILL);
>>     sleep(1);
>>     return 0;
>> }
>
> Thanks.
>
> Can't reproduce, but at first glance the problem looks clear...

Humm... did you run as root?
It reproduces all the time on my 4.3 kernel VM. Also firmly killed my
desktop running 3.13.



>> Yes, it is weird and it kills init right afterwards.
>
> Could you confirm that this WARN_ON() happens _after_ the reproducer exits?
>
>> But I wasn't able
>> to figure out what's the root cause (why task does not have
>> JOBCTL_STOP_PENDING) and maybe the same WARNING can be triggered
>> without root and/or with other than init process. So still posting it
>> here.
>
> Yes I think you are right. SIGSTOP can race with SIGKILL which (unlike SIGCONT)
> doesn't clear JOBCTL_STOP_DEQUEUED/PENDING/etc.
>
> This is mostly fine, the task won't block in TASK_STOPPED if SIGKILL is pending,
> but still is not right and leads to the warning above: JOBCTL_STOP_PENDING was not
> set because do_signal_stop()->task_set_jobctl_pending() checks fatal_signal_pending().
>
> Probably the patch below should fix the problem, but I'd like to think more before
> I send the fix.


Will test it.


> Oleg.
>
> --- x/kernel/signal.c
> +++ x/kernel/signal.c
> @@ -2002,7 +2002,7 @@ static bool do_signal_stop(int signr)
>                 WARN_ON_ONCE(signr & ~JOBCTL_STOP_SIGMASK);
>
>                 if (!likely(current->jobctl & JOBCTL_STOP_DEQUEUED) ||
> -                   unlikely(signal_group_exit(sig)))
> +                   unlikely(fatal_signal_pending(current)))
>                         return false;
>                 /*
>                  * There is no group stop already in progress.  We must
>
--
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]

#1260685

On 11/02, Dmitry Vyukov wrote:
>
> On Mon, Nov 2, 2015 at 4:13 PM, Oleg Nesterov <oleg@redhat.com> wrote:
> > Hi Dmitry,
> >
> > On 11/02, Dmitry Vyukov wrote:
> >>
> >> WARNING: CPU: 1 PID: 1 at kernel/signal.c:334
> >> task_participate_group_stop+0x157/0x1d0()
> >> Modules linked in:
> >> CPU: 1 PID: 1 Comm: init Not tainted 4.3.0 #48
> >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> >>  ffffffff82e40280 ffff88003eb0fae0 ffffffff819efe55 0000000000000000
> >>  ffff88003eb0fb20 ffffffff810ec871 ffffffff8110f4d7 ffff88003eb00000
> >>  ffff88003eb20000 0000000000000000 ffff88003eb0fbf8 ffff88003eb20000
> >> Call Trace:
> >>  [<ffffffff810eca35>] warn_slowpath_null+0x15/0x20 kernel/panic.c:480
> >>  [<ffffffff8110f4d7>] task_participate_group_stop+0x157/0x1d0
> >> kernel/signal.c:334
> >>  [<ffffffff81113587>] do_signal_stop+0x1e7/0x6e0 kernel/signal.c:2060
> >>  [<ffffffff81116ab7>] get_signal+0x387/0x11b0 kernel/signal.c:2316
> >>  [<ffffffff8100cf0d>] do_signal+0x8d/0x19e0 arch/x86/kernel/signal.c:707
> >>  [<ffffffff81005d8d>] prepare_exit_to_usermode+0x11d/0x170
> >> arch/x86/entry/common.c:251
> >>  [<ffffffff81005e83>] syscall_return_slowpath+0xa3/0x2b0
> >> arch/x86/entry/common.c:317
> >>  [<ffffffff82d4f6a7>] int_ret_from_sys_call+0x25/0x8f
> >> arch/x86/entry/entry_64.S:281
> >> ---[ end trace f6697fd630b7c361 ]---
> >>
> >>
> >> The reproducer is (needs to be run as root):
> >>
> >> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> >> #include <sys/ptrace.h>
> >> #include <unistd.h>
> >>
> >> int main()
> >> {
> >>     int pid = 1;
> >>     ptrace(PTRACE_ATTACH, pid, 0, 0);
> >>     ptrace(PTRACE_SETOPTIONS, pid, 0, PTRACE_O_EXITKILL);
> >>     sleep(1);
> >>     return 0;
> >> }
> >
> > Thanks.
> >
> > Can't reproduce, but at first glance the problem looks clear...
>
> Humm... did you run as root?

Yes,

> It reproduces all the time on my 4.3 kernel VM. Also firmly killed my
> desktop running 3.13.

Yes, it kills init and crashes the kernel. But I do not see the warning.


> >> Yes, it is weird and it kills init right afterwards.
> >
> > Could you confirm that this WARN_ON() happens _after_ the reproducer exits?
> >
> >> But I wasn't able
> >> to figure out what's the root cause (why task does not have
> >> JOBCTL_STOP_PENDING) and maybe the same WARNING can be triggered
> >> without root and/or with other than init process. So still posting it
> >> here.
> >
> > Yes I think you are right. SIGSTOP can race with SIGKILL which (unlike SIGCONT)
> > doesn't clear JOBCTL_STOP_DEQUEUED/PENDING/etc.
> >
> > This is mostly fine, the task won't block in TASK_STOPPED if SIGKILL is pending,
> > but still is not right and leads to the warning above: JOBCTL_STOP_PENDING was not
> > set because do_signal_stop()->task_set_jobctl_pending() checks fatal_signal_pending().

On a second thought, in this particular case (your test-case), SIGSTOP/SIGKILL
do not race, although (so far) I think this doesn't matter. JOBCTL_STOP_PENDING
comes from __ptrace_unlink() when the tracee already has the pending SIGKILL
due to PTRACE_O_EXITKILL.

Now. If the tracee (init) wakes up and dequeues SIGKILL before __ptrace_unlink()
adds JOBCTL_STOP_PENDING, it won't see JOBCTL_STOP_PENDING and probably this is
what happens on my testing machine.

Perhaps __ptrace_unlink() should me more carefull too...

> > Probably the patch below should fix the problem, but I'd like to think more before
> > I send the fix.
>
>
> Will test it.

Great, thanks.

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]

#1260739

On 11/02, Oleg Nesterov wrote:
>
> On 11/02, Dmitry Vyukov wrote:
> >
> > But I wasn't able
> > to figure out what's the root cause (why task does not have
> > JOBCTL_STOP_PENDING) and maybe the same WARNING can be triggered
> > without root and/or with other than init process. So still posting it
> > here.
>
> Yes I think you are right. SIGSTOP can race with SIGKILL which (unlike SIGCONT)
> doesn't clear JOBCTL_STOP_DEQUEUED/PENDING/etc.

I was wrong... I forgot that complete_signal(SIGKILL) sets SIGNAL_GROUP_EXIT.
Unless SIGNAL_UNKILLABLE is set, and this is what makes init "special".

So it seems that everything is clear. Except I can't understand why your
test-case doesn't work for me ;) It should.

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]

#1260809

On 11/02, Oleg Nesterov wrote:
>
> On 11/02, Oleg Nesterov wrote:
> >
> > On 11/02, Dmitry Vyukov wrote:
> > >
> > > But I wasn't able
> > > to figure out what's the root cause (why task does not have
> > > JOBCTL_STOP_PENDING) and maybe the same WARNING can be triggered
> > > without root and/or with other than init process. So still posting it
> > > here.
> >
> > Yes I think you are right. SIGSTOP can race with SIGKILL which (unlike SIGCONT)
> > doesn't clear JOBCTL_STOP_DEQUEUED/PENDING/etc.
>
> I was wrong... I forgot that complete_signal(SIGKILL) sets SIGNAL_GROUP_EXIT.
> Unless SIGNAL_UNKILLABLE is set, and this is what makes init "special".

and I think this should be fixed anyway, if nothing else to make this logic more
correct. I'll try to recheck this all later.

> So it seems that everything is clear. Except I can't understand why your
> test-case doesn't work for me ;) It should.

Damn. Sorry Dmitry it actually works. Just I didn't see the output from
pr_warn()'s on the serial console due to the wrong console_loglevel.

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]

#1262489 — [PATCH 1/1] signal: kill the obsolete SIGNAL_UNKILLABLE check in complete_signal()

complete_signal() checks SIGNAL_UNKILLABLE before it starts to destroy the
thread group, today this is unnecessary and even not 100% correct.

After the commit f008faff0e27 ("signals: protect init from unwanted signals
more") we rely on sig_task_ignored(), complete_signal(SIGKILL) can only see
a SIGNAL_UNKILLABLE task if we actually want to kill it. And note that after
the commit b3bfa0cba867 ("signals: protect cinit from blocked fatal signals")
we do not drop SIGKILL dequeued by /sbin/init.

And it does not look right. fatal_signal_pending() should always imply that
the whole thread group (except ->group_exit_task if it is not NULL) is killed,
this check breaks the rule.

This explains WARN_ON(!JOBCTL_STOP_PENDING) in task_participate_group_stop()
triggered by the test-case from Dmitry:

	int main()
	{
		int pid = 1;
		ptrace(PTRACE_ATTACH, pid, 0, 0);
		ptrace(PTRACE_SETOPTIONS, pid, 0, PTRACE_O_EXITKILL);
		sleep(1);
		return 0;
	}

do_signal_stop()->signal_group_exit() returns false because SIGNAL_GROUP_EXIT
is not set, but task_set_jobctl_pending() checks fatal_signal_pending() and
does not set JOBCTL_STOP_PENDING.

The test-case above needs root and (correctly) crashes the kernel, but we can
trigger the same warning inside the container or using another test-case:

	static int init(void *arg)
	{
		for (;;)
			pause();
	}

	int main(void)
	{
		char stack[16 * 1024];

		for (;;) {
			int pid = clone(init, stack + sizeof(stack)/2,
					CLONE_NEWPID | SIGCHLD, NULL);
			assert(pid > 0);

			assert(ptrace(PTRACE_ATTACH, pid, 0, 0) == 0);
			assert(waitpid(-1, NULL, WSTOPPED) == pid);

			assert(ptrace(PTRACE_DETACH, pid, 0, SIGSTOP) == 0);
			assert(syscall(__NR_tkill, pid, SIGKILL) == 0);
			assert(pid == wait(NULL));
		}
	}

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/signal.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index d64efad..9b6f385 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -914,7 +914,7 @@ static void complete_signal(int sig, struct task_struct *p, int group)
 	 * then start taking the whole group down immediately.
 	 */
 	if (sig_fatal(p, sig) &&
-	    !(signal->flags & (SIGNAL_UNKILLABLE | SIGNAL_GROUP_EXIT)) &&
+	    !(signal->flags & SIGNAL_GROUP_EXIT) &&
 	    !sigismember(&t->real_blocked, sig) &&
 	    (sig == SIGKILL || !t->ptrace)) {
 		/*
-- 
1.5.5.1


--
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]

#1262795 — Re: [PATCH 1/1] signal: kill the obsolete SIGNAL_UNKILLABLE check in complete_signal()

On Wed, 4 Nov 2015 20:19:12 +0100 Oleg Nesterov <oleg@redhat.com> wrote:

> complete_signal() checks SIGNAL_UNKILLABLE before it starts to destroy the
> thread group, today this is unnecessary and even not 100% correct.
> 
> After the commit f008faff0e27 ("signals: protect init from unwanted signals
> more") we rely on sig_task_ignored(), complete_signal(SIGKILL) can only see
> a SIGNAL_UNKILLABLE task if we actually want to kill it. And note that after
> the commit b3bfa0cba867 ("signals: protect cinit from blocked fatal signals")
> we do not drop SIGKILL dequeued by /sbin/init.
> 
> And it does not look right. fatal_signal_pending() should always imply that
> the whole thread group (except ->group_exit_task if it is not NULL) is killed,
> this check breaks the rule.
> 
> This explains WARN_ON(!JOBCTL_STOP_PENDING) in task_participate_group_stop()
> triggered by the test-case from Dmitry:
> 
> 	int main()
> 	{
> 		int pid = 1;
> 		ptrace(PTRACE_ATTACH, pid, 0, 0);
> 		ptrace(PTRACE_SETOPTIONS, pid, 0, PTRACE_O_EXITKILL);
> 		sleep(1);
> 		return 0;
> 	}
> 
> do_signal_stop()->signal_group_exit() returns false because SIGNAL_GROUP_EXIT
> is not set, but task_set_jobctl_pending() checks fatal_signal_pending() and
> does not set JOBCTL_STOP_PENDING.
> 
> The test-case above needs root and (correctly) crashes the kernel, but we can
> trigger the same warning inside the container or using another test-case:
> 
> 	static int init(void *arg)
> 	{
> 		for (;;)
> 			pause();
> 	}
> 
> 	int main(void)
> 	{
> 		char stack[16 * 1024];
> 
> 		for (;;) {
> 			int pid = clone(init, stack + sizeof(stack)/2,
> 					CLONE_NEWPID | SIGCHLD, NULL);
> 			assert(pid > 0);
> 
> 			assert(ptrace(PTRACE_ATTACH, pid, 0, 0) == 0);
> 			assert(waitpid(-1, NULL, WSTOPPED) == pid);
> 
> 			assert(ptrace(PTRACE_DETACH, pid, 0, SIGSTOP) == 0);
> 			assert(syscall(__NR_tkill, pid, SIGKILL) == 0);
> 			assert(pid == wait(NULL));
> 		}
> 	}

I'm thinking this should be backported into -stable due to WARN_ONs and
kernel crashes.  And as f008faff0e27 is from 2009, that means all
kernels.

Your thoughts on this?
--
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]

#1263324 — Re: [PATCH 1/1] signal: kill the obsolete SIGNAL_UNKILLABLE check in complete_signal()

On 11/04, Andrew Morton wrote:
>
> On Wed, 4 Nov 2015 20:19:12 +0100 Oleg Nesterov <oleg@redhat.com> wrote:
>
> > This explains WARN_ON(!JOBCTL_STOP_PENDING) in task_participate_group_stop()
> > triggered by the test-case from Dmitry:
> >
> > 	int main()
> > 	{
> > 		int pid = 1;
> > 		ptrace(PTRACE_ATTACH, pid, 0, 0);
> > 		ptrace(PTRACE_SETOPTIONS, pid, 0, PTRACE_O_EXITKILL);
> > 		sleep(1);
> > 		return 0;
> > 	}

...

> > The test-case above needs root and (correctly) crashes the kernel,

> I'm thinking this should be backported into -stable due to WARN_ONs and
> kernel crashes.

Ah, sorry for confusion. The kernel crash is fine/correct. Debugger kills
init process, the exiting init calls panic(). With or without this patch.
BTW, I always thought we should remove this panic(), but this is off-topic.

After this patch the test-case above still crashes the kernel, but without
warning ;)

> And as f008faff0e27 is from 2009, that means all
> kernels.

Yes, I think this change is safe for -stable. But the only visible problem
is WARN_ON_ONCE() in task_participate_group_stop(), so I am not sure...

Well. Actually there are more problems. zap_threads(), de_thread() can be
fooled by signal_group_exit() == F too. So a multi-threaded /sbin/init can
miss SIGKILL if it does execve(), or if it starts the coredump. But only if
SIGKILL was private (sent by tkill).

I do not see any serious problem this patch could fix.

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]

#1263433 — Re: [PATCH 1/1] signal: kill the obsolete SIGNAL_UNKILLABLE check in complete_signal()

On 11/05, Oleg Nesterov wrote:
>
> On 11/04, Andrew Morton wrote:
> >
> > I'm thinking this should be backported into -stable due to WARN_ONs and
> > kernel crashes.
>
> Ah, sorry for confusion. The kernel crash is fine/correct. Debugger kills
> init process, the exiting init calls panic(). With or without this patch.
> BTW, I always thought we should remove this panic(), but this is off-topic.
>
> After this patch the test-case above still crashes the kernel, but without
> warning ;)
>
> > And as f008faff0e27 is from 2009, that means all
> > kernels.
>
> Yes, I think this change is safe for -stable. But the only visible problem
> is WARN_ON_ONCE() in task_participate_group_stop(), so I am not sure...
>
> Well. Actually there are more problems. zap_threads(), de_thread() can be
> fooled by signal_group_exit() == F too. So a multi-threaded /sbin/init can
> miss SIGKILL if it does execve(), or if it starts the coredump. But only if
> SIGKILL was private (sent by tkill).
>
> I do not see any serious problem this patch could fix.

Cough... and on the second thought this patch needs v2. Sorry Andrew, please
drop signal-kill-the-obsolete-signal_unkillable-check-in-complete_signal.patch
I'll send the updated version.

With this patch the parent namespace can use any fatal signal (not only SIGKILL)
to kill the init process in container. I do not think this is actually bad, but
in any case this should not silently come as a side effect. And this is not
consistent with SIGNAL_UNKILLABLE/sig_kernel_only() check in get_signal().

Most probably I will just resend this patch as 2/2, while 1/2 will change
sig_task_ignored() because afaics it is not actually right too (albeit not
really buggy).

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]

#1262492 — [PATCH 0/1] (Was: WARNING in task_participate_group_stop)

On 11/02, Oleg Nesterov wrote:
>
> and I think this should be fixed anyway, if nothing else to make this logic more
> correct.

Yes, please see the patch.

> I'll try to recheck this all later.

I found more problems, will try to send the fixes/cleanups tomorrow...

do_signal_stop() is buggy, a multi-threaded exec can miss SIGSTOP.

task_participate_group_stop() and prepare_signal() are buggy. We
must not clear UNKILLABLE when we set STOPPED/CONTINUED.

signal_group_exit() can probably die, it was added before we had
fatal_signal_pending().

This reminds that we should finally remove signal_pending() check
in fatal_signal_pending(). And change force_sig_info() to take
->ptrace into account.

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