Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1706846 > unrolled thread

[PATCH v2 0/1] add (un)patch callbacks

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/1] add (un)patch callbacks Joe Lawrence <joe.lawrence@redhat.com> - 2017-08-08 21:40 +0200
    [PATCH v2 1/1] livepatch: add (un)patch callbacks Joe Lawrence <joe.lawrence@redhat.com> - 2017-08-08 21:40 +0200
      Re: [PATCH v2 1/1] livepatch: add (un)patch callbacks Josh Poimboeuf <jpoimboe@redhat.com> - 2017-08-11 22:50 +0200

#1706846 — [PATCH v2 0/1] add (un)patch callbacks

Hi folks,

This is v2 of the livepatch (un)patch hook/notifier/callback/etc code.

The documentation is still a little rough, but I wanted to post up the
code for feedback before getting too far in revising it.

This implements the pre and post handlers as Josh suggested.  See
summary of related changes below...

v2:
- s/hooks/callbacks/g
- implemented pre-(un)patch and post-(un)patch callbacks
  - pre-patch and pre-unpatch callbacks run from callers of
    klp_patch_object() and klp_unpatch_object()
  - post-patch and post-unpatch callbacks run from
    klp_complete_transition() and klp_module_coming/going()
- reduce callbacks from a list to a single per-klp_object instance
- revamp the sample callback demo
- revamp documentation

Feedback appreciated as always.

Joe Lawrence (1):
  livepatch: add (un)patch callbacks

 Documentation/livepatch/callbacks.txt        |  75 +++++++++++
 include/linux/livepatch.h                    |  38 ++++++
 kernel/livepatch/core.c                      |  30 ++++-
 kernel/livepatch/patch.c                     |   5 +-
 kernel/livepatch/transition.c                |  19 ++-
 samples/livepatch/Makefile                   |   2 +
 samples/livepatch/livepatch-callbacks-demo.c | 180 +++++++++++++++++++++++++++
 samples/livepatch/livepatch-callbacks-mod.c  |  53 ++++++++
 8 files changed, 393 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/livepatch/callbacks.txt
 create mode 100644 samples/livepatch/livepatch-callbacks-demo.c
 create mode 100644 samples/livepatch/livepatch-callbacks-mod.c

-- 
1.8.3.1

[toc] | [next] | [standalone]

#1706847 — [PATCH v2 1/1] livepatch: add (un)patch callbacks

Provide livepatch modules a klp_object (un)patching notification
mechanism.  Pre and post-(un)patch callbacks allow livepatch modules to
setup or synchronize changes that would be difficult to support in only
patched-or-unpatched code contexts.

Callbacks can be registered for target module or vmlinux klp_objects,
but each implementation is klp_object specific.

  - Pre-(un)patch callbacks run before any (un)patching action takes
    place.

  - Post-(un)patch callbacks run once an object has been (un)patched and
    the klp_patch fully transitioned to its target state.

Example use cases include modification of global data and registration
of newly available services/handlers.

See Documentation/livepatch/callback.txt for details.

Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
 Documentation/livepatch/callbacks.txt        |  75 +++++++++++
 include/linux/livepatch.h                    |  38 ++++++
 kernel/livepatch/core.c                      |  32 ++++-
 kernel/livepatch/patch.c                     |   5 +-
 kernel/livepatch/transition.c                |  19 ++-
 samples/livepatch/Makefile                   |   2 +
 samples/livepatch/livepatch-callbacks-demo.c | 190 +++++++++++++++++++++++++++
 samples/livepatch/livepatch-callbacks-mod.c  |  53 ++++++++
 8 files changed, 405 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/livepatch/callbacks.txt
 create mode 100644 samples/livepatch/livepatch-callbacks-demo.c
 create mode 100644 samples/livepatch/livepatch-callbacks-mod.c

