* [PATCH V2] exit: trigger panic when global init has exited
@ 2021-03-12 3:24 Qianli Zhao
2021-03-12 16:23 ` Oleg Nesterov
2021-03-12 18:23 ` Eric W. Biederman
0 siblings, 2 replies; 4+ messages in thread
From: Qianli Zhao @ 2021-03-12 3:24 UTC (permalink / raw)
To: christian, axboe, ebiederm, oleg, tglx, pcc; +Cc: linux-kernel, zhaoqianli
From: Qianli Zhao <zhaoqianli@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 <zhaoqianli@xiaomi.com>
---
V2:
- Changelog update
- Remove wrong useage of SIGNAL_UNKILLABLE.
- Add thread_group_empty() test to handle single init thread exit
---
kernel/exit.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/kernel/exit.c b/kernel/exit.c
index 04029e3..9d2716b 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -767,6 +767,17 @@ void __noreturn do_exit(long code)
validate_creds_for_do_exit(tsk);
/*
+ * If global init has exited,
+ * panic immediately to get a useable coredump.
+ */
+ if (unlikely(is_global_init(tsk) &&
+ (thread_group_empty(tsk) ||
+ (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.
*/
@@ -786,14 +797,6 @@ 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);
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH V2] exit: trigger panic when global init has exited
2021-03-12 3:24 [PATCH V2] exit: trigger panic when global init has exited Qianli Zhao
@ 2021-03-12 16:23 ` Oleg Nesterov
2021-03-12 18:23 ` Eric W. Biederman
1 sibling, 0 replies; 4+ messages in thread
From: Oleg Nesterov @ 2021-03-12 16:23 UTC (permalink / raw)
To: Qianli Zhao
Cc: christian, axboe, ebiederm, tglx, pcc, linux-kernel, zhaoqianli
On 03/12, Qianli Zhao wrote:
>
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -767,6 +767,17 @@ void __noreturn do_exit(long code)
> validate_creds_for_do_exit(tsk);
>
> /*
> + * If global init has exited,
> + * panic immediately to get a useable coredump.
> + */
> + if (unlikely(is_global_init(tsk) &&
> + (thread_group_empty(tsk) ||
> + (tsk->signal->flags & SIGNAL_GROUP_EXIT)))) {
> + panic("Attempted to kill init! exitcode=0x%08x\n",
> + tsk->signal->group_exit_code ?: (int)code);
See our discussion with Eric, this is not right.
https://lore.kernel.org/lkml/20210310173236.GB8973@redhat.com/
Oleg.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH V2] exit: trigger panic when global init has exited
2021-03-12 3:24 [PATCH V2] exit: trigger panic when global init has exited Qianli Zhao
2021-03-12 16:23 ` Oleg Nesterov
@ 2021-03-12 18:23 ` Eric W. Biederman
2021-03-13 13:12 ` qianli zhao
1 sibling, 1 reply; 4+ messages in thread
From: Eric W. Biederman @ 2021-03-12 18:23 UTC (permalink / raw)
To: Qianli Zhao; +Cc: christian, axboe, oleg, tglx, pcc, linux-kernel, zhaoqianli
Qianli Zhao <zhaoqianligood@gmail.com> writes:
> From: Qianli Zhao <zhaoqianli@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 <zhaoqianli@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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH V2] exit: trigger panic when global init has exited
2021-03-12 18:23 ` Eric W. Biederman
@ 2021-03-13 13:12 ` qianli zhao
0 siblings, 0 replies; 4+ messages in thread
From: qianli zhao @ 2021-03-13 13:12 UTC (permalink / raw)
To: Eric W. Biederman
Cc: christian, axboe, Oleg Nesterov, Thomas Gleixner,
Peter Collingbourne, linux-kernel, Qianli Zhao
Hi Eric,Oleg
> 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);
>
Sorry,i missed this discussion,we needs use group_dead to replace
thread_group_empty().
> 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");
>
My patch moves panic to before setting PF_EXITING,it can avoid this case.
What if we move atomic_dec_and_test() to before exit_signals()?
Such as:
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);
+ }
...
exit_signals(tsk); /* sets PF_EXITING */
...
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);
Thanks
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-13 13:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-12 3:24 [PATCH V2] exit: trigger panic when global init has exited Qianli Zhao
2021-03-12 16:23 ` Oleg Nesterov
2021-03-12 18:23 ` Eric W. Biederman
2021-03-13 13:12 ` qianli zhao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.