linux-kernel - Re: [PATCH V3] 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] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 18 Mar 2021 10:47:02 +0800
From:   qianli zhao <zhaoqianligood@...il.com>
To:     Oleg Nesterov <oleg@...hat.com>
Cc:     christian@...uner.io, axboe@...nel.dk,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Collingbourne <pcc@...gle.com>,
        linux-kernel@...r.kernel.org, Qianli Zhao <zhaoqianli@...omi.com>
Subject: Re: [PATCH V3] exit: trigger panic when global init has exited

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 <oleg@...hat.com> 于2021年3月17日周三 下午10:38写道：
>
> On 03/17, Qianli Zhao wrote:
> >
> > From: Qianli Zhao <zhaoqianli@...omi.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 <zhaoqianli@...omi.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
> >
>