Re: [PATCH V3] exit: trigger panic when global init has exited

[Oleg Nesterov]

On 03/17, Qianli Zhao wrote:
>
> From: Qianli Zhao <zhaoqianli@xxxxxxxxxx>
>
> When init sub-threads running on different CPUs exit at the same time,
> zap_pid_ns_processe()->BUG() may be happened.

and why do you think your patch can't prevent this?

Sorry, I must have missed something. But it seems to me that you are trying
to fix the wrong problem. Yes, zap_pid_ns_processes() must not be called in
the root namespace, and this has nothing to do with CONFIG_PID_NS.

> And every thread status is abnormal after exit(PF_EXITING set,task->mm=NULL etc),
> which makes it difficult to parse coredump from fulldump normally.
> In order to fix the above problem, when any one init has been set to SIGNAL_GROUP_EXIT,
> trigger panic immediately, and prevent other init threads from continuing to exit
>
> [   24.705376] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00007f00
> [   24.705382] CPU: 4 PID: 552 Comm: init Tainted: G S         O    4.14.180-perf-g4483caa8ae80-dirty #1
> [   24.705390] kernel BUG at include/linux/pid_namespace.h:98!
>
> PID: 552   CPU: 4   COMMAND: "init"
> PID: 1     CPU: 7   COMMAND: "init"
> core4                           core7
> ...                             sys_exit_group()
>                                 do_group_exit()
>                                    - sig->flags = SIGNAL_GROUP_EXIT
>                                    - zap_other_threads()
>                                 do_exit() //PF_EXITING is set
> ret_to_user()
> do_notify_resume()
> get_signal()
>     - signal_group_exit
>     - goto fatal;
> do_group_exit()
> do_exit() //PF_EXITING is set
>     - panic("Attempted to kill init! exitcode=0x%08x\n")
>                                 exit_notify()
>                                 find_alive_thread() //no alive sub-threads
>                                 zap_pid_ns_processes()//CONFIG_PID_NS is not set
>                                 BUG()
>
> Signed-off-by: Qianli Zhao <zhaoqianli@xxxxxxxxxx>
> ---
> V3:
> - Use group_dead instead of thread_group_empty() to test single init exit.
>
> V2:
> - Changelog update
> - Remove wrong useage of SIGNAL_UNKILLABLE.
> - Add thread_group_empty() test to handle single init thread exit
> ---
>  kernel/exit.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/exit.c b/kernel/exit.c
> index 04029e3..32b74e4 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -766,6 +766,17 @@ void __noreturn do_exit(long code)
>
>  	validate_creds_for_do_exit(tsk);
>
> +	group_dead = atomic_dec_and_test(&tsk->signal->live);
> +	/*
> +	 * If global init has exited,
> +	 * panic immediately to get a useable coredump.
> +	 */
> +	if (unlikely(is_global_init(tsk) &&
> +	    (group_dead || (tsk->signal->flags & SIGNAL_GROUP_EXIT)))) {
> +			panic("Attempted to kill init! exitcode=0x%08x\n",
> +				tsk->signal->group_exit_code ?: (int)code);
> +	}
> +
>  	/*
>  	 * We're taking recursive faults here in do_exit. Safest is to just
>  	 * leave this task alone and wait for reboot.
> @@ -784,16 +795,7 @@ void __noreturn do_exit(long code)
>  	if (tsk->mm)
>  		sync_mm_rss(tsk->mm);
>  	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);
> --
> 1.9.1
>