diff --git a/Documentation/livepatch/callbacks.txt b/Documentation/livepatch/callbacks.txt
new file mode 100644
index 000000000000..d78f9ba9f74c
--- /dev/null
+++ b/Documentation/livepatch/callbacks.txt
@@ -0,0 +1,75 @@
+(Un)patching Callbacks
+======================
+
+Livepatch (un)patch-callbacks provide a mechanism for livepatch modules
+to execute callback functions when a kernel object is (un)patched.
+
+These callbacks differ from existing kernel facilities:
+
+  - Module init/exit code doesn't run when disabling and re-enabling a
+    patch.
+
+  - A module notifier can't stop the to-be-patched module from loading.
+
+Callbacks are part of the klp_object structure and their implementation
+is specific to the given object.  Other livepatch objects may or may not
+be patched, irrespective of the target klp_object's current state.
+
+Callbacks can be registered for the following livepatch actions:
+
+  * Pre-patch    - before klp_object is patched
+
+  * Post-patch   - after klp_object has been patched and is active
+                   across all tasks
+
+  * Pre-unpatch  - before klp_object is unpatched, patched code is active
+
+  * Post-unpatch - after klp_object has been patched, all code has been
+		   restored and no tasks are running patched code
+
+Callbacks are only executed if its host klp_object is loaded.  For
+in-kernel vmlinux targets, this means that callbacks will always execute
+when a livepatch is enabled/disabled.
+
+For kernel module targets, callbacks will only execute if the target
+module is loaded.  When a kernel module target is (un)loaded, its
+callbacks will execute only if the livepatch module is enabled.
+
+The pre-patch callback is expected to return a status code (0 for
+success, -ERRNO on error).  An error status code will indicate to the
+livepatching core that patching of the current klp_object is not safe
+and to stop the current patching request.  If the problematic klp_object
+is already loaded (i.e. a livepatch is loaded after target code), the
+kernel's module loader will refuse to load the livepatch.  On the other
+hand, if the problematic klp_object is already in place (i.e. a target
+module is loaded after a livepatch), then the module loader will refuse
+to load the target kernel module.
+
+
+Example Use-cases
+-----------------
+
+1 - Update global data
+
+A pre-patch callback can be useful to update a global variable.  For
+example, 75ff39ccc1bd ("tcp: make challenge acks less predictable")
+changes a global sysctl, as well as patches the tcp_send_challenge_ack()
+function.
+
+In this case, if we're being super paranoid, it might make sense to
+patch the data *after* patching is complete with a post-patch callback,
+so that tcp_send_challenge_ack() could first be changed to read
+sysctl_tcp_challenge_ack_limit with READ_ONCE.
+
+
+2 - Support __init and probe function patches
+
+Although __init and probe functions are not directly livepatch-able, it
+may be possible to implement similar updates via pre/post-patch
+callbacks.
+
+48900cb6af42 ("virtio-net: drop NETIF_F_FRAGLIST") change the way that
+virtnet_probe() initialized its driver's net_device features.  A
+pre/post-patch callback could iterate over all such devices, making a
+similar change to their hw_features value.  (Client functions of the
+value may need to be updated accordingly.)
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 194991ef9347..5fb8925fc488 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -87,10 +87,25 @@ struct klp_func {
 	bool transition;
 };
 
+struct klp_object;
+
+/**
+ * struct klp_callback - callback structure for live patching
+ * @callback:	function to be executed on callback
+ *
+ */
+struct klp_callbacks {
+	int (*pre_patch)(struct klp_object *obj);
+	void (*post_patch)(struct klp_object *obj);
+	void (*pre_unpatch)(struct klp_object *obj);
+	void (*post_unpatch)(struct klp_object *obj);
+};
+
 /**
  * struct klp_object - kernel object structure for live patching
  * @name:	module name (or NULL for vmlinux)
  * @funcs:	function entries for functions to be patched in the object
+ * @callbacks:	functions to be executed pre/post (un)patching
  * @kobj:	kobject for sysfs resources
  * @mod:	kernel module associated with the patched object
  *		(NULL for vmlinux)
@@ -100,6 +115,7 @@ struct klp_object {
 	/* external */
 	const char *name;
 	struct klp_func *funcs;
+	struct klp_callbacks callbacks;
 
 	/* internal */
 	struct kobject kobj;
@@ -138,6 +154,28 @@ struct klp_patch {
 	     func->old_name || func->new_func || func->old_sympos; \
 	     func++)
 
