Re: [PATCH] sysctl: Add panic-fatal-signals

[Kees Cook]

On Wed, Jan 16, 2019 at 4:54 AM Vincent Whitchurch
<vincent.whitchurch@xxxxxxxx> wrote:
>
> Add a sysctl which asks the kernel to panic when any userspace process
> receives a fatal signal which would trigger a core dump.  This has
> proven to be quite useful when debugging problems seen during testing of
> embedded systems:  When combined with kernel core dumps (saved due to
> the panic), it allows easier debugging of crashes which have their
> origin in system-wide problems such as buggy drivers or other kernel or
> hardware-related issues.
>
> The crashing process's core dump can be extracted from the kernel core
> dump using tools such as the crash utility's gcore extension.
>
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@xxxxxxxx>

Whoa. That's intense. :) If you have a use-case for it, then I guess I
can't complain. Given that this is an instant DoS for a system that
isn't expecting it, I do wonder if it should live behind a CONFIG or
something to make it available only when really wanted?

-Kees

> ---
>  include/linux/signal.h | 1 +
>  kernel/signal.c        | 5 ++++-
>  kernel/sysctl.c        | 7 +++++++
>  3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/signal.h b/include/linux/signal.h
> index cc7e2c1cd444..109efd1432e9 100644
> --- a/include/linux/signal.h
> +++ b/include/linux/signal.h
> @@ -10,6 +10,7 @@ struct task_struct;
>
>  /* for sysctl */
>  extern int print_fatal_signals;
> +extern int panic_fatal_signals;
>
>  static inline void copy_siginfo(kernel_siginfo_t *to,
>                                 const kernel_siginfo_t *from)
> diff --git a/kernel/signal.c b/kernel/signal.c
> index e1d7ad8e6ab1..83c6877b0182 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -59,6 +59,7 @@
>  static struct kmem_cache *sigqueue_cachep;
>
>  int print_fatal_signals __read_mostly;
> +int panic_fatal_signals __read_mostly;
>
>  static void __user *sig_handler(struct task_struct *t, int sig)
>  {
> @@ -2497,8 +2498,10 @@ bool get_signal(struct ksignal *ksig)
>                 current->flags |= PF_SIGNALED;
>
>                 if (sig_kernel_coredump(signr)) {
> -                       if (print_fatal_signals)
> +                       if (print_fatal_signals || panic_fatal_signals)
>                                 print_fatal_signal(ksig->info.si_signo);
> +                       if (panic_fatal_signals)
> +                               panic("Fatal signal and panic_fatal_signals=1");
>                         proc_coredump_connector(current);
>                         /*
>                          * If it was able to dump core, this kills all
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index ba4d9e85feb8..ad1ac6ec2004 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -557,6 +557,13 @@ static struct ctl_table kern_table[] = {
>                 .mode           = 0644,
>                 .proc_handler   = proc_dointvec,
>         },
> +       {
> +               .procname       = "panic-fatal-signals",
> +               .data           = &panic_fatal_signals,
> +               .maxlen         = sizeof(int),
> +               .mode           = 0644,
> +               .proc_handler   = proc_dointvec,
> +       },
>  #ifdef CONFIG_SPARC
>         {
>                 .procname       = "reboot-cmd",
> --
> 2.20.0
>


-- 
Kees Cook