On 2/12/19 11:08 AM, Petr Mladek wrote:
On Tue 2019-02-12 10:02:19, Joe Lawrence wrote:
On 2/12/19 6:39 AM, Petr Mladek wrote:
On Mon 2019-02-11 11:05:17, Joe Lawrence wrote:
[ ... snip ... ]
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index cc120b5018e1..50bf39ec2a9d 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -380,7 +380,7 @@ static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr,
*/
if (patch == klp_transition_patch)
klp_reverse_transition();
- else if (!enabled)
+ else if (!enabled && !patch->sticky)
ret = __klp_disable_patch(patch);
Hmm, I could imagine that even the revert would not be safe.
A problematic operations might be done already by pre-patch
callbacks or by already patched threads.
This part of the policy was an incidental side effect of where I decided to
place the final update. I would agree that reversing the transition may not
be safe... however, I was afraid of nailing down a livepatch that wasn't
disable-able but never finishes its transition. Wouldn't we be dead in the
water with respect to the state machine? ie, how to escape that condition?
I see the point.
[ ... snip ... ]
If the transition is safe then the problematic
operation need to be done by a callback anyway.
Not sure I follow. Are you saying that if the transition can be safely
reversed, problematic operations will be handled by unpatch callback(s)?
I mean something else. If a migrated thread starts doing anything that
could not get simply reverted then klp_reverse_transition() is not
safe.
Agreed.
If it is safe to revert transition before klp_transition_patch
is cleared in klp_complete_transition() then the hard-to-revert
operation must be done by some other patch-specific operations.
And the only patch-specific operation called from
klp_complete_transition() is the post-patch callback.
Ah I see.
By other words, there are two situations:
+ transition is problematic => the patch is sticky once
the transition starts and revert must be disabled
+ post_patch callback is problematic => sticky bit could
get set later by the post_patch callback.
Does this make any sense?
Yes, I got it now. That is an interesting distinction (pre-patch
callback may be safe, but post-patch callback not).
If it wasn't for the stuck-in-transition corner case I mentioned at the
top, I would say we shouldn't allow any transition reversal for these
patches. But admittedly, I haven't thought through all the cases. My
gut says that dangerous pre-patch callbacks are less likely than
post-patch ones. What do you think?
-- Joe
