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@xxxxxxxxxxxxxxxxxx Signed-off-by: Petr Mladek <pmladek@xxxxxxxx> --- 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 -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
