Hi,Oleg
Thank you for your reply.
>> 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.
Yes, i try to fix this exception by test SIGNAL_GROUP_EXIT and call
panic before setting PF_EXITING to prevent zap_pid_ns_processes()
being called when init do_exit().
In addition, the patch also protects the init process state to
successfully get usable init coredump.
In my test,this patch works.
Oleg Nesterov <o...@redhat.com> 于2021年3月17日周三 下午10:38写道：
>
> On 03/17, Qianli Zhao wrote:
> >
> > From: Qianli Zhao <zhaoqia...@xiaomi.com>
> >
> > 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 <zhaoqia...@xiaomi.com>
> > ---
> > 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
> >
>
