[PATCH v3] kernel/exit: do panic earlier to get coredump if global init task exit

[PATCH v3] kernel/exit: do panic earlier to get coredump if global init task exit

[qiwuchen55]

From: chenqiwu <chenqiwu@xiaomi.com>

When global init task get a chance to be killed, panic will happen in
later calling steps by do_exit()->exit_notify()->forget_original_parent()
->find_child_reaper() if all init threads have exited.

However, it's hard to extract the coredump of init task from a kernel
crashdump, since exit_mm() has released its mm before panic. In order
to get the backtrace of init task in userspace, it's better to do panic
earlier at the beginning of exitting route.

It's worth noting that we must take case of a multi-threaded init exitting
issue. We need the test for is_global_init() && group_dead to ensure that
it is all threads exiting and not just the current thread.

Signed-off-by: chenqiwu <chenqiwu@xiaomi.com>
---
changes in v3:
 - move panic into group_dead condition.
 - keep exitcode as the original code does.
 - fix logic error for comment.
---
 kernel/exit.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index bcbd598..7271e13 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -517,10 +517,6 @@ static struct task_struct *find_child_reaper(struct task_struct *father,
 	}
 
 	write_unlock_irq(&tasklist_lock);
-	if (unlikely(pid_ns == &init_pid_ns)) {
-		panic("Attempted to kill init! exitcode=0x%08x\n",
-			father->signal->group_exit_code ?: father->exit_code);
-	}
 
 	list_for_each_entry_safe(p, n, dead, ptrace_entry) {
 		list_del_init(&p->ptrace_entry);
@@ -766,6 +762,15 @@ void __noreturn do_exit(long code)
 	acct_update_integrals(tsk);
 	group_dead = atomic_dec_and_test(&tsk->signal->live);
 	if (group_dead) {
+		/*
+		 * If the last thread of global init exit, do panic
+		 * immeddiately to get the coredump to find any clue
+		 * for init task in userspace.
+		 */
+		if (unlikely(is_global_init(tsk)))
+			panic("Attempted to kill init! exitcode=0x%08x\n",
+				tsk->signal->group_exit_code ?: (int)code);
+
 #ifdef CONFIG_POSIX_TIMERS
 		hrtimer_cancel(&tsk->signal->real_timer);
 		exit_itimers(tsk->signal);
-- 
1.9.1

Re: [PATCH v3] kernel/exit: do panic earlier to get coredump if global init task exit

[Christian Brauner]

On Thu, Dec 19, 2019 at 02:29:53PM +0800, qiwuchen55@gmail.com wrote:
> From: chenqiwu <chenqiwu@xiaomi.com>
> 
> When global init task get a chance to be killed, panic will happen in
> later calling steps by do_exit()->exit_notify()->forget_original_parent()
> ->find_child_reaper() if all init threads have exited.
> 
> However, it's hard to extract the coredump of init task from a kernel
> crashdump, since exit_mm() has released its mm before panic. In order
> to get the backtrace of init task in userspace, it's better to do panic
> earlier at the beginning of exitting route.
> 
> It's worth noting that we must take case of a multi-threaded init exitting
> issue. We need the test for is_global_init() && group_dead to ensure that
> it is all threads exiting and not just the current thread.
> 
> Signed-off-by: chenqiwu <chenqiwu@xiaomi.com>

Acked-by: Christian Brauner <christian.brauner@ubuntu.com>

Re: [PATCH v3] kernel/exit: do panic earlier to get coredump if global init task exit

[Oleg Nesterov]

On 12/19, qiwuchen55@gmail.com wrote:
>
> @@ -517,10 +517,6 @@ static struct task_struct *find_child_reaper(struct task_struct *father,
>  	}
>  
>  	write_unlock_irq(&tasklist_lock);
> -	if (unlikely(pid_ns == &init_pid_ns)) {
> -		panic("Attempted to kill init! exitcode=0x%08x\n",
> -			father->signal->group_exit_code ?: father->exit_code);
> -	}
>  
>  	list_for_each_entry_safe(p, n, dead, ptrace_entry) {
>  		list_del_init(&p->ptrace_entry);
> @@ -766,6 +762,15 @@ void __noreturn do_exit(long code)
>  	acct_update_integrals(tsk);
>  	group_dead = atomic_dec_and_test(&tsk->signal->live);
>  	if (group_dead) {
> +		/*
> +		 * If the last thread of global init exit, do panic
> +		 * immeddiately to get the coredump to find any clue
> +		 * for init task in userspace.
> +		 */
> +		if (unlikely(is_global_init(tsk)))
> +			panic("Attempted to kill init! exitcode=0x%08x\n",
> +				tsk->signal->group_exit_code ?: (int)code);
> +

Acked-by: Oleg Nesterov <oleg@redhat.com>

Re: [PATCH v3] kernel/exit: do panic earlier to get coredump if global init task exit

[Christian Brauner]

On December 20, 2019 8:38:00 PM GMT+01:00, Oleg Nesterov <oleg@redhat.com> wrote:
>On 12/19, qiwuchen55@gmail.com wrote:
>>
>> @@ -517,10 +517,6 @@ static struct task_struct
>*find_child_reaper(struct task_struct *father,
>>  	}
>>  
>>  	write_unlock_irq(&tasklist_lock);
>> -	if (unlikely(pid_ns == &init_pid_ns)) {
>> -		panic("Attempted to kill init! exitcode=0x%08x\n",
>> -			father->signal->group_exit_code ?: father->exit_code);
>> -	}
>>  
>>  	list_for_each_entry_safe(p, n, dead, ptrace_entry) {
>>  		list_del_init(&p->ptrace_entry);
>> @@ -766,6 +762,15 @@ void __noreturn do_exit(long code)
>>  	acct_update_integrals(tsk);
>>  	group_dead = atomic_dec_and_test(&tsk->signal->live);
>>  	if (group_dead) {
>> +		/*
>> +		 * If the last thread of global init exit, do panic
>> +		 * immeddiately to get the coredump to find any clue
>> +		 * for init task in userspace.
>> +		 */
>> +		if (unlikely(is_global_init(tsk)))
>> +			panic("Attempted to kill init! exitcode=0x%08x\n",
>> +				tsk->signal->group_exit_code ?: (int)code);
>> +
>
>Acked-by: Oleg Nesterov <oleg@redhat.com>

Thanks. I'll pick this up unless someone objects.

Christian

Applied patch "exit: panic before exit_mm() on global init exit"

[Christian Brauner]

I've added the patch:

  exit: panic before exit_mm() on global init exit

to
  https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=fixes

I've rewritten the commit message and fixed a typo in the comment.

If noone objects this will be part of the threads-fixes pr early next week.

Thanks!
Christian

From 43cf75d96409a20ef06b756877a2e72b10a026fc Mon Sep 17 00:00:00 2001
From: chenqiwu <chenqiwu@xiaomi.com>
Date: Thu, 19 Dec 2019 14:29:53 +0800
Subject: [PATCH] exit: panic before exit_mm() on global init exit

Currently, when global init and all threads in its thread-group have exited
we panic via:
do_exit()
-> exit_notify()
   -> forget_original_parent()
      -> find_child_reaper()
This makes it hard to extract a useable coredump for global init from a
kernel crashdump because by the time we panic exit_mm() will have already
released global init's mm.
This patch moves the panic futher up before exit_mm() is called. As was the
case previously, we only panic when global init and all its threads in the
thread-group have exited.

Signed-off-by: chenqiwu <chenqiwu@xiaomi.com>
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Acked-by: Oleg Nesterov <oleg@redhat.com>
[christian.brauner@ubuntu.com: fix typo, rewrite commit message]
Link: https://lore.kernel.org/r/1576736993-10121-1-git-send-email-qiwuchen55@gmail.com
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 kernel/exit.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index a46a50d67002..fc364272759d 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -517,10 +517,6 @@ static struct task_struct *find_child_reaper(struct task_struct *father,
 	}
 
 	write_unlock_irq(&tasklist_lock);
-	if (unlikely(pid_ns == &init_pid_ns)) {
-		panic("Attempted to kill init! exitcode=0x%08x\n",
-			father->signal->group_exit_code ?: father->exit_code);
-	}
 
 	list_for_each_entry_safe(p, n, dead, ptrace_entry) {
 		list_del_init(&p->ptrace_entry);
@@ -786,6 +782,14 @@ void __noreturn do_exit(long code)
 	acct_update_integrals(tsk);
 	group_dead = atomic_dec_and_test(&tsk->signal->live);
 	if (group_dead) {
+		/*
+		 * If the last thread of global init has exited, panic
+		 * immediately to get a useable coredump.
+		 */
+		if (unlikely(is_global_init(tsk)))
+			panic("Attempted to kill init! exitcode=0x%08x\n",
+				tsk->signal->group_exit_code ?: (int)code);
+
 #ifdef CONFIG_POSIX_TIMERS
 		hrtimer_cancel(&tsk->signal->real_timer);
 		exit_itimers(tsk->signal);
-- 
2.24.0