linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Josh Poimboeuf <jpoimboe@redhat.com>,
	Jessica Yu <jeyu@redhat.com>, Jiri Kosina <jikos@kernel.org>,
	Miroslav Benes <mbenes@suse.cz>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org,
	Petr Mladek <pmladek@suse.com>
Subject: [PATCH 2/3] livepatch/rcu: Warn when system consistency is broken in RCU code
Date: Thu,  4 May 2017 12:55:15 +0200	[thread overview]
Message-ID: <1493895316-19165-3-git-send-email-pmladek@suse.com> (raw)
In-Reply-To: <1493895316-19165-1-git-send-email-pmladek@suse.com>

RCU is not watching inside some RCU code. As a result, the livepatch
ftrace handler might see ops->func_stack and some other flags in
a wrong state. Then a livepatch might make the system unstable.

Note that there might be serious consequences only when the livepatch
modifies semantic of functions used by some parts of RCU infrastructure.
We are safe when none of the patched functions is used by this RCU code.
Also everything is good when the functions might be changed one by one
(using the immediate flag). See Documentation/livepatch/livepatch.txt
for more details about the consistency model.

Fortunately, the sensitive parts of the RCU code are annotated
and could be detected. This patch adds a warning when an insecure
usage is detected.

Unfortunately, the ftrace handler might be called when the problematic
patch has already been removed from ops->func stack. In this case,
it is not able to read the immediate flag. It makes the check
unreliable. We rather avoid it and report the problem even when
the system stability is not affected.

It would be possible to add some more complex logic to avoid
warnings when RCU infrastructure is modified using immediate
patches. But let's keep it simple until real life experience
forces us to do the opposite.

This patch is inspired by similar problems solved in stack
tracer, see
https://lkml.kernel.org/r/20170412115304.3077dbc8@gandalf.local.home

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 Documentation/livepatch/livepatch.txt | 15 +++++++++++++++
 kernel/livepatch/patch.c              |  8 ++++++++
 2 files changed, 23 insertions(+)

diff --git a/Documentation/livepatch/livepatch.txt b/Documentation/livepatch/livepatch.txt
index ecdb18104ab0..92619f7d86fd 100644
--- a/Documentation/livepatch/livepatch.txt
+++ b/Documentation/livepatch/livepatch.txt
@@ -476,6 +476,21 @@ The current Livepatch implementation has several limitations:
     by "notrace".
 
 
+  + Limited patching of RCU infrastructure
+
+    The livepatch ftrace handler uses RCU for handling the stack of patches.
+    Also the ftrace framework uses RCU to detect when the handlers are not
+    longer in use.
+
+    The directly used RCU functions could not be patched. Otherwise,
+    there would be an infinite recursion.
+
+    Some other RCU functions can not be patched safely because the RCU
+    framework might be in an inconsistent state. This context is annotated
+    and warning is printed. In theory, it might be safe to modify such
+    functions using immediate patches. But this is hard to detect properly
+    in the ftrace handler, so the warning is always printed.
+
 
   + Livepatch works reliably only when the dynamic ftrace is located at
     the very beginning of the function.
diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
index 4c4fbe409008..ffdf5fa8005b 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -62,6 +62,14 @@ static void notrace klp_ftrace_handler(unsigned long ip,
 	/* RCU may not be watching, make it see us. */
 	rcu_irq_enter_irqson();
 
+	/*
+	 * RCU still might not see us if we patch a function inside
+	 * the RCU infrastructure. Then we might see wrong state of
+	 * func->stack and other flags.
+	 */
+	if (unlikely(!rcu_is_watching()))
+		WARN_ONCE(1, "Livepatch modified a function that can not be handled a safe way.!");
+
 	rcu_read_lock();
 
 	func = list_first_or_null_rcu(&ops->func_stack, struct klp_func,
-- 
1.8.5.6

  parent reply	other threads:[~2017-05-04 10:56 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-04 10:55 [PATCH 0/3] livepatch/rcu: Handle some subtle issues between livepatching and RCU Petr Mladek
2017-05-04 10:55 ` [PATCH 1/3] livepatch/rcu: Guarantee consistency when patching idle kthreads Petr Mladek
2017-05-04 10:55 ` Petr Mladek [this message]
2017-05-08 16:51   ` [PATCH 2/3] livepatch/rcu: Warn when system consistency is broken in RCU code Josh Poimboeuf
2017-05-08 19:13     ` Steven Rostedt
2017-05-08 19:47       ` Josh Poimboeuf
2017-05-08 20:15         ` Paul E. McKenney
2017-05-08 20:43           ` Josh Poimboeuf
2017-05-08 20:51             ` Josh Poimboeuf
2017-05-08 21:08               ` Paul E. McKenney
2017-05-08 21:07             ` Paul E. McKenney
2017-05-08 21:18               ` Steven Rostedt
2017-05-08 21:30                 ` Paul E. McKenney
2017-05-08 22:16               ` Josh Poimboeuf
2017-05-08 22:36                 ` Paul E. McKenney
2017-05-09 16:18                   ` Josh Poimboeuf
2017-05-09 16:36                     ` Paul E. McKenney
2017-05-10 16:04                     ` Petr Mladek
2017-05-10 16:45                       ` Paul E. McKenney
2017-05-10 17:58                       ` Josh Poimboeuf
2017-05-11 12:40                         ` Miroslav Benes
2017-05-11 15:03                           ` Josh Poimboeuf
2017-05-08 21:16             ` Steven Rostedt
2017-05-08 20:18         ` Steven Rostedt
2017-05-11 12:50           ` Miroslav Benes
2017-05-11 13:52       ` Petr Mladek
2017-05-11 14:50         ` Paul E. McKenney
2017-05-11 15:27         ` Josh Poimboeuf
2017-05-11 12:44     ` Petr Mladek
2017-05-04 10:55 ` [PATCH 3/3] livepatch/rcu: Disable livepatch removal when safety is not guaranteed Petr Mladek
2017-05-04 16:55 ` [PATCH 0/3] livepatch/rcu: Handle some subtle issues between livepatching and RCU Paul E. McKenney

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=1493895316-19165-3-git-send-email-pmladek@suse.com \
    --to=pmladek@suse.com \
    --cc=jeyu@redhat.com \
    --cc=jikos@kernel.org \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=rostedt@goodmis.org \
    /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).