+static inline int klp_pre_patch_callback(struct klp_object *obj)
+{
+	if (!obj->patched && obj->callbacks.pre_patch)
+		return (*obj->callbacks.pre_patch)(obj);
+	return 0;
+}
+static inline void klp_post_patch_callback(struct klp_object *obj)
+{
+	if (obj->patched && obj->callbacks.post_patch)
+		(*obj->callbacks.post_patch)(obj);
+}
+static inline void klp_pre_unpatch_callback(struct klp_object *obj)
+{
+	if (obj->patched && obj->callbacks.pre_unpatch)
+		(*obj->callbacks.pre_unpatch)(obj);
+}
+static inline void klp_post_unpatch_callback(struct klp_object *obj)
+{
+	if (!obj->patched && obj->callbacks.post_unpatch)
+		(*obj->callbacks.post_unpatch)(obj);
+}
+
 int klp_register_patch(struct klp_patch *);
 int klp_unregister_patch(struct klp_patch *);
 int klp_enable_patch(struct klp_patch *);
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index b9628e43c78f..6f205238582b 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -388,13 +388,18 @@ static int __klp_enable_patch(struct klp_patch *patch)
 		if (!klp_is_object_loaded(obj))
 			continue;
 
-		ret = klp_patch_object(obj);
+		ret = klp_pre_patch_callback(obj);
 		if (ret) {
-			pr_warn("failed to enable patch '%s'\n",
-				patch->mod->name);
+			pr_warn("pre-patch callback failed for object '%s'\n",
+				klp_is_module(obj) ? obj->name : "vmlinux");
+			goto err;
+		}
 
-			klp_cancel_transition();
-			return ret;
+		ret = klp_patch_object(obj);
+		if (ret) {
+			pr_warn("failed to patch object '%s'\n",
+				klp_is_module(obj) ? obj->name : "vmlinux");
+			goto err;
 		}
 	}
 
@@ -403,6 +408,11 @@ static int __klp_enable_patch(struct klp_patch *patch)
 	patch->enabled = true;
 
 	return 0;
+err:
+	pr_warn("failed to enable patch '%s'\n", patch->mod->name);
+
+	klp_cancel_transition();
+	return ret;
 }
 
 /**
@@ -871,6 +881,13 @@ int klp_module_coming(struct module *mod)
 			pr_notice("applying patch '%s' to loading module '%s'\n",
 				  patch->mod->name, obj->mod->name);
 
+			ret = klp_pre_patch_callback(obj);
+			if (ret) {
+				pr_warn("pre-patch callback failed for object '%s'\n",
+					obj->name);
+				goto err;
+			}
+
 			ret = klp_patch_object(obj);
 			if (ret) {
 				pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
@@ -878,6 +895,8 @@ int klp_module_coming(struct module *mod)
 				goto err;
 			}
 
+			klp_post_patch_callback(obj);
+
 			break;
 		}
 	}
@@ -929,7 +948,10 @@ void klp_module_going(struct module *mod)
 			if (patch->enabled || patch == klp_transition_patch) {
 				pr_notice("reverting patch '%s' on unloading module '%s'\n",
 					  patch->mod->name, obj->mod->name);
+
+				klp_pre_unpatch_callback(obj);
 				klp_unpatch_object(obj);
+				klp_post_unpatch_callback(obj);
 			}
 
 			klp_free_object_loaded(obj);
diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
index 52c4e907c14b..0eed0df6e6d9 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -257,6 +257,7 @@ int klp_patch_object(struct klp_object *obj)
 	klp_for_each_func(obj, func) {
 		ret = klp_patch_func(func);
 		if (ret) {
+			klp_pre_unpatch_callback(obj);
 			klp_unpatch_object(obj);
 			return ret;
 		}
@@ -271,6 +272,8 @@ void klp_unpatch_objects(struct klp_patch *patch)
 	struct klp_object *obj;
 
 	klp_for_each_object(patch, obj)
-		if (obj->patched)
+		if (obj->patched) {
+			klp_pre_unpatch_callback(obj);
 			klp_unpatch_object(obj);
+		}
 }
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index b004a1fb6032..fb2a986e66fc 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -109,9 +109,6 @@ static void klp_complete_transition(void)
 		}
 	}
 
-	if (klp_target_state == KLP_UNPATCHED && !immediate_func)
-		module_put(klp_transition_patch->mod);
-
 	/* Prevent klp_ftrace_handler() from seeing KLP_UNDEFINED state */
 	if (klp_target_state == KLP_PATCHED)
 		klp_synchronize_transition();
@@ -130,6 +127,22 @@ static void klp_complete_transition(void)
 	}
 
 done:
+	klp_for_each_object(klp_transition_patch, obj) {
+		if (klp_target_state == KLP_PATCHED)
+			klp_post_patch_callback(obj);
+		else if (klp_target_state == KLP_PATCHED)
+			klp_post_unpatch_callback(obj);
+	}
+
+	/*
+	 * See complementary comment in __klp_enable_patch() for why we
+	 * keep the module reference for immediate patches.
+	 */
+	if (!klp_transition_patch->immediate) {
+		if (klp_target_state == KLP_UNPATCHED && !immediate_func)
+			module_put(klp_transition_patch->mod);
+	}
+
 	klp_target_state = KLP_UNDEFINED;
 	klp_transition_patch = NULL;
 }
diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
index 10319d7ea0b1..f8e545af5de4 100644
--- a/samples/livepatch/Makefile
+++ b/samples/livepatch/Makefile
@@ -1 +1,3 @@
 obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-sample.o
+obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-demo.o
+obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-callbacks-mod.o
diff --git a/samples/livepatch/livepatch-callbacks-demo.c b/samples/livepatch/livepatch-callbacks-demo.c
new file mode 100644
index 000000000000..a91f05c8258c
--- /dev/null
+++ b/samples/livepatch/livepatch-callbacks-demo.c
@@ -0,0 +1,190 @@
+/*
+ * Copyright (C) 2017 Joe Lawrence <joe.lawrence@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * livepatch-callbacks-demo.c - (un)patching callbacks livepatch demo
+ *
+ *
+ * Purpose
+ * -------
+ *
+ * Demonstration of registering livepatch (un)patching callbacks.
+ *
+ *
+ * Usage
+ * -----
+ *
+ * Step 1 - load the simple module
+ *
+ *   insmod samples/livepatch/livepatch-callbacks-mod.ko
+ *
+ *
+ * Step 2 - load the demonstration livepatch (with callbacks)
+ *
+ *   insmod samples/livepatch/livepatch-callbacks-demo.ko
+ *
+ *
+ * Step 3 - cleanup
+ *
+ *   echo 0 > /sys/kernel/livepatch/livepatch_callbacks_demo/enabled
+ *   rmmod livepatch_callbacks_demo
+ *   rmmod livepatch_callbacks_mod
+ *
+ * Watch dmesg output to see livepatch enablement, callback execution
+ * and patching operations for both vmlinux and module targets.
+ *
+ * NOTE: swap the insmod order of livepatch-callbacks-mod.ko and
+ *       livepatch-callbacks-demo.ko to observe what happens when a
+ *       target module is loaded after a livepatch with callbacks.
+ *
+ * NOTE: 'pre_patch_ret' is a module parameter that sets the pre-patch
+ *       callback return status.  Try setting up a non-zero status
+ *       such as -19 (-ENODEV):
+ *
+ *       # Load demo livepatch, vmlinux is patched
+ *       insmod samples/livepatch/livepatch-callbacks-demo.ko
+ *
+ *       # Setup next pre-patch callback to return -ENODEV
+ *       echo -19 > /sys/module/livepatch_callbacks_demo/parameters/pre_patch_ret 
+ *
+ *       # Module loader refuses to load the target module
+ *       insmod samples/livepatch/livepatch-callbacks-mod.ko 
+ *       insmod: ERROR: could not insert module samples/livepatch/livepatch-callbacks-mod.ko: No such device
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/livepatch.h>
+#include <linux/stat.h>
+
+static int pre_patch_ret;
+module_param(pre_patch_ret, int, 0644);
+MODULE_PARM_DESC(pre_patch_ret, "pre_patch_ret (default=0)");
+
+const char *module_state[] = {
+	[MODULE_STATE_LIVE]	= "[MODULE_STATE_LIVE] Normal state",
+	[MODULE_STATE_COMING]	= "[MODULE_STATE_COMING] Full formed, running module_init",
+	[MODULE_STATE_GOING]	= "[MODULE_STATE_GOING] Going away",
+	[MODULE_STATE_UNFORMED]	= "[MODULE_STATE_UNFORMED] Still setting it up",
+};
+
+static void callback_info(const char *callback, struct klp_object *obj)
+{
+	if (obj->mod)
+		pr_info("%s: %s -> %s\n", callback, obj->mod->name,
+			module_state[obj->mod->state]);
+	else
+		pr_info("%s: vmlinux\n", callback);
+}
+
+/* Executed on object patching (ie, patch enablement) */
+static int pre_patch_callback(struct klp_object *obj)
+{
+	callback_info(__func__, obj);
+	return pre_patch_ret;
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void post_patch_callback(struct klp_object *obj)
+{
+	callback_info(__func__, obj);
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void pre_unpatch_callback(struct klp_object *obj)
+{
+	callback_info(__func__, obj);
+}
+
+/* Executed on object unpatching (ie, patch disablement) */
+static void post_unpatch_callback(struct klp_object *obj)
+{
+	callback_info(__func__, obj);
+}
+
+static struct klp_func funcs[] = {
+	{ }
+};
+
+static struct klp_object objs[] = {
+	{
+		.name = NULL,	/* vmlinux */
+		.funcs = funcs,
+		.callbacks = {
+			.pre_patch = pre_patch_callback,
+			.post_patch = post_patch_callback,
+			.pre_unpatch = pre_unpatch_callback,
+			.post_unpatch = post_unpatch_callback,
+		},
+	},	{
+		.name = "livepatch_callbacks_mod",
+		.funcs = funcs,
+		.callbacks = {
+			.pre_patch = pre_patch_callback,
+			.post_patch = post_patch_callback,
+			.pre_unpatch = pre_unpatch_callback,
+			.post_unpatch = post_unpatch_callback,
+		},
+	}, { }
+};
+
+static struct klp_patch patch = {
+	.mod = THIS_MODULE,
+	.objs = objs,
+};
+
+static int livepatch_callbacks_demo_init(void)
+{
+	int ret;
+
+	if (!klp_have_reliable_stack() && !patch.immediate) {
+		/*
+		 * WARNING: Be very careful when using 'patch.immediate' in
+		 * your patches.  It's ok to use it for simple patches like
+		 * this, but for more complex patches which change function
+		 * semantics, locking semantics, or data structures, it may not
+		 * be safe.  Use of this option will also prevent removal of
+		 * the patch.
+		 *
+		 * See Documentation/livepatch/livepatch.txt for more details.
+		 */
+		patch.immediate = true;
+		pr_notice("The consistency model isn't supported for your architecture.  Bypassing safety mechanisms and applying the patch immediately.\n");
+	}
+
+	ret = klp_register_patch(&patch);
+	if (ret)
+		return ret;
+	ret = klp_enable_patch(&patch);
+	if (ret) {
+		WARN_ON(klp_unregister_patch(&patch));
+		return ret;
+	}
+	return 0;
+}
+
+static void livepatch_callbacks_demo_exit(void)
+{
+	WARN_ON(klp_unregister_patch(&patch));
+}
+
+module_init(livepatch_callbacks_demo_init);
+module_exit(livepatch_callbacks_demo_exit);
+MODULE_LICENSE("GPL");
+MODULE_INFO(livepatch, "Y");
diff --git a/samples/livepatch/livepatch-callbacks-mod.c b/samples/livepatch/livepatch-callbacks-mod.c
new file mode 100644
index 000000000000..d0f8657e19f3
--- /dev/null
+++ b/samples/livepatch/livepatch-callbacks-mod.c
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2017 Joe Lawrence <joe.lawrence@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * livepatch-callbacks-mod.c - (un)patching callbacks demo support module
+ *
+ *
+ * Purpose
+ * -------
+ *
+ * Simple module to demonstrate livepatch (un)patching callbacks.
+ *
+ *
+ * Usage
+ * -----
+ *
+ * This module is not intended to be standalone.  See the "Usage"
+ * section of livepatch-callbacks-mod.c.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+
+static int livepatch_callbacks_mod_init(void)
+{
+	pr_info("%s\n", __func__);
+	return 0;
+}
+
+static void livepatch_callbacks_mod_exit(void)
+{
+	pr_info("%s\n", __func__);
+}
+
+module_init(livepatch_callbacks_mod_init);
+module_exit(livepatch_callbacks_mod_exit);
+MODULE_LICENSE("GPL");
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1709955 — Re: [PATCH v2 1/1] livepatch: add (un)patch callbacks

On Tue, Aug 08, 2017 at 03:36:07PM -0400, Joe Lawrence wrote:
> +++ b/Documentation/livepatch/callbacks.txt
> @@ -0,0 +1,75 @@
> +(Un)patching Callbacks
> +======================
> +
> +Livepatch (un)patch-callbacks provide a mechanism for livepatch modules
> +to execute callback functions when a kernel object is (un)patched.

I think it would be helpful to put a little blurb here about why
callbacks are needed and when they might be used.  Maybe steal some of
the description from the first two bullet points here:

  https://lkml.kernel.org/r/20170720041723.35r6qk2fia7xix3t@treble

Also, I tested stop_machine() in the callbacks and it seemed to work
fine.  It might be worth mentioning in the docs that it's an option.

> +
> +These callbacks differ from existing kernel facilities:
> +
> +  - Module init/exit code doesn't run when disabling and re-enabling a
> +    patch.
> +
> +  - A module notifier can't stop the to-be-patched module from loading.
> +
> +Callbacks are part of the klp_object structure and their implementation
> +is specific to the given object.  Other livepatch objects may or may not
> +be patched, irrespective of the target klp_object's current state.
> +
> +Callbacks can be registered for the following livepatch actions:
> +
> +  * Pre-patch    - before klp_object is patched
> +
> +  * Post-patch   - after klp_object has been patched and is active
> +                   across all tasks
> +
> +  * Pre-unpatch  - before klp_object is unpatched, patched code is active
> +
> +  * Post-unpatch - after klp_object has been patched, all code has been
> +		   restored and no tasks are running patched code
> +
> +Callbacks are only executed if its host klp_object is loaded.  For

"Callbacks are" -> "A callback is" ?

> +static inline int klp_pre_patch_callback(struct klp_object *obj)
> +{
> +	if (!obj->patched && obj->callbacks.pre_patch)
> +		return (*obj->callbacks.pre_patch)(obj);
> +	return 0;
> +}
> +static inline void klp_post_patch_callback(struct klp_object *obj)
> +{
> +	if (obj->patched && obj->callbacks.post_patch)
> +		(*obj->callbacks.post_patch)(obj);
> +}
> +static inline void klp_pre_unpatch_callback(struct klp_object *obj)
> +{
> +	if (obj->patched && obj->callbacks.pre_unpatch)
> +		(*obj->callbacks.pre_unpatch)(obj);
> +}
> +static inline void klp_post_unpatch_callback(struct klp_object *obj)
> +{
> +	if (!obj->patched && obj->callbacks.post_unpatch)
> +		(*obj->callbacks.post_unpatch)(obj);
> +}
> +

Do these need the obj->patched checks?  As far as I can tell they seem
to be called in the right places and the checks are superfluous.

> --- a/kernel/livepatch/transition.c
> +++ b/kernel/livepatch/transition.c
> @@ -109,9 +109,6 @@ static void klp_complete_transition(void)
>  		}
>  	}
>  
> -	if (klp_target_state == KLP_UNPATCHED && !immediate_func)
> -		module_put(klp_transition_patch->mod);
> -
>  	/* Prevent klp_ftrace_handler() from seeing KLP_UNDEFINED state */
>  	if (klp_target_state == KLP_PATCHED)
>  		klp_synchronize_transition();
> @@ -130,6 +127,22 @@ static void klp_complete_transition(void)
>  	}
>  
>  done:
> +	klp_for_each_object(klp_transition_patch, obj) {
> +		if (klp_target_state == KLP_PATCHED)
> +			klp_post_patch_callback(obj);
> +		else if (klp_target_state == KLP_PATCHED)

s/KLP_PATCHED/KLP_UNPATCHED

> +			klp_post_unpatch_callback(obj);
> +	}
> +
> +	/*
> +	 * See complementary comment in __klp_enable_patch() for why we
> +	 * keep the module reference for immediate patches.
> +	 */
> +	if (!klp_transition_patch->immediate) {
> +		if (klp_target_state == KLP_UNPATCHED && !immediate_func)
> +			module_put(klp_transition_patch->mod);
> +	}
> +

Maybe combine these into a single 'if' for clarity:

	if (klp_target_state == KLP_UNPATCHED && !immediate_func &&
	    !klp_transition_patch->immediate)
		module_put(klp_transition_patch->mod);

> + * NOTE: 'pre_patch_ret' is a module parameter that sets the pre-patch
> + *       callback return status.  Try setting up a non-zero status
> + *       such as -19 (-ENODEV):
> + *
> + *       # Load demo livepatch, vmlinux is patched
> + *       insmod samples/livepatch/livepatch-callbacks-demo.ko
> + *
> + *       # Setup next pre-patch callback to return -ENODEV
> + *       echo -19 > /sys/module/livepatch_callbacks_demo/parameters/pre_patch_ret 

Git complained about trailing whitespace here ^

> + *
> + *       # Module loader refuses to load the target module
> + *       insmod samples/livepatch/livepatch-callbacks-mod.ko 

and here ^

> +/* Executed on object unpatching (ie, patch disablement) */
> +static void post_patch_callback(struct klp_object *obj)

s/unpatching/patching/

-- 
Josh

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web