livepatch: Remove immediate feature

Immediate flag has been used to disable per-task consistency and patch
all tasks immediately. It could be useful if the patch doesn't change any
function or data semantics.

However, it causes problems on its own. The consistency problem is
currently broken with respect to immediate patches.

func            a
patches         1i
                2i
                3

When the patch 3 is applied, only 2i function is checked (by stack
checking facility). There might be a task sleeping in 1i though. Such
task is migrated to 3, because we do not check 1i in
klp_check_stack_func() at all.

Coming atomic replace feature would be easier to implement and more
reliable without immediate.

Thus, remove immediate feature completely and save us from the problems.

Note that force feature has the similar problem. However it is
considered as a last resort. If used, administrator should not apply any
new live patches and should plan for reboot into an updated kernel.

The architectures would now need to provide HAVE_RELIABLE_STACKTRACE to
fully support livepatch.

Signed-off-by: Miroslav Benes <mbenes@suse.cz>
Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>

1 files changed, 27 insertions, 62 deletions

diff --git a/Documentation/livepatch/livepatch.txt b/Documentation/livepatch/livepatch.txt
index 896ba8941702..1ae2de758c08 100644
--- a/Documentation/livepatch/livepatch.txt
+++ b/Documentation/livepatch/livepatch.txt
@@ -72,8 +72,7 @@ example, they add a NULL pointer or a boundary check, fix a race by adding
 a missing memory barrier, or add some locking around a critical section.
 Most of these changes are self contained and the function presents itself
 the same way to the rest of the system. In this case, the functions might
-be updated independently one by one.  (This can be done by setting the
-'immediate' flag in the klp_patch struct.)
+be updated independently one by one.
 
 But there are more complex fixes. For example, a patch might change
 ordering of locking in multiple functions at the same time. Or a patch
@@ -125,12 +124,6 @@ safe to patch tasks:
    b) Patching CPU-bound user tasks.  If the task is highly CPU-bound
       then it will get patched the next time it gets interrupted by an
       IRQ.
-   c) In the future it could be useful for applying patches for
-      architectures which don't yet have HAVE_RELIABLE_STACKTRACE.  In
-      this case you would have to signal most of the tasks on the
-      system.  However this isn't supported yet because there's
-      currently no way to patch kthreads without
-      HAVE_RELIABLE_STACKTRACE.
 
 3. For idle "swapper" tasks, since they don't ever exit the kernel, they
    instead have a klp_update_patch_state() call in the idle loop which
