lkml.org 
[lkml]   [2019]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH 4/4] livepatch: Remove the redundant enabled flag in struct klp_patch
Date
Livepatches can not longer get enabled and disabled repeatedly.
The list klp_patches contains only enabled patches and eventually
the patch in transition. As a result, the enabled flag in
struct klp_patch provides redundant information and can get
removed.

The flag is replaced by helper function klp_patch_enabled().
It simplifies the code. Also it helps to understand the semantic,
especially for the patch in transition.

Alternative solution was to remove klp_target_state. But this
would be unfortunate. The three state variable helps to
catch bugs and regressions. Also it makes it easier to get
the state a lockless way in klp_update_patch_state().

Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/livepatch.h | 2 --
kernel/livepatch/core.c | 23 +++++++++++++++--------
kernel/livepatch/transition.c | 7 +++----
kernel/livepatch/transition.h | 1 +
4 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 53551f470722..fa68192e6bb2 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -155,7 +155,6 @@ struct klp_object {
* @kobj: kobject for sysfs resources
* @obj_list: dynamic list of the object entries
* @kobj_added: @kobj has been added and needs freeing
- * @enabled: the patch is enabled (but operation may be incomplete)
* @forced: was involved in a forced transition
* @free_work: patch cleanup from workqueue-context
* @finish: for waiting till it is safe to remove the patch module
@@ -171,7 +170,6 @@ struct klp_patch {
struct kobject kobj;
struct list_head obj_list;
bool kobj_added;
- bool enabled;
bool forced;
struct work_struct free_work;
struct completion finish;
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 684766d306ad..8e644837e668 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -59,6 +59,17 @@ static bool klp_is_module(struct klp_object *obj)
return obj->name;
}

+static bool klp_patch_enabled(struct klp_patch *patch)
+{
+ if (patch == klp_transition_patch) {
+ WARN_ON_ONCE(klp_target_state == KLP_UNDEFINED);
+
+ return klp_target_state == KLP_PATCHED;
+ }
+
+ return !list_empty(&patch->list);
+}
+
/* sets obj->mod if object is not vmlinux and module is found */
static void klp_find_object_module(struct klp_object *obj)
{
@@ -335,7 +346,7 @@ static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr,

mutex_lock(&klp_mutex);

- if (patch->enabled == enabled) {
+ if (klp_patch_enabled(patch) == enabled) {
/* already in requested state */
ret = -EINVAL;
goto out;
@@ -369,7 +380,7 @@ static ssize_t enabled_show(struct kobject *kobj,
struct klp_patch *patch;

patch = container_of(kobj, struct klp_patch, kobj);
- return snprintf(buf, PAGE_SIZE-1, "%d\n", patch->enabled);
+ return snprintf(buf, PAGE_SIZE-1, "%d\n", klp_patch_enabled(patch));
}

static ssize_t transition_show(struct kobject *kobj,
@@ -862,7 +873,6 @@ static int klp_init_patch_early(struct klp_patch *patch)
INIT_LIST_HEAD(&patch->list);
INIT_LIST_HEAD(&patch->obj_list);
patch->kobj_added = false;
- patch->enabled = false;
patch->forced = false;
INIT_WORK(&patch->free_work, klp_free_patch_work_fn);
init_completion(&patch->finish);
@@ -919,7 +929,7 @@ static int __klp_disable_patch(struct klp_patch *patch)
{
struct klp_object *obj;

- if (WARN_ON(!patch->enabled))
+ if (WARN_ON(!klp_patch_enabled(patch)))
return -EINVAL;

if (klp_transition_patch)
@@ -941,7 +951,6 @@ static int __klp_disable_patch(struct klp_patch *patch)
smp_wmb();

klp_start_transition();
- patch->enabled = false;
klp_try_complete_transition();

return 0;
@@ -955,7 +964,7 @@ static int __klp_enable_patch(struct klp_patch *patch)
if (klp_transition_patch)
return -EBUSY;

- if (WARN_ON(patch->enabled))
+ if (list_empty(&patch->list))
return -EINVAL;

if (!patch->kobj_added)
@@ -994,7 +1003,6 @@ static int __klp_enable_patch(struct klp_patch *patch)
}

klp_start_transition();
- patch->enabled = true;
klp_try_complete_transition();

return 0;
@@ -1093,7 +1101,6 @@ void klp_discard_replaced_patches(struct klp_patch *new_patch)
if (old_patch == new_patch)
return;

- old_patch->enabled = false;
klp_unpatch_objects(old_patch);
klp_free_patch_start(old_patch);
schedule_work(&old_patch->free_work);
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index a3a6f32c6fd0..a40b58660640 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -31,7 +31,7 @@

struct klp_patch *klp_transition_patch;

-static int klp_target_state = KLP_UNDEFINED;
+int klp_target_state = KLP_UNDEFINED;

/*
* This work can be performed periodically to finish patching or unpatching any
@@ -354,6 +354,7 @@ static bool klp_try_switch_task(struct task_struct *task)
void klp_try_complete_transition(void)
{
unsigned int cpu;
+ int target_state = klp_target_state;
struct task_struct *g, *task;
struct klp_patch *patch;
bool complete = true;
@@ -412,7 +413,7 @@ void klp_try_complete_transition(void)
* klp_complete_transition() but it is called also
* from klp_cancel_transition().
*/
- if (!patch->enabled) {
+ if (target_state == KLP_UNPATCHED) {
klp_free_patch_start(patch);
schedule_work(&patch->free_work);
}
@@ -545,8 +546,6 @@ void klp_reverse_transition(void)
klp_target_state == KLP_PATCHED ? "patching to unpatching" :
"unpatching to patching");

- klp_transition_patch->enabled = !klp_transition_patch->enabled;
-
klp_target_state = !klp_target_state;

/*
diff --git a/kernel/livepatch/transition.h b/kernel/livepatch/transition.h
index f9d0bc016067..b9f3e96d8c13 100644
--- a/kernel/livepatch/transition.h
+++ b/kernel/livepatch/transition.h
@@ -5,6 +5,7 @@
#include <linux/livepatch.h>

extern struct klp_patch *klp_transition_patch;
+extern int klp_target_state;

void klp_init_transition(struct klp_patch *patch, int state);
void klp_cancel_transition(void);
--
2.13.7
\
 
 \ /
  Last update: 2019-01-16 17:18    [W:0.137 / U:3.360 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site