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

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

[Vincent Whitchurch]

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@axis.com>
---
v2: Put the sysctl behind a config option

 include/linux/signal.h |  1 +
 init/Kconfig           | 14 ++++++++++++++
 kernel/signal.c        |  5 ++++-
 kernel/sysctl.c        |  9 +++++++++
 4 files changed, 28 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/init/Kconfig b/init/Kconfig
index d47cb77a220e..875442f6ab53 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1242,6 +1242,20 @@ config SYSCTL_SYSCALL
 
 	  If unsure say N here.
 
+config SYSCTL_PANIC_FATAL_SIGNALS
+	bool "panic-fatal-signals sysctl" if EXPERT
+	depends on PROC_SYSCTL
+	help
+	  If you say Y here, a kernel.panic-fatal-signals sysctl will be
+	  offered.  If this sysctl is turned on, the kernel will panic if any
+	  userspace process receives a fatal signal which would trigger a core
+	  dump.
+
+	  When used together with kernel core dumps, this can be useful for
+	  debugging some system-wide problems, primarily on embedded systems.
+
+	  If unsure, say N.
+
 config FHANDLE
 	bool "open by fhandle syscalls" if EXPERT
 	select EXPORTFS
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..48023bad5b08 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -557,6 +557,15 @@ static struct ctl_table kern_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
 	},
+#ifdef CONFIG_SYSCTL_PANIC_FATAL_SIGNALS
+	{
+		.procname	= "panic-fatal-signals",
+		.data		= &panic_fatal_signals,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+	},
+#endif
 #ifdef CONFIG_SPARC
 	{
 		.procname	= "reboot-cmd",
-- 
2.20.0

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

[Andrew Morton]

On Mon, 28 Jan 2019 09:49:59 +0100 Vincent Whitchurch <vincent.whitchurch@axis.com> 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.
> 

I can't speak to the usefulness of this, but the feature is small and
simple.

Some documentation would be appreciated.  I assume in
Documentation/sysctl/kernel.txt.  Please also check that
print-fatal-signals is appropriately documented while we're in there -
it's documented in Documentation/admin-guide/kernel-parameters.rst but
not Documentation/sysctl/kernel.txt.

> v2: Put the sysctl behind a config option

I suppose so...  The option is root-only (surely?) so I'm not sure this
is really needed.

> ...
>
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1242,6 +1242,20 @@ config SYSCTL_SYSCALL
>  
>  	  If unsure say N here.
>  
> +config SYSCTL_PANIC_FATAL_SIGNALS
> +	bool "panic-fatal-signals sysctl" if EXPERT
> +	depends on PROC_SYSCTL
> +	help
> +	  If you say Y here, a kernel.panic-fatal-signals sysctl will be
> +	  offered.  If this sysctl is turned on, the kernel will panic if any
> +	  userspace process receives a fatal signal which would trigger a core
> +	  dump.
> +
> +	  When used together with kernel core dumps, this can be useful for
> +	  debugging some system-wide problems, primarily on embedded systems.
> +
> +	  If unsure, say N.

I suggest that the Kconfig help and the forthcoming documentation
should clearly explain the dangers of enabling this!

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

[Luis Chamberlain]

On Mon, Jan 28, 2019 at 10:05:45AM -0800, Andrew Morton wrote:
> On Mon, 28 Jan 2019 09:49:59 +0100 Vincent Whitchurch <vincent.whitchurch@axis.com> wrote:
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -1242,6 +1242,20 @@ config SYSCTL_SYSCALL
> >  
> >  	  If unsure say N here.
> >  
> > +config SYSCTL_PANIC_FATAL_SIGNALS
> > +	bool "panic-fatal-signals sysctl" if EXPERT
> > +	depends on PROC_SYSCTL
> > +	help
> > +	  If you say Y here, a kernel.panic-fatal-signals sysctl will be
> > +	  offered.  If this sysctl is turned on, the kernel will panic if any
> > +	  userspace process receives a fatal signal which would trigger a core
> > +	  dump.
> > +
> > +	  When used together with kernel core dumps, this can be useful for
> > +	  debugging some system-wide problems, primarily on embedded systems.
> > +
> > +	  If unsure, say N.
> 
> I suggest that the Kconfig help and the forthcoming documentation
> should clearly explain the dangers of enabling this!

And so, should default to n.

  Luis

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

[Richard Weinberger]

On Wed, Jan 30, 2019 at 3:49 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> On Mon, Jan 28, 2019 at 10:05:45AM -0800, Andrew Morton wrote:
> > On Mon, 28 Jan 2019 09:49:59 +0100 Vincent Whitchurch <vincent.whitchurch@axis.com> wrote:
> > > --- a/init/Kconfig
> > > +++ b/init/Kconfig
> > > @@ -1242,6 +1242,20 @@ config SYSCTL_SYSCALL
> > >
> > >       If unsure say N here.
> > >
> > > +config SYSCTL_PANIC_FATAL_SIGNALS
> > > +   bool "panic-fatal-signals sysctl" if EXPERT
> > > +   depends on PROC_SYSCTL
> > > +   help
> > > +     If you say Y here, a kernel.panic-fatal-signals sysctl will be
> > > +     offered.  If this sysctl is turned on, the kernel will panic if any
> > > +     userspace process receives a fatal signal which would trigger a core
> > > +     dump.
> > > +
> > > +     When used together with kernel core dumps, this can be useful for
> > > +     debugging some system-wide problems, primarily on embedded systems.
> > > +
> > > +     If unsure, say N.
> >
> > I suggest that the Kconfig help and the forthcoming documentation
> > should clearly explain the dangers of enabling this!
>
> And so, should default to n.
>
>   Luis

Hmm, why can't this be a core dump helper?
Such as:

#!/bin/sh
# Usage:
# echo "|/root/crash.sh %p %e %s" > /proc/sys/kernel/core_pattern

PID="$1"
COMM="$2"
SIGNAL="$3"

echo "$COMM ($PID) died by signal $SIGNAL, killing kernel" > /dev/kmsg
echo c > /proc/sysrq-trigger

-- 
Thanks,
//richard

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

[Vincent Whitchurch]

On Wed, Jan 30, 2019 at 04:02:36PM +0100, Richard Weinberger wrote:
> Hmm, why can't this be a core dump helper?
> Such as:
> 
> #!/bin/sh
> # Usage:
> # echo "|/root/crash.sh %p %e %s" > /proc/sys/kernel/core_pattern
> 
> PID="$1"
> COMM="$2"
> SIGNAL="$3"
> 
> echo "$COMM ($PID) died by signal $SIGNAL, killing kernel" > /dev/kmsg
> echo c > /proc/sysrq-trigger

Yes, that looks like it should work.  Thanks.