[RFC PATCH] do_exit(): panic() recursion detected

[RFC PATCH] do_exit(): panic() recursion detected

[Vladimir Kondratiev]

From: Vladimir Kondratiev <vladimir.kondratiev@intel.com>

Recursive do_exit() is symptom of compromised kernel integrity.
For safety critical systems, it may be better to
panic() in this case to minimize risk.

Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
Change-Id: I42f45900a08c4282c511b05e9e6061360d07db60
---
 Documentation/admin-guide/kernel-parameters.txt | 6 ++++++
 include/linux/kernel.h                          | 1 +
 kernel/exit.c                                   | 7 +++++++
 kernel/sysctl.c                                 | 9 +++++++++
 4 files changed, 23 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 44fde25bb221..6e12a6804557 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3508,6 +3508,12 @@
 			bit 4: print ftrace buffer
 			bit 5: print all printk messages in buffer
 
+	panic_on_exit_recursion
+			panic() when do_exit() recursion detected, rather then
+			try to stay running whenever possible.
+			Useful on safety critical systems; re-entry in do_exit
+			is a symptom of compromised kernel integrity.
+
 	panic_on_taint=	Bitmask for conditionally calling panic() in add_taint()
 			Format: <hex>[,nousertaint]
 			Hexadecimal bitmask representing the set of TAINT flags
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 2f05e9128201..5afb20534cb2 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -539,6 +539,7 @@ extern int sysctl_panic_on_rcu_stall;
 extern int sysctl_panic_on_stackoverflow;
 
 extern bool crash_kexec_post_notifiers;
+extern int panic_on_exit_recursion;
 
 /*
  * panic_cpu is used for synchronizing panic() and crash_kexec() execution. It
diff --git a/kernel/exit.c b/kernel/exit.c
index 1f236ed375f8..162799a8b539 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -68,6 +68,9 @@
 #include <asm/unistd.h>
 #include <asm/mmu_context.h>
 
+int panic_on_exit_recursion __read_mostly;
+core_param(panic_on_exit_recursion, panic_on_exit_recursion, int, 0644);
+
 static void __unhash_process(struct task_struct *p, bool group_dead)
 {
 	nr_threads--;
@@ -757,6 +760,10 @@ void __noreturn do_exit(long code)
 	 */
 	if (unlikely(tsk->flags & PF_EXITING)) {
 		pr_alert("Fixing recursive fault but reboot is needed!\n");
+		if (panic_on_exit_recursion)
+			panic("Recursive do_exit() detected in %s[%d]\n",
+			      current->comm, task_pid_nr(current));
+
 		futex_exit_recursive(tsk);
 		set_current_state(TASK_UNINTERRUPTIBLE);
 		schedule();
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index afad085960b8..bb397fba2c42 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2600,6 +2600,15 @@ static struct ctl_table kern_table[] = {
 		.extra2		= &one_thousand,
 	},
 #endif
+	{
+		.procname	= "panic_on_exit_recursion",
+		.data		= &panic_on_exit_recursion,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
+		.extra2		= SYSCTL_ONE,
+	},
 	{
 		.procname	= "panic_on_warn",
 		.data		= &panic_on_warn,
-- 
2.27.0

Re: [RFC PATCH] do_exit(): panic() recursion detected

[Peter Zijlstra]

On Mon, Dec 07, 2020 at 02:40:49PM +0200, Vladimir Kondratiev wrote:
> From: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
> 
> Recursive do_exit() is symptom of compromised kernel integrity.
> For safety critical systems, it may be better to
> panic() in this case to minimize risk.

You've not answered the previously posed question on why panic_on_oops
isn't more suitable for your type of systems.

> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
> Change-Id: I42f45900a08c4282c511b05e9e6061360d07db60

This Change-ID crap doesn't belong in kernel patches.

Re: [RFC PATCH] do_exit(): panic() recursion detected

[Vladimir Kondratiev]

We do panic on oops as well. We panic on anything that may point to 
system stability issues. I can't proof this code can't be reached 
without oops, so I want to panic here as well.

On 12/7/20 2:51 PM, Peter Zijlstra wrote:
> On Mon, Dec 07, 2020 at 02:40:49PM +0200, Vladimir Kondratiev wrote:
>> From: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
>>
>> Recursive do_exit() is symptom of compromised kernel integrity.
>> For safety critical systems, it may be better to
>> panic() in this case to minimize risk.
> 
> You've not answered the previously posed question on why panic_on_oops
> isn't more suitable for your type of systems.
> 
>> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
>> Change-Id: I42f45900a08c4282c511b05e9e6061360d07db60
> 
> This Change-ID crap doesn't belong in kernel patches.
> 

Re: [NEEDS-REVIEW] [RFC PATCH] do_exit(): panic() recursion detected

[Dave Hansen]

On 12/7/20 4:40 AM, Vladimir Kondratiev wrote:
> Recursive do_exit() is symptom of compromised kernel integrity.
> For safety critical systems, it may be better to
> panic() in this case to minimize risk.

This changelog is still woefully inadequate.  It doesn't really describe
the problem which is being fixed.  Patches are generally not accepted by
batting around things like "safety-critical".

> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@intel.com>
> Change-Id: I42f45900a08c4282c511b05e9e6061360d07db60

What's this Change-Id?  Is this for some system outside of Linux?  If
so, we don't need that in kernel changelogs.