linux-kernel - Re: [PATCH V2] exit: trigger panic when global init has exited

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAPx_LQGkDE5vqwB4h7-k=AfJ5TJKHae_=B47OnfDXZSuUxjkbQ@mail.gmail.com>
Date:   Sat, 13 Mar 2021 21:12:35 +0800
From:   qianli zhao <zhaoqianligood@...il.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     christian@...uner.io, axboe@...nel.dk,
        Oleg Nesterov <oleg@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Collingbourne <pcc@...gle.com>,
        linux-kernel@...r.kernel.org, Qianli Zhao <zhaoqianli@...omi.com>
Subject: Re: [PATCH V2] exit: trigger panic when global init has exited

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