Qianli Zhao <zhaoqianlig...@gmail.com> writes:
> 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 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>
The changelog is much better thank you.
As Oleg pointer out we need to do something like the code below.
diff --git a/kernel/exit.c b/kernel/exit.c
index 04029e35e69a..bc676c06ef9a 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -785,15 +785,16 @@ void __noreturn do_exit(long code)
sync_mm_rss(tsk->mm);
acct_update_integrals(tsk);
group_dead = atomic_dec_and_test(&tsk->signal->live);
+ /*
+ * If the 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);
+ }
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);
There is still a race that could lead to the BUG in zap_pid_ns_processes.
We still have a case where the last two threads of a process call
pthread_exit (aka do_exit not do_group_exit in the kernel).
Thread A Thread B
do_exit() do_exit()
exit_signals()
tsk->flags |= PF_EXITING;
group_dead = false;
exit_signals()
tsk->flags |= PF_EXITING;
exit_notify()
forget_original_parent
find_child_reaper
reaper = find_alive_thread()
zap_pid_ns_processes()
BUG()
group_dead = true;
if (is_global_init())
panic("Attemted to kill init");
As we are guaranteed to see the panic with my change above I suggest
we augment it by simply removing the BUG in zap_pid_ns_processes.
Or maybe not if there is a better way to write the panic code. I don't
think having pid namespaces compiled out is a particularly common case.
So whatever we can do to keep the code correct and reduce testing.
Eric
