linux-kernel - [PATCH 3/3] livepatch/rcu: Disable livepatch removal when safety is not guaranteed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu,  4 May 2017 12:55:16 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Josh Poimboeuf <jpoimboe@...hat.com>, Jessica Yu <jeyu@...hat.com>,
        Jiri Kosina <jikos@...nel.org>, Miroslav Benes <mbenes@...e.cz>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
        Petr Mladek <pmladek@...e.com>
Subject: [PATCH 3/3] livepatch/rcu: Disable livepatch removal when safety is not guaranteed

We already warn when RCU is not watching and the system consistency
is not guaranteed.

One problem is that tasks might use incompatible implementations
of the modified functions. We print the warning and could not
do much more about it.

Second problem is that we could not reliably detect unused code
after the patch was disabled. Here we could disable removing
patches to avoid possible damage.

The solution is sub-optimal. First, it might be too late to disable
the patch removal. Second, it affects all patches even though some
of them might be safe. But the question is if a better solution is
worth the effort. Most of the affected functions could not be patched
anyway. They are used by the ftrace handler and patching would
cause an infinite recursion.

Signed-off-by: Petr Mladek <pmladek@...e.com>
---
 Documentation/livepatch/livepatch.txt | 4 ++++
 kernel/livepatch/patch.c              | 4 +++-
 kernel/livepatch/transition.c         | 7 ++++++-
 kernel/livepatch/transition.h         | 2 ++
 4 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/livepatch/livepatch.txt b/Documentation/livepatch/livepatch.txt
index 92619f7d86fd..87069ab46121 100644
--- a/Documentation/livepatch/livepatch.txt
+++ b/Documentation/livepatch/livepatch.txt
@@ -491,6 +491,10 @@ The current Livepatch implementation has several limitations:
     functions using immediate patches. But this is hard to detect properly
     in the ftrace handler, so the warning is always printed.
 
+    Also we are not able to detect when the code is not longer used in
+    the problematic context. We rather disable removing of livepatches
+    to avoid more possible damage.
+
 
   + 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 ffdf5fa8005b..c670a45d32f4 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -67,8 +67,10 @@ static void notrace klp_ftrace_handler(unsigned long ip,
 	 * the RCU infrastructure. Then we might see wrong state of
 	 * func->stack and other flags.
 	 */
-	if (unlikely(!rcu_is_watching()))
+	if (unlikely(!rcu_is_watching())) {
+		klp_block_patch_removal = true;
 		WARN_ONCE(1, "Livepatch modified a function that can not be handled a safe way.!");
+	}
 
 	rcu_read_lock();
 
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index adc0cc64aa4b..0669517ea6a8 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -31,6 +31,8 @@
 
 struct klp_patch *klp_transition_patch;
 
+bool klp_block_patch_removal;
+
 static int klp_target_state = KLP_UNDEFINED;
 
 /*
@@ -87,8 +89,11 @@ static void klp_complete_transition(void)
 		}
 	}
 
-	if (klp_target_state == KLP_UNPATCHED && !immediate_func)
+	if (klp_target_state == KLP_UNPATCHED &&
+	    !immediate_func &&
+	    !klp_block_patch_removal) {
 		module_put(klp_transition_patch->mod);
+	}
 
 	/* Prevent klp_ftrace_handler() from seeing KLP_UNDEFINED state */
 	if (klp_target_state == KLP_PATCHED)
diff --git a/kernel/livepatch/transition.h b/kernel/livepatch/transition.h
index ce09b326546c..cc20b19d8707 100644
--- a/kernel/livepatch/transition.h
+++ b/kernel/livepatch/transition.h
@@ -5,6 +5,8 @@
 
 extern struct klp_patch *klp_transition_patch;
 
+extern bool klp_block_patch_removal;
+
 void klp_init_transition(struct klp_patch *patch, int state);
 void klp_cancel_transition(void);
 void klp_start_transition(void);
-- 
1.8.5.6