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:   Mon, 20 Nov 2017 16:57:19 +0100 (CET)
From:   Miroslav Benes <mbenes@...e.cz>
To:     jpoimboe@...hat.com, jeyu@...nel.org, jikos@...nel.org
cc:     pmladek@...e.com, lpechacek@...e.cz, pavel@....cz,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/2] livepatch: force transition to finish

On Wed, 15 Nov 2017, Miroslav Benes wrote:

> If a task sleeps in a set of patched functions uninterruptedly, it could
> block the whole transition indefinitely.  Thus it may be useful to clear
> its TIF_PATCH_PENDING to allow the process to finish.
> 
> Admin can do that now by writing to force sysfs attribute in livepatch
> sysfs directory. TIF_PATCH_PENDING is then cleared for all tasks and the
> transition can finish successfully.
> 
> Important note! Administrator should not use this feature without a
> clearance from a patch distributor. It must be checked that by doing so
> the consistency model guarantees are not violated.
> 
> Signed-off-by: Miroslav Benes <mbenes@...e.cz>

While working on "immediate" removal, I realized we had the similar 
problem here with modules removal. There is no way out of the rabbit hole.

If a patch is forced, we obviously cannot say there is no task sleeping in 
the old code. This could be disastrous if such old module is then removed 
(either we disabled it and we want to rmmod it, or there is a new "atomic 
replace" patch and we want to remove the old one).

We need something like the following (at least as a starting point)

diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index 566ab210853f..df4f2bbd9731 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -33,6 +33,8 @@ struct klp_patch *klp_transition_patch;
 
 static int klp_target_state = KLP_UNDEFINED;
 
+static bool klp_forced = false;
+
 /*
  * This work can be performed periodically to finish patching or unpatching any
  * "straggler" tasks which failed to transition in the first attempt.
@@ -109,7 +111,7 @@ static void klp_complete_transition(void)
                }
        }
 
-       if (klp_target_state == KLP_UNPATCHED && !immediate_func)
+       if (klp_target_state == KLP_UNPATCHED && !klp_forced && !immediate_func)
                module_put(klp_transition_patch->mod);
 
        /* Prevent klp_ftrace_handler() from seeing KLP_UNDEFINED state */
@@ -642,4 +644,6 @@ void klp_force_transition(void)
 
        for_each_possible_cpu(cpu)
                klp_update_patch_state(idle_task(cpu));
+
+       klp_forced = true;
 }


It is still better than immediate, because it is a "ex post" action. We 
can also try to improve later. We could remember all forced tasks and 
reenable rmmod once those tasks are really migrated ("shadow migration"). 

Thoughts are really welcome here.

Miroslav

Powered by blists - more mailing lists