@@ -138,27 +131,16 @@ safe to patch tasks:
 
    (Note there's not yet such an approach for kthreads.)
 
-All the above approaches may be skipped by setting the 'immediate' flag
-in the 'klp_patch' struct, which will disable per-task consistency and
-patch all tasks immediately.  This can be useful if the patch doesn't
-change any function or data semantics.  Note that, even with this flag
-set, it's possible that some tasks may still be running with an old
-version of the function, until that function returns.
+Architectures which don't have HAVE_RELIABLE_STACKTRACE solely rely on
+the second approach. It's highly likely that some tasks may still be
+running with an old version of the function, until that function
+returns. In this case you would have to signal the tasks. This
+especially applies to kthreads. They may not be woken up and would need
+to be forced. See below for more information.
 
-There's also an 'immediate' flag in the 'klp_func' struct which allows
-you to specify that certain functions in the patch can be applied
-without per-task consistency.  This might be useful if you want to patch
-a common function like schedule(), and the function change doesn't need
-consistency but the rest of the patch does.
-
-For architectures which don't have HAVE_RELIABLE_STACKTRACE, the user
-must set patch->immediate which causes all tasks to be patched
-immediately.  This option should be used with care, only when the patch
-doesn't change any function or data semantics.
-
-In the future, architectures which don't have HAVE_RELIABLE_STACKTRACE
-may be allowed to use per-task consistency if we can come up with
-another way to patch kthreads.
+Unless we can come up with another way to patch kthreads, architectures
+without HAVE_RELIABLE_STACKTRACE are not considered fully supported by
+the kernel livepatching.
 
 The /sys/kernel/livepatch/<patch>/transition file shows whether a patch
 is in transition.  Only a single patch (the topmost patch on the stack)
@@ -197,6 +179,11 @@ modules is permanently disabled when the force feature is used. It cannot be
 guaranteed there is no task sleeping in such module. It implies unbounded
 reference count if a patch module is disabled and enabled in a loop.
 
+Moreover, the usage of force may also affect future applications of live
+patches and cause even more harm to the system. Administrator should first
+consider to simply cancel a transition (see above). If force is used, reboot
+should be planned and no more live patches applied.
+
 3.1 Adding consistency model support to new architectures
 ---------------------------------------------------------
 
@@ -234,13 +221,6 @@ few options:
    a good backup option for those architectures which don't have
    reliable stack traces yet.
 
-In the meantime, patches for such architectures can bypass the
-consistency model by setting klp_patch.immediate to true.  This option
-is perfectly fine for patches which don't change the semantics of the
-patched functions.  In practice, this is usable for ~90% of security
-fixes.  Use of this option also means the patch can't be unloaded after
-it has been disabled.
-
 
 4. Livepatch module
 ===================
@@ -296,9 +276,6 @@ into three levels:
     only for a particular object ( vmlinux or a kernel module ). Note that
     kallsyms allows for searching symbols according to the object name.
 
-    There's also an 'immediate' flag which, when set, patches the
-    function immediately, bypassing the consistency model safety checks.
-
   + struct klp_object defines an array of patched functions (struct
     klp_func) in the same object. Where the object is either vmlinux
     (NULL) or a module name.
@@ -317,9 +294,6 @@ into three levels:
     symbols are found. The only exception are symbols from objects
     (kernel modules) that have not been loaded yet.
 
-    Setting the 'immediate' flag applies the patch to all tasks
-    immediately, bypassing the consistency model safety checks.
-
     For more details on how the patch is applied on a per-task basis,
     see the "Consistency model" section.
 
@@ -334,14 +308,12 @@ section "Livepatch life-cycle" below for more details about these
 two operations.
 
 Module removal is only safe when there are no users of the underlying
-functions. The immediate consistency model is not able to detect this. The
-code just redirects the functions at the very beginning and it does not
-check if the functions are in use. In other words, it knows when the
-functions get called but it does not know when the functions return.
-Therefore it cannot be decided when the livepatch module can be safely
-removed. This is solved by a hybrid consistency model. When the system is
-transitioned to a new patch state (patched/unpatched) it is guaranteed that
-no task sleeps or runs in the old code.
+functions. This is the reason why the force feature permanently disables
+the removal. The forced tasks entered the functions but we cannot say
+that they returned back.  Therefore it cannot be decided when the
+livepatch module can be safely removed. When the system is successfully
+transitioned to a new patch state (patched/unpatched) without being
+forced it is guaranteed that no task sleeps or runs in the old code.
 
 
 5. Livepatch life-cycle
@@ -355,19 +327,12 @@ First, the patch is applied only when all patched symbols for already
 loaded objects are found. The error handling is much easier if this
 check is done before particular functions get redirected.
 
-Second, the immediate consistency model does not guarantee that anyone is not
-sleeping in the new code after the patch is reverted. This means that the new
-code needs to stay around "forever". If the code is there, one could apply it
-again. Therefore it makes sense to separate the operations that might be done
-once and those that need to be repeated when the patch is enabled (applied)
-again.
-
-Third, it might take some time until the entire system is migrated
-when a more complex consistency model is used. The patch revert might
-block the livepatch module removal for too long. Therefore it is useful
-to revert the patch using a separate operation that might be called
-explicitly. But it does not make sense to remove all information
-until the livepatch module is really removed.
+Second, it might take some time until the entire system is migrated with
+the hybrid consistency model being used. The patch revert might block
+the livepatch module removal for too long. Therefore it is useful to
+revert the patch using a separate operation that might be called
+explicitly. But it does not make sense to remove all information until
+the livepatch module is really removed.
 
 
 5.1. Registration