From: Vladimir Kondratiev <vladimir.kondratiev@linux.intel.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Jonathan Corbet <corbet@lwn.net>,
Luis Chamberlain <mcgrof@kernel.org>,
Kees Cook <keescook@chromium.org>,
Iurii Zaikin <yzaikin@google.com>,
"Paul E. McKenney" <paulmck@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Randy Dunlap <rdunlap@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
Mike Kravetz <mike.kravetz@oracle.com>,
"Guilherme G. Piccoli" <gpiccoli@canonical.com>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Kars Mulder <kerneldev@karsmulder.nl>,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Kishon Vijay Abraham I <kishon@ti.com>,
Arvind Sankar <nivedita@alum.mit.edu>,
Joe Perches <joe@perches.com>, Rafael Aquini <aquini@redhat.com>,
Christian Brauner <christian.brauner@ubuntu.com>,
Alexei Starovoitov <ast@kernel.org>,
"Peter Zijlstra (Intel)" <peterz@infradead.org>,
Davidlohr Bueso <dave@stgolabs.net>,
Michel Lespinasse <walken@google.com>,
Jann Horn <jannh@google.com>, chenqiwu <chenqiwu@xiaomi.com>,
Minchan Kim <minchan@kernel.org>,
Christophe Leroy <christophe.leroy@c-s.fr>,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fsdevel@vger.kernel.org
Subject: Re: [RFC PATCH v2] do_exit(): panic() recursion detected
Date: Mon, 7 Dec 2020 18:19:56 +0200 [thread overview]
Message-ID: <f6f1208a-12c4-77b8-2e1d-fb4a03a2211a@linux.intel.com> (raw)
In-Reply-To: <87pn3ly5u3.fsf@x220.int.ebiederm.org>
I see 2 paths how "bad things" can cause recursive do_exit - various
traps that go through die() and therefore covered by panic_on_oops; and
do_group_exit() as result of fatal signal.
Provided one add "panic on coredump" functionality, path through
do_group_exit() covered as well.
Let's drop this patch.
Thanks, Vladimir
On 12/7/20 5:49 PM, Eric W. Biederman wrote:
> Vladimir Kondratiev <vladimir.kondratiev@linux.intel.com> writes:
>
>> Please ignore version 1 of the patch - it was sent from wrong mail address.
>>
>> To clarify the reason:
>>
>> Situation where do_exit() re-entered, discovered by static code analysis.
>> For safety critical system, it is better to panic() when minimal chance of
>> corruption detected. For this reason, we also panic on fatal signal delivery -
>> patch for this not submitted yet.
>
> What did the static code analysis say? What triggers the recursion.
>
> What makes it safe to even call panic on this code path? Is there
> enough kernel stack?
>
> My sense is that if this actually can happen and is a real concern,
> and that it is safe to do something on this code path it is probably
> better just to ooops. That way if someone is trying to debug such
> a recursion they will have a backtrace to work with. Plus panic
> on oops will work.
>
> Eric
>
>>
>> On 12/7/20 2:44 PM, 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.
>>>
>>> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@linux.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,
>>>
prev parent reply other threads:[~2020-12-07 16:22 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-07 12:44 [RFC PATCH v2] do_exit(): panic() recursion detected Vladimir Kondratiev
2020-12-07 12:52 ` Vladimir Kondratiev
2020-12-07 15:49 ` Eric W. Biederman
2020-12-07 16:19 ` Vladimir Kondratiev [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f6f1208a-12c4-77b8-2e1d-fb4a03a2211a@linux.intel.com \
--to=vladimir.kondratiev@linux.intel.com \
--cc=akpm@linux-foundation.org \
--cc=andriy.shevchenko@linux.intel.com \
--cc=aquini@redhat.com \
--cc=ast@kernel.org \
--cc=chenqiwu@xiaomi.com \
--cc=christian.brauner@ubuntu.com \
--cc=christophe.leroy@c-s.fr \
--cc=corbet@lwn.net \
--cc=dave@stgolabs.net \
--cc=ebiederm@xmission.com \
--cc=gpiccoli@canonical.com \
--cc=jannh@google.com \
--cc=joe@perches.com \
--cc=keescook@chromium.org \
--cc=kerneldev@karsmulder.nl \
--cc=kishon@ti.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=mcgrof@kernel.org \
--cc=mchehab+huawei@kernel.org \
--cc=mike.kravetz@oracle.com \
--cc=minchan@kernel.org \
--cc=nivedita@alum.mit.edu \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--cc=rdunlap@infradead.org \
--cc=tglx@linutronix.de \
--cc=walken@google.com \
--cc=yzaikin@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).