Kernel Live Patching

This commit introduces code for the live patching core. It implements an ftrace-based mechanism and kernel interface for doing live patching of kernel and kernel module functions.

It represents the greatest common functionality set between kpatch and kgraft and can accept patches built using either method.

This first version does not implement any consistency mechanism that ensures that old and new code do not run together. In practice, ~90% of CVEs are safe to apply in this way, since they simply add a conditional check. However, any function change that can not execute safely with the old version of the function can _not_ be safely applied in this version.

include/linux/livepatch.h new file +67 -0

new file mode 100644
index 0000000..4e01b59
@@ -0,0 +1,121 @@
+/*
+ * livepatch.h - Kernel Live Patching Core
+ *
+ * Copyright (C) 2014 Seth Jennings <[email protected]>
+ *
+ * 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/>.
+ */
+
+#ifndef _LINUX_LIVEPATCH_H_
+#define _LINUX_LIVEPATCH_H_
+
+#include <linux/module.h>
+#include <linux/ftrace.h>
+
+#if IS_ENABLED(CONFIG_LIVE_PATCHING)
+
+#include <asm/livepatch.h>
+
+enum klp_state {
+	KLP_DISABLED,
+	KLP_ENABLED
+};
+
+/**
+ * struct klp_func - function structure for live patching
+ * @old_name:	name of the function to be patched
+ * @new_func:	pointer to the patched function code
+ * @old_addr:	a hint conveying at what address the old function
+ *		can be found (optional, vmlinux patches only)
+ */
+struct klp_func {
+	/* external */
+	const char *old_name;
+	void *new_func;
+	/*
+	 * The old_addr field is optional and can be used to resolve
+	 * duplicate symbol names in the vmlinux object.  If this
+	 * information is not present, the symbol is located by name
+	 * with kallsyms. If the name is not unique and old_addr is
+	 * not provided, the patch application fails as there is no
+	 * way to resolve the ambiguity.
+	 */
+	unsigned long old_addr;
+
+	/* internal */
+	struct kobject kobj;
+	struct ftrace_ops *fops;
+	enum klp_state state;
+};
+
+/**
+ * struct klp_reloc - relocation structure for live patching
+ * @dest	address where the relocation will be written
+ * @src		address of the referenced symbol (optional,
+ *		vmlinux	patches only)
+ * @type	ELF relocation type

arch/x86/include/asm/livepatch.h new file +0 -0

@@ -0,0 +1,40 @@

arch/x86/include/asm/livepatch.h new file +21 -0

+#ifndef _ASM_X86_LIVEPATCH_H
+#define _ASM_X86_LIVEPATCH_H
+
+#include <linux/module.h>
+
+#ifdef CONFIG_LIVE_PATCHING
+#ifndef CC_USING_FENTRY
+#error Your compiler must support -mfentry for live patching to work
+#endif
+extern int klp_write_module_reloc(struct module *mod, unsigned long type,
+				  unsigned long loc, unsigned long value);
+
+#else
+static inline int klp_write_module_reloc(struct module *mod, unsigned long type,
+					 unsigned long loc, unsigned long value)
+{
+	return -ENOSYS;
+}
+#endif
+
+#endif /* _ASM_X86_LIVEPATCH_H */

arch/x86/kernel/livepatch.c new file +0 -0

@@ -0,0 +1,74 @@

arch/x86/kernel/livepatch.c new file +29 -0

+int klp_write_module_reloc(struct module *mod, unsigned long type,
+			   unsigned long loc, unsigned long value)
+{
+	int ret, readonly, numpages, size = 4;
+	unsigned long val;
+	unsigned long core = (unsigned long)mod->module_core;
+	unsigned long core_ro_size = mod->core_ro_size;
+	unsigned long core_size = mod->core_size;
+
+	switch (type) {
+	case R_X86_64_NONE:
+		return 0;
+	case R_X86_64_64:
+		val = value;
+		size = 8;
+		break;
+	case R_X86_64_32:
+		val = (u32)value;
+		break;
+	case R_X86_64_32S:
+		val = (s32)value;
+		break;
+	case R_X86_64_PC32:
+		val = (u32)(value - loc);
+		break;
+	default:
+		/* unsupported relocation type */
+		return -EINVAL;
+	}

arch/x86/kernel/livepatch.c new file +10 -0

+	if (readonly)
+		set_memory_rw(loc & PAGE_MASK, numpages);
+
+	ret = probe_kernel_write((void *)loc, &val, size);
+
+	if (readonly)
+		set_memory_ro(loc & PAGE_MASK, numpages);
+
+	return ret;
+}

include/linux/livepatch.h new file +0 -0

new file mode 100644
index 0000000..4e01b59
@@ -0,0 +1,121 @@

include/linux/livepatch.h new file +26 -0

+/**
+ * struct klp_func - function structure for live patching
+ * @old_name:	name of the function to be patched
+ * @new_func:	pointer to the patched function code
+ * @old_addr:	a hint conveying at what address the old function
+ *		can be found (optional, vmlinux patches only)
+ */
+struct klp_func {
+	/* external */
+	const char *old_name;
+	void *new_func;
+	/*
+	 * The old_addr field is optional and can be used to resolve
+	 * duplicate symbol names in the vmlinux object.  If this
+	 * information is not present, the symbol is located by name
+	 * with kallsyms. If the name is not unique and old_addr is
+	 * not provided, the patch application fails as there is no
+	 * way to resolve the ambiguity.
+	 */
+	unsigned long old_addr;
+
+	/* internal */
+	struct kobject kobj;
+	struct ftrace_ops *fops;
+	enum klp_state state;
+};

include/linux/livepatch.h new file +20 -0

+struct klp_reloc {
+	unsigned long dest;
+	unsigned long src;
+	unsigned long type;
+	const char *name;
+	int addend;
+	int external;
+};
+
+/* struct klp_object - kernel object structure for live patching
+ * @name	module name (or NULL for vmlinux)
+ * @relocs	relocation entries to be applied at load time
+ * @funcs	function entries for functions to be patched in the object
+ */
+struct klp_object {
+	/* external */
+	const char *name;
+	struct klp_reloc *relocs;
+	struct klp_func *funcs;
+

+ */

include/linux/livepatch.h new file +10 -0

+struct klp_patch {
+	/* external */
+	struct module *mod;
+	struct klp_object *objs;
+
+	/* internal */
+	struct list_head list;
+	struct kobject kobj;
+	enum klp_state state;
+};

kernel/livepatch/core.c new file +0 -0

@@ -0,0 +1,807 @@

kernel/livepatch/core.c new file +4 -0

+static void klp_module_notify_coming(struct module *pmod,
+				     struct klp_object *obj)
+{
+	struct module *mod = obj->mod;

kernel/livepatch/core.c new file +5 -0

+	ret = klp_enable_object(pmod, obj);
+	if (ret)
+		pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
+			pmod->name, mod->name, ret);
+}

kernel/livepatch/core.c new file +7 -0

+}
+
+static struct kobj_type klp_ktype_patch = {
+	.release = klp_kobj_release_patch,
+	.sysfs_ops = &kobj_sysfs_ops,
+	.default_attrs = klp_patch_attrs
+};

kernel/livepatch/core.c new file +12 -0

+}
+
+static int klp_init_funcs(struct klp_object *obj)
+{
+	struct klp_func *func;
+	struct ftrace_ops *ops;
+	int ret;
+
+	if (!obj->funcs)
+		return -EINVAL;
+
+	for (func = obj->funcs; func->old_name; func++) {

kernel/livepatch/core.c new file +14 -0

+		/* sysfs */
+		ret = kobject_init_and_add(&func->kobj, &klp_ktype_func,
+					   obj->kobj, func->old_name);
+		if (ret) {
+			kfree(ops);
+			goto free;
+		}
+
+		func->fops = ops;
+	}
+
+	return 0;
+free:
+	klp_free_funcs_limited(obj, func);

kernel/livepatch/core.c new file +9 -0

+		obj->state = KLP_DISABLED;
+
+		/* sysfs */
+		obj->kobj = kobject_create_and_add(obj->name, &patch->kobj);
+		if (!obj->kobj)
+			goto free;
+
+		/* init functions */
+		ret = klp_init_funcs(obj);

kernel/livepatch/core.c new file +6 -0

+	mutex_lock(&klp_mutex);
+	list_add(&patch->list, &klp_patches);
+	mutex_unlock(&klp_mutex);
+
+	return 0;
+}

kernel/livepatch/core.c new file +5 -0

+/**
+ * klp_unregister_patch() - unregisters a patch
+ * @patch:	Disabled patch to be unregistered
+ *
+ * Frees the data structures and removes the sysfs interface.

kernel/livepatch/core.c new file +17 -0

+		goto out;
+	}
+	klp_free_patch(patch);
+out:
+	mutex_unlock(&klp_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(klp_unregister_patch);
+
+static int klp_init(void)
+{
+	int ret;
+
+	ret = register_module_notifier(&klp_module_nb);
+	if (ret)
+		return ret;
+

include/linux/livepatch.h new file +44 -0

new file mode 100644
index 0000000..4e01b59
@@ -0,0 +1,121 @@
+/*
+ * livepatch.h - Kernel Live Patching Core
+ *
+ * Copyright (C) 2014 Seth Jennings <[email protected]>
+ *
+ * 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/>.
+ */
+
+#ifndef _LINUX_LIVEPATCH_H_
+#define _LINUX_LIVEPATCH_H_
+
+#include <linux/module.h>
+#include <linux/ftrace.h>
+
+#if IS_ENABLED(CONFIG_LIVE_PATCHING)
+
+#include <asm/livepatch.h>
+
+enum klp_state {
+	KLP_DISABLED,
+	KLP_ENABLED
+};
+
+/**
+ * struct klp_func - function structure for live patching
+ * @old_name:	name of the function to be patched
+ * @new_func:	pointer to the patched function code
+ * @old_addr:	a hint conveying at what address the old function
+ *		can be found (optional, vmlinux patches only)
+ */
+struct klp_func {
+	/* external */
+	const char *old_name;

include/linux/livepatch.h new file +19 -0

+{
+	struct klp_func *func = ops->private;
+
+	regs->ip = (unsigned long)func->new_func;
+}
+
+static int klp_enable_func(struct klp_func *func)
+{
+	int ret;
+
+	if (WARN_ON(!func->old_addr))
+		return -EINVAL;
+
+	if (WARN_ON(func->state != KLP_DISABLED))
+		return -EINVAL;
+
+	ret = ftrace_set_filter_ip(func->fops, func->old_addr, 0, 0);
+	if (ret) {
+		pr_err("failed to set ftrace filter for function '%s' (%d)\n",

include/linux/livepatch.h new file +3 -0

+
+	return ret;
+}

include/linux/livepatch.h new file +7 -0

+/**
+ * klp_disable_patch() - disables a registered patch
+ * @patch:	The registered, enabled patch to be disabled
+ *
+ * Unregisters the patched functions from ftrace.
+ *
+ * Return: 0 on success, otherwise error

include/linux/livepatch.h new file +7 -0

+	ret = __klp_disable_patch(patch);
+out:
+	mutex_unlock(&klp_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(klp_disable_patch);
+

include/linux/livepatch.h new file +49 -0

+	ret = klp_enable_object(pmod, obj);
+	if (ret)
+		pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
+			pmod->name, mod->name, ret);
+}
+
+static void klp_module_notify_going(struct module *pmod,
+				    struct klp_object *obj)
+{
+	struct module *mod = obj->mod;
+	int ret;
+
+	pr_notice("reverting patch '%s' on unloading module '%s'\n",
+		  pmod->name, mod->name);
+	ret = klp_disable_object(obj);
+	if (ret)
+		pr_warn("failed to revert patch '%s' on module '%s' (%d)\n",
+			pmod->name, mod->name, ret);
+	obj->mod = NULL;
+}
+
+static int klp_module_notify(struct notifier_block *nb, unsigned long action,
+			    void *data)
+{
+	struct module *mod = data;
+	struct klp_patch *patch;
+	struct klp_object *obj;
+
+	if (action != MODULE_STATE_COMING && action != MODULE_STATE_GOING)
+		return 0;
+
+	mutex_lock(&klp_mutex);
+	list_for_each_entry(patch, &klp_patches, list) {
+		if (patch->state == KLP_DISABLED)
+			continue;
+		for (obj = patch->objs; obj->funcs; obj++) {
+			if (!obj->name || strcmp(obj->name, mod->name))
+				continue;
+			if (action == MODULE_STATE_COMING) {
+				obj->mod = mod;
+				klp_module_notify_coming(patch->mod, obj);
+			} else /* MODULE_STATE_GOING */
+				klp_module_notify_going(patch->mod, obj);
+			break;
+		}
+	}
+	mutex_unlock(&klp_mutex);
+	return 0;
+}

include/linux/livepatch.h new file +13 -0

+		if (!obj->kobj)
+			goto free;
+
+		/* init functions */
+		ret = klp_init_funcs(obj);
+		if (ret) {
+			kobject_put(obj->kobj);
+			goto free;
+		}
+	}
+
+	return 0;
+free:

include/linux/livepatch.h new file +8 -0

+
+	/*
+	 * A reference is taken on the patch module to prevent it from being
+	 * unloaded.  Right now, we don't allow patch modules to unload since
+	 * there is currently no method to determine if a thread is still
+	 * running in the patched code contained in the patch module once
+	 * the ftrace registration is successful.
+	 */

Quoted 10 lines

include/linux/livepatch.h new file +10 -0

+/**
+ * struct klp_func - function structure for live patching
+ * @old_name:	name of the function to be patched
+ * @new_func:	pointer to the patched function code
+ * @old_addr:	a hint conveying at what address the old function
+ *		can be found (optional, vmlinux patches only)
+ */
+struct klp_func {
+	/* external */
+	const char *old_name;

include/linux/livepatch.h line 54

Quoted 10 lines

include/linux/livepatch.h new file +10 -0

+	void *new_func;
+	/*
+	 * The old_addr field is optional and can be used to resolve
+	 * duplicate symbol names in the vmlinux object.  If this
+	 * information is not present, the symbol is located by name
+	 * with kallsyms. If the name is not unique and old_addr is
+	 * not provided, the patch application fails as there is no
+	 * way to resolve the ambiguity.
+	 */
+	unsigned long old_addr;

I wonder if we really need old_addr as an external field. I assume that userspace tool in kpatch use it as a "hint" for kernel part and thus kallsyms is not needed there (and it solves ambiguity problem as well). But I am not sure if it is gonna be the same in upstream. When kernel is randomized (CONFIG_RANDOMIZE_BASE is set to 'y', though default is 'n') old_addr is not usable (and we throw it away in the code). Without old_addr being set by the user we could spare some of code (calls to klp_verify_vmlinux_symbol and such).

So the question is whether future userspace tool in upstream would need it and would use it. Please note that I do not mean it as a kpatch or kgraft way to do things, I'm just not sure about old_addr being "public" and want do discuss the options.

The ambiguity of symbols was discussed in some other thread in lkml in october (I guess) with no conclusion IIRC...

Hi Miroslav,

Just addressing one of your comments below. I'll let Seth respond to the others :-)

Quoted 21 lines · Miroslav Benes

Quoted 10 lines

include/linux/livepatch.h new file +10 -0

+/**
+ * struct klp_func - function structure for live patching
+ * @old_name:	name of the function to be patched
+ * @new_func:	pointer to the patched function code
+ * @old_addr:	a hint conveying at what address the old function
+ *		can be found (optional, vmlinux patches only)
+ */
+struct klp_func {
+	/* external */
+	const char *old_name;

include/linux/livepatch.h line 54

Quoted 10 lines

include/linux/livepatch.h new file +10 -0

+	void *new_func;
+	/*
+	 * The old_addr field is optional and can be used to resolve
+	 * duplicate symbol names in the vmlinux object.  If this
+	 * information is not present, the symbol is located by name
+	 * with kallsyms. If the name is not unique and old_addr is
+	 * not provided, the patch application fails as there is no
+	 * way to resolve the ambiguity.
+	 */
+	unsigned long old_addr;

Quoted 8 lines

I wonder if we really need old_addr as an external field. I assume that userspace tool in kpatch use it as a "hint" for kernel part and thus kallsyms is not needed there (and it solves ambiguity problem as well). But I am not sure if it is gonna be the same in upstream. When kernel is randomized (CONFIG_RANDOMIZE_BASE is set to 'y', though default is 'n') old_addr is not usable (and we throw it away in the code). Without old_addr being set by the user we could spare some of code (calls to klp_verify_vmlinux_symbol and such).

Even with CONFIG_RANDOMIZE_BASE, the function offsets will be the same regardless of the base address. So we could still use old_addr to determine the offset.

Quoted 5 lines

So the question is whether future userspace tool in upstream would need it and would use it. Please note that I do not mean it as a kpatch or kgraft way to do things, I'm just not sure about old_addr being "public" and want do discuss the options.

Quoted 2 lines

The ambiguity of symbols was discussed in some other thread in lkml in october (I guess) with no conclusion IIRC...

We need to resolve ambiguity somehow, and old_addr is a way to do that. Do you have any other ideas?

Quoted 45 lines · Josh Poimboeuf

Hi Miroslav,

Just addressing one of your comments below. I'll let Seth respond to the others :-)

Quoted 21 lines · Miroslav Benes

Quoted 10 lines

include/linux/livepatch.h new file +10 -0

+/**
+ * struct klp_func - function structure for live patching
+ * @old_name:	name of the function to be patched
+ * @new_func:	pointer to the patched function code
+ * @old_addr:	a hint conveying at what address the old function
+ *		can be found (optional, vmlinux patches only)
+ */
+struct klp_func {
+	/* external */
+	const char *old_name;

include/linux/livepatch.h line 54

Quoted 10 lines

include/linux/livepatch.h new file +10 -0

+	void *new_func;
+	/*
+	 * The old_addr field is optional and can be used to resolve
+	 * duplicate symbol names in the vmlinux object.  If this
+	 * information is not present, the symbol is located by name
+	 * with kallsyms. If the name is not unique and old_addr is
+	 * not provided, the patch application fails as there is no
+	 * way to resolve the ambiguity.
+	 */
+	unsigned long old_addr;

Quoted 8 lines

I wonder if we really need old_addr as an external field. I assume that userspace tool in kpatch use it as a "hint" for kernel part and thus kallsyms is not needed there (and it solves ambiguity problem as well). But I am not sure if it is gonna be the same in upstream. When kernel is randomized (CONFIG_RANDOMIZE_BASE is set to 'y', though default is 'n') old_addr is not usable (and we throw it away in the code). Without old_addr being set by the user we could spare some of code (calls to klp_verify_vmlinux_symbol and such).

Even with CONFIG_RANDOMIZE_BASE, the function offsets will be the same regardless of the base address. So we could still use old_addr to determine the offset.

Quoted 5 lines

So the question is whether future userspace tool in upstream would need it and would use it. Please note that I do not mean it as a kpatch or kgraft way to do things, I'm just not sure about old_addr being "public" and want do discuss the options.

Quoted 5 lines

Quoted 2 lines

The ambiguity of symbols was discussed in some other thread in lkml in october (I guess) with no conclusion IIRC...

We need to resolve ambiguity somehow, and old_addr is a way to do that. Do you have any other ideas?

Unfortunately I don't.

But similarly we don't deal with ambiguity in modules either. And it is (at least theoretically) possible. Two static functions of the same name in two different .c files which the final module is linked from. You have to use kallsyms and it would get confused. Maybe this sounds odd but it could happen.

Thus the old_addr value is not general protection (as modules are still affected) and it is questionable whether the user should use it.

I do not have strong opinion on this and if no one else shares my thoughts, I am not against.

Mira

Quoted 53 lines · Miroslav Benes

Quoted 45 lines · Josh Poimboeuf

Hi Miroslav,

Just addressing one of your comments below. I'll let Seth respond to the others :-)

Quoted 21 lines · Miroslav Benes

Quoted 10 lines

include/linux/livepatch.h new file +10 -0

+/**
+ * struct klp_func - function structure for live patching
+ * @old_name:	name of the function to be patched
+ * @new_func:	pointer to the patched function code
+ * @old_addr:	a hint conveying at what address the old function
+ *		can be found (optional, vmlinux patches only)
+ */
+struct klp_func {
+	/* external */
+	const char *old_name;

include/linux/livepatch.h line 54

Quoted 10 lines

include/linux/livepatch.h new file +10 -0

+	void *new_func;
+	/*
+	 * The old_addr field is optional and can be used to resolve
+	 * duplicate symbol names in the vmlinux object.  If this
+	 * information is not present, the symbol is located by name
+	 * with kallsyms. If the name is not unique and old_addr is
+	 * not provided, the patch application fails as there is no
+	 * way to resolve the ambiguity.
+	 */
+	unsigned long old_addr;

Quoted 8 lines

I wonder if we really need old_addr as an external field. I assume that userspace tool in kpatch use it as a "hint" for kernel part and thus kallsyms is not needed there (and it solves ambiguity problem as well). But I am not sure if it is gonna be the same in upstream. When kernel is randomized (CONFIG_RANDOMIZE_BASE is set to 'y', though default is 'n') old_addr is not usable (and we throw it away in the code). Without old_addr being set by the user we could spare some of code (calls to klp_verify_vmlinux_symbol and such).

Even with CONFIG_RANDOMIZE_BASE, the function offsets will be the same regardless of the base address. So we could still use old_addr to determine the offset.

Quoted 5 lines

So the question is whether future userspace tool in upstream would need it and would use it. Please note that I do not mean it as a kpatch or kgraft way to do things, I'm just not sure about old_addr being "public" and want do discuss the options.

Quoted 5 lines

Quoted 2 lines

The ambiguity of symbols was discussed in some other thread in lkml in october (I guess) with no conclusion IIRC...

We need to resolve ambiguity somehow, and old_addr is a way to do that. Do you have any other ideas?

Quoted 7 lines

Unfortunately I don't.

But similarly we don't deal with ambiguity in modules either. And it is (at least theoretically) possible. Two static functions of the same name in two different .c files which the final module is linked from. You have to use kallsyms and it would get confused. Maybe this sounds odd but it could happen.

True, this is a remote possibility, but we haven't run into this issue yet. If it becomes a problem, we can try to come up with another way to resolve duplicates.

Here's one idea: since the symbols are always listed in the same order in kallsyms (per-object), instead of old_addr we could have sym_idx. A sym_idx of 2 could mean "I want the 2nd occurence of foo in the object's kallsyms list".

However I'd rather keep our current old_addr scheme for now, since it's what we have implemented already. And there are plenty of more important things we need to do first.

Quoted 2 lines

Thus the old_addr value is not general protection (as modules are still affected) and it is questionable whether the user should use it.

It's not really protection, since even if you don't specify old_addr and you rely on kallsyms, klp_find_symbol will return an error if there are any duplicates.

It's really just a way to increase the size of the set of functions which can be patched (duplicately named functions).

We also rely on something similar for relocations: klp_reloc.src. It's even more important there, since duplicately named static objects are more common than duplicately named functions.

Quoted 4 lines

I do not have strong opinion on this and if no one else shares my thoughts, I am not against.

Mira

Signature · 1 line

Josh

This commit introduces code for the live patching core. It implements an ftrace-based mechanism and kernel interface for doing live patching of kernel and kernel module functions.

It represents the greatest common functionality set between kpatch and kgraft and can accept patches built using either method.

This first version does not implement any consistency mechanism that ensures that old and new code do not run together. In practice, ~90% of CVEs are safe to apply in this way, since they simply add a conditional check. However, any function change that can not execute safely with the old version of the function can _not_ be safely applied in this version.

arch/x86/include/asm/livepatch.h new file +28 -0

new file mode 100644
index 0000000..38fa496
@@ -0,0 +1,40 @@
+/*
+ * livepatch.h - x86-specific Kernel Live Patching Core
+ *
+ * Copyright (C) 2014 Seth Jennings <[email protected]>
+ *
+ * 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/>.
+ */
+
+#ifndef _ASM_X86_LIVEPATCH_H
+#define _ASM_X86_LIVEPATCH_H
+
+#include <linux/module.h>
+
+#ifdef CONFIG_LIVE_PATCHING
+#ifndef CC_USING_FENTRY
+#error Your compiler must support -mfentry for live patching to work
+#endif

arch/x86/include/asm/livepatch.h new file +2 -0

+
+#endif /* _ASM_X86_LIVEPATCH_H */

arch/x86/kernel/livepatch.c new file +14 -0

@@ -0,0 +1,74 @@
+/*
+ * livepatch.c - x86-specific Kernel Live Patching Core
+ *
+ * Copyright (C) 2014 Seth Jennings <[email protected]>
+ *
+ * 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.

arch/x86/kernel/livepatch.c new file +23 -0

+	unsigned long val;
+	unsigned long core = (unsigned long)mod->module_core;
+	unsigned long core_ro_size = mod->core_ro_size;
+	unsigned long core_size = mod->core_size;
+
+	switch (type) {
+	case R_X86_64_NONE:
+		return 0;
+	case R_X86_64_64:
+		val = value;
+		size = 8;
+		break;
+	case R_X86_64_32:
+		val = (u32)value;
+		break;
+	case R_X86_64_32S:
+		val = (s32)value;
+		break;
+	case R_X86_64_PC32:
+		val = (u32)(value - loc);
+		break;
+	default:
+		/* unsupported relocation type */

arch/x86/kernel/livepatch.c new file +12 -0

+	numpages = ((loc & PAGE_MASK) == ((loc + size) & PAGE_MASK)) ? 1 : 2;
+
+	if (readonly)
+		set_memory_rw(loc & PAGE_MASK, numpages);
+
+	ret = probe_kernel_write((void *)loc, &val, size);
+
+	if (readonly)
+		set_memory_ro(loc & PAGE_MASK, numpages);
+
+	return ret;
+}

include/linux/livepatch.h new file +29 -0

new file mode 100644
index 0000000..4e01b59
@@ -0,0 +1,121 @@
+/*
+ * livepatch.h - Kernel Live Patching Core
+ *
+ * Copyright (C) 2014 Seth Jennings <[email protected]>
+ *
+ * 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/>.
+ */
+
+#ifndef _LINUX_LIVEPATCH_H_
+#define _LINUX_LIVEPATCH_H_
+
+#include <linux/module.h>
+#include <linux/ftrace.h>
+
+#if IS_ENABLED(CONFIG_LIVE_PATCHING)
+
+#include <asm/livepatch.h>
+

+ * @name module name (or NULL for vmlinux)
+ * @relocs relocation entries to be applied at load time
+ * @funcs function entries for functions to be patched in the object
+ */

include/linux/livepatch.h new file +36 -0

+struct klp_object {
+	/* external */
+	const char *name;
+	struct klp_reloc *relocs;
+	struct klp_func *funcs;
+
+	/* internal */
+	struct kobject *kobj;
+	struct module *mod;
+	enum klp_state state;
+};
+
+/**
+ * struct klp_patch - patch structure for live patching
+ * @mod		reference to the live patch module
+ * @objs	object entries for kernel objects to be patched
+ */
+struct klp_patch {
+	/* external */
+	struct module *mod;
+	struct klp_object *objs;
+
+	/* internal */
+	struct list_head list;
+	struct kobject kobj;
+	enum klp_state state;
+};
+
+extern int klp_register_patch(struct klp_patch *);
+extern int klp_unregister_patch(struct klp_patch *);
+extern int klp_enable_patch(struct klp_patch *);
+extern int klp_disable_patch(struct klp_patch *);
+
+#endif /* CONFIG_LIVE_PATCHING */
+
+#endif /* _LINUX_LIVEPATCH_H_ */

kernel/livepatch/core.c new file +47 -0

@@ -0,0 +1,807 @@
+/*
+ * core.c - Kernel Live Patching Core
+ *
+ * Copyright (C) 2014 Seth Jennings <[email protected]>
+ *
+ * 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/>.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/ftrace.h>
+#include <linux/list.h>
+#include <linux/kallsyms.h>
+#include <linux/livepatch.h>
+
+/*
+ * The klp_mutex protects the klp_patches list and state transitions of any
+ * structure reachable from the patches list.  References to any structure must
+ * be obtained under mutex protection.
+ */
+
+static DEFINE_MUTEX(klp_mutex);
+static LIST_HEAD(klp_patches);
+
+static struct kobject *klp_root_kobj;
+
+/* sets obj->mod if object is not vmlinux and module is found */
+static bool klp_find_object_module(struct klp_object *obj)
+{
+	if (!obj->name)
+		return 1;
+

kernel/livepatch/core.c new file +1 -0

+}

kernel/livepatch/core.c new file +5 -0

+		if (!obj->name) {
+			ret = klp_verify_vmlinux_symbol(reloc->name,
+							reloc->src);
+			if (ret)
+				return ret;

kernel/livepatch/core.c new file +15 -0

+			if (ret)
+				return ret;
+		}
+		ret = klp_write_module_reloc(pmod, reloc->type, reloc->dest,
+					     reloc->src + reloc->addend);
+		if (ret) {
+			pr_err("relocation failed for symbol '%s' at 0x%016lx (%d)\n",
+			       reloc->name, reloc->src, ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+

kernel/livepatch/core.c new file +10 -0

+
+	for (func = obj->funcs; func->old_name; func++) {
+		if (func->state != KLP_ENABLED)
+			continue;
+		ret = klp_disable_func(func);
+		if (ret)
+			return ret;
+		if (obj->name)
+			func->old_addr = 0;
+	}

kernel/livepatch/core.c new file +16 -0

+		if (ret)
+			goto unregister;
+	}
+
+	for (func = obj->funcs; func->old_name; func++) {
+		ret = klp_find_verify_func_addr(func, obj->name);
+		if (ret)
+			goto unregister;
+
+		ret = klp_enable_func(func);
+		if (ret)
+			goto unregister;
+	}
+	obj->state = KLP_ENABLED;
+
+	return 0;

kernel/livepatch/core.c new file +18 -0

+	pr_notice("disabling patch '%s'\n", patch->mod->name);
+
+	for (obj = patch->objs; obj->funcs; obj++) {
+		if (obj->state != KLP_ENABLED)
+			continue;
+		ret = klp_disable_object(obj);
+		if (ret)
+			return ret;
+	}
+	patch->state = KLP_DISABLED;
+
+	return 0;
+}
+
+/**
+ * klp_disable_patch() - disables a registered patch
+ * @patch:	The registered, enabled patch to be disabled
+ *

kernel/livepatch/core.c new file +51 -0

+		ret = -EINVAL;
+		goto out;
+	}
+	ret = __klp_disable_patch(patch);
+out:
+	mutex_unlock(&klp_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(klp_disable_patch);
+
+static int __klp_enable_patch(struct klp_patch *patch)
+{
+	struct klp_object *obj;
+	int ret;
+
+	if (WARN_ON(patch->state != KLP_DISABLED))
+		return -EINVAL;
+
+	pr_notice_once("tainting kernel with TAINT_LIVEPATCH\n");
+	add_taint(TAINT_LIVEPATCH, LOCKDEP_STILL_OK);
+
+	pr_notice("enabling patch '%s'\n", patch->mod->name);
+
+	for (obj = patch->objs; obj->funcs; obj++) {
+		if (!klp_find_object_module(obj))
+			continue;
+		ret = klp_enable_object(patch->mod, obj);
+		if (ret)
+			goto unregister;
+	}
+	patch->state = KLP_ENABLED;
+	return 0;
+
+unregister:
+	WARN_ON(__klp_disable_patch(patch));
+	return ret;
+}
+
+/**
+ * klp_enable_patch() - enables a registered patch
+ * @patch:	The registered, disabled patch to be enabled
+ *
+ * Performs the needed symbol lookups and code relocations,
+ * then registers the patched functions with ftrace.
+ *
+ * Return: 0 on success, otherwise error
+ */
+int klp_enable_patch(struct klp_patch *patch)
+{
+	int ret;
+

kernel/livepatch/core.c new file +41 -0

+	struct module *mod = obj->mod;
+	int ret;
+
+	pr_notice("reverting patch '%s' on unloading module '%s'\n",
+		  pmod->name, mod->name);
+	ret = klp_disable_object(obj);
+	if (ret)
+		pr_warn("failed to revert patch '%s' on module '%s' (%d)\n",
+			pmod->name, mod->name, ret);
+	obj->mod = NULL;
+}
+
+static int klp_module_notify(struct notifier_block *nb, unsigned long action,
+			    void *data)
+{
+	struct module *mod = data;
+	struct klp_patch *patch;
+	struct klp_object *obj;
+
+	if (action != MODULE_STATE_COMING && action != MODULE_STATE_GOING)
+		return 0;
+
+	mutex_lock(&klp_mutex);
+	list_for_each_entry(patch, &klp_patches, list) {
+		if (patch->state == KLP_DISABLED)
+			continue;
+		for (obj = patch->objs; obj->funcs; obj++) {
+			if (!obj->name || strcmp(obj->name, mod->name))
+				continue;
+			if (action == MODULE_STATE_COMING) {
+				obj->mod = mod;
+				klp_module_notify_coming(patch->mod, obj);
+			} else /* MODULE_STATE_GOING */
+				klp_module_notify_going(patch->mod, obj);
+			break;
+		}
+	}
+	mutex_unlock(&klp_mutex);
+	return 0;
+}
+

kernel/livepatch/core.c new file +12 -0

+	/* init */
+	patch->state = KLP_DISABLED;
+
+	/* sysfs */
+	ret = kobject_init_and_add(&patch->kobj, &klp_ktype_patch,
+				   klp_root_kobj, patch->mod->name);
+	if (ret)
+		return ret;
+
+	ret = klp_init_objects(patch);
+	if (ret) {
+		kobject_put(&patch->kobj);

Quoted 3 lines · Seth Jennings

This commit introduces code for the live patching core. It implements an ftrace-based mechanism and kernel interface for doing live patching of kernel and kernel module functions.

Quoted 6 lines

(quoted diff) +6 -0

+/* sets obj->mod if object is not vmlinux and module is found */
+static bool klp_find_object_module(struct klp_object *obj)
+{
+	if (!obj->name)
+		return 1;
+

Review

Quoted 10 lines

(quoted diff) +10 -0

+	mutex_lock(&module_mutex);
+	/*
+	 * We don't need to take a reference on the module here because we have
+	 * the klp_mutex, which is also taken by the module notifier.  This
+	 * prevents any module from unloading until we release the klp_mutex.
+	 */
+	obj->mod = find_module(obj->name);
+	mutex_unlock(&module_mutex);
+
+	return !!obj->mod;

I know that this is effective to return boolean here because it handles also patch against the kernel core (vmlinux). But the logic looks tricky. I would prefer a cleaner design and use this function only to set obj->mod.

I wanted to see how it would look like, so I will send a patch for this in a separate mail.

From 93eb9f9a25ad8aa0301e246f7685d3e787037566 Mon Sep 17 00:00:00 2001

Quoted 4 lines · Petr Mladek

Quoted 3 lines · Seth Jennings

This commit introduces code for the live patching core. It implements an ftrace-based mechanism and kernel interface for doing live patching of kernel and kernel module functions.

[...]

Quoted 22 lines

Quoted 6 lines

(quoted diff) +6 -0

+/* sets obj->mod if object is not vmlinux and module is found */
+static bool klp_find_object_module(struct klp_object *obj)
+{
+	if (!obj->name)
+		return 1;
+

Review

Quoted 10 lines

(quoted diff) +10 -0

+	mutex_lock(&module_mutex);
+	/*
+	 * We don't need to take a reference on the module here because we have
+	 * the klp_mutex, which is also taken by the module notifier.  This
+	 * prevents any module from unloading until we release the klp_mutex.
+	 */
+	obj->mod = find_module(obj->name);
+	mutex_unlock(&module_mutex);
+
+	return !!obj->mod;

I know that this is effective to return boolean here because it handles also patch against the kernel core (vmlinux). But the logic looks tricky. I would prefer a cleaner design and use this function only to set obj->mod.

Quoted 2 lines

I wanted to see how it would look like, so I will send a patch for this in a separate mail.

The patch is below. Of course, merge it into the original patch if you agree with the idea, please.

Quoted 1 line

From 93eb9f9a25ad8aa0301e246f7685d3e787037566 Mon Sep 17 00:00:00 2001

From: Petr Mladek <[email protected]>
Date: Fri, 28 Nov 2014 15:32:27 +0100
Subject: [PATCH 1/2] livepatch: clean up klp_find_object_module() usage

The function klp_find_object_module() looks quite tricky. It has two effects:
sets obj->mod and decides whether the module is available or not. The second effect is the tricky part because it handles also the code kernel code "vmlinux" and is not module related. It causes returning bool, and doing the crazy double negation.

This patch tries to make a bit cleaner design:

1. klp_find_object_module() handles only obj->mod. It returns the pointer or NULL.

2. It modifies klp_enable_object() to do nothing when the related module has not been loaded yet.

3. The result is that the return value klp_find_object_module() is not used in the end.

We lose a check for potential klp_enable_object() misuse but it makes the code easier. In fact, the check for unloaded module is rather long. We might want to make it easier using some extra flag or another state of the object. Such flag might be used for the check of misuse.

Signed-off-by Petr Mladek <[email protected]>

---
 kernel/livepatch/core.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

kernel/livepatch/core.c +2 -2

index 8e2e8cd242f5..9b1601729014 100644
@@ -40,10 +40,10 @@ static LIST_HEAD(klp_patches);
 static struct kobject *klp_root_kobj;
 
 /* sets obj->mod if object is not vmlinux and module is found */
-static bool klp_find_object_module(struct klp_object *obj)
+static struct module *klp_find_object_module(struct klp_object *obj)
 {
 	if (!obj->name)
-		return 1;
+		return NULL;
 
 	mutex_lock(&module_mutex);
 	/*

Quoted 59 lines · Petr Mladek

Quoted 4 lines · Petr Mladek

Quoted 3 lines · Seth Jennings

This commit introduces code for the live patching core. It implements an ftrace-based mechanism and kernel interface for doing live patching of kernel and kernel module functions.

[...]

Quoted 22 lines

Quoted 6 lines

(quoted diff) +6 -0

+/* sets obj->mod if object is not vmlinux and module is found */
+static bool klp_find_object_module(struct klp_object *obj)
+{
+	if (!obj->name)
+		return 1;
+

Review

Quoted 10 lines

(quoted diff) +10 -0

+	mutex_lock(&module_mutex);
+	/*
+	 * We don't need to take a reference on the module here because we have
+	 * the klp_mutex, which is also taken by the module notifier.  This
+	 * prevents any module from unloading until we release the klp_mutex.
+	 */
+	obj->mod = find_module(obj->name);
+	mutex_unlock(&module_mutex);
+
+	return !!obj->mod;

I know that this is effective to return boolean here because it handles also patch against the kernel core (vmlinux). But the logic looks tricky. I would prefer a cleaner design and use this function only to set obj->mod.

Quoted 2 lines

I wanted to see how it would look like, so I will send a patch for this in a separate mail.

The patch is below. Of course, merge it into the original patch if you agree with the idea, please.

Quoted 1 line

From 93eb9f9a25ad8aa0301e246f7685d3e787037566 Mon Sep 17 00:00:00 2001

From: Petr Mladek <[email protected]>
Date: Fri, 28 Nov 2014 15:32:27 +0100
Subject: [PATCH 1/2] livepatch: clean up klp_find_object_module() usage

The function klp_find_object_module() looks quite tricky. It has two effects:
sets obj->mod and decides whether the module is available or not. The second effect is the tricky part because it handles also the code kernel code "vmlinux" and is not module related. It causes returning bool, and doing the crazy double negation.

This patch tries to make a bit cleaner design:

1. klp_find_object_module() handles only obj->mod. It returns the pointer or NULL.

2. It modifies klp_enable_object() to do nothing when the related module has not been loaded yet.

3. The result is that the return value klp_find_object_module() is not used in the end.

Quoted 6 lines

We lose a check for potential klp_enable_object() misuse but it makes the code easier. In fact, the check for unloaded module is rather long. We might want to make it easier using some extra flag or another state of the object. Such flag might be used for the check of misuse.

Signed-off-by Petr Mladek <[email protected]>

Hi,

I agree with the idea but actually don't like the implementation. I'll try to propose few changes which would hopefully preserve the effect but make the end result slightly better.

Quoted 22 lines

---
 kernel/livepatch/core.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

kernel/livepatch/core.c +2 -2

index 8e2e8cd242f5..9b1601729014 100644
@@ -40,10 +40,10 @@ static LIST_HEAD(klp_patches);
 static struct kobject *klp_root_kobj;
 
 /* sets obj->mod if object is not vmlinux and module is found */
-static bool klp_find_object_module(struct klp_object *obj)
+static struct module *klp_find_object_module(struct klp_object *obj)
 {
 	if (!obj->name)
-		return 1;
+		return NULL;
 
 	mutex_lock(&module_mutex);
 	/*

kernel/livepatch/core.c static bool klp_find_object_module(struct klp_object *obj) · line 58

Quoted 6 lines

kernel/livepatch/core.c +1 -1

@@ -54,7 +54,7 @@ static bool klp_find_object_module(struct klp_object *obj)
 	obj->mod = find_module(obj->name);
 	mutex_unlock(&module_mutex);
 
-	return !!obj->mod;
+	return obj->mod;
 }

As we do not need the return value in the end, we could maybe drop it completely, leading to change in if condition

	if (!obj->name)
		return;

Quoted 5 lines

kernel/livepatch/core.c +0 -0

 struct klp_find_arg {

@@ -318,8 +318,9 @@ static int klp_enable_object(struct module *pmod, struct klp_object *obj)
 	struct klp_func *func;
 	int ret;
 

kernel/livepatch/core.c static int klp_enable_object(struct module *pmod, struct klp_object *obj) · line 323

Quoted 5 lines

kernel/livepatch/core.c +3 -2

-	if (WARN_ON(!obj->mod && obj->name))
-		return -EINVAL;
+	/* nope when the patched module has not been loaded yet */
+	if (obj->name && !obj->mod)
+		return 0;

I have a problem with this one. In case the condition is true we do nothing, return back and pretend that everything is alright (return 0). I see that we would call klp_enable_object everytime in your proposal and decide here whether we want to do something or not. But I think that we should return some error and deal with it in the caller function. Thus original WARN_ON should stay here.

kernel/livepatch/core.c static int __klp_enable_patch(struct klp_patch *patch) · line 408

Quoted 12 lines

kernel/livepatch/core.c +1 -2

 	if (obj->relocs) {
 		ret = klp_write_object_relocations(pmod, obj);

@@ -401,8 +402,7 @@ static int __klp_enable_patch(struct klp_patch *patch)
 	pr_notice("enabling patch '%s'\n", patch->mod->name);
 
 	for (obj = patch->objs; obj->funcs; obj++) {
-		if (!klp_find_object_module(obj))
-			continue;
+		klp_find_object_module(obj);
 		ret = klp_enable_object(patch->mod, obj);
 		if (ret)
 			goto unregister;

I propose this piece of code

for (obj = patch->objs; obj->funcs; obj++) {
	klp_find_object_module(obj);
	if (obj->name && !obj->mod)
		continue;
	ret = klp_enable_object(patch->mod, obj);
	...
}

What do you think? Also it could pay off to define inline function for the check. Somethink like klp_is_module_and_loaded...

Quoted 3 lines · Seth Jennings

This commit introduces code for the live patching core. It implements an ftrace-based mechanism and kernel interface for doing live patching of kernel and kernel module functions.

Review

Quoted 5 lines

(quoted diff) +5 -0

+static int klp_enable_object(struct module *pmod, struct klp_object *obj)
+{
+	struct klp_func *func;
+	int ret;
+

	if (WARN_ON(obj->state != KLP_DISABLED))
		return -EINVAL;

Review

Quoted 5 lines

(quoted diff) +5 -0

+	if (WARN_ON(!obj->mod && obj->name))
+		return -EINVAL;
+
+	if (obj->relocs) {
+		ret = klp_write_object_relocations(pmod, obj);

I was curious why the relocation is here. I think that the motivation was to safe the call when handling comming modules.

IMHO, more clean solution would be to do this in klp_init_object(). It will also cause removing the "pmod" parameter and this function will be more symmetric with klp_disable_object();

I was curious how it would work, so I prepared a patch. I will send it in separate mail.

From ba3b08938b6f6f1a041af645ff43825f2ec1222f Mon Sep 17 00:00:00 2001

This commit introduces code for the live patching core. It implements an ftrace-based mechanism and kernel interface for doing live patching of kernel and kernel module functions.

It represents the greatest common functionality set between kpatch and kgraft and can accept patches built using either method.

This first version does not implement any consistency mechanism that ensures that old and new code do not run together. In practice, ~90% of CVEs are safe to apply in this way, since they simply add a conditional check. However, any function change that can not execute safely with the old version of the function can _not_ be safely applied in this version.

Signed-off-by Seth Jennings <[email protected]>

---

include/linux/livepatch.h new file +102 -0

new file mode 100644
index 0000000..4e01b59
@@ -0,0 +1,121 @@
+/*
+ * livepatch.h - Kernel Live Patching Core
+ *
+ * Copyright (C) 2014 Seth Jennings <[email protected]>
+ *
+ * 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/>.
+ */
+
+#ifndef _LINUX_LIVEPATCH_H_
+#define _LINUX_LIVEPATCH_H_
+
+#include <linux/module.h>
+#include <linux/ftrace.h>
+
+#if IS_ENABLED(CONFIG_LIVE_PATCHING)
+
+#include <asm/livepatch.h>
+
+enum klp_state {
+	KLP_DISABLED,
+	KLP_ENABLED
+};
+
+/**
+ * struct klp_func - function structure for live patching
+ * @old_name:	name of the function to be patched
+ * @new_func:	pointer to the patched function code
+ * @old_addr:	a hint conveying at what address the old function
+ *		can be found (optional, vmlinux patches only)
+ */
+struct klp_func {
+	/* external */
+	const char *old_name;
+	void *new_func;
+	/*
+	 * The old_addr field is optional and can be used to resolve
+	 * duplicate symbol names in the vmlinux object.  If this
+	 * information is not present, the symbol is located by name
+	 * with kallsyms. If the name is not unique and old_addr is
+	 * not provided, the patch application fails as there is no
+	 * way to resolve the ambiguity.
+	 */
+	unsigned long old_addr;
+
+	/* internal */
+	struct kobject kobj;
+	struct ftrace_ops *fops;
+	enum klp_state state;
+};
+
+/**
+ * struct klp_reloc - relocation structure for live patching
+ * @dest	address where the relocation will be written
+ * @src		address of the referenced symbol (optional,
+ *		vmlinux	patches only)
+ * @type	ELF relocation type
+ * @name	name of the referenced symbol (for lookup/verification)
+ * @addend	offset from the referenced symbol
+ * @external	symbol is either exported or within the	live patch module itself
+ */
+struct klp_reloc {
+	unsigned long dest;
+	unsigned long src;
+	unsigned long type;
+	const char *name;
+	int addend;
+	int external;
+};
+
+/* struct klp_object - kernel object structure for live patching
+ * @name	module name (or NULL for vmlinux)
+ * @relocs	relocation entries to be applied at load time
+ * @funcs	function entries for functions to be patched in the object
+ */
+struct klp_object {
+	/* external */
+	const char *name;
+	struct klp_reloc *relocs;
+	struct klp_func *funcs;
+
+	/* internal */
+	struct kobject *kobj;
+	struct module *mod;
+	enum klp_state state;
+};
+
+/**
+ * struct klp_patch - patch structure for live patching
+ * @mod		reference to the live patch module
+ * @objs	object entries for kernel objects to be patched
+ */

From 0e925c303acf8b596ba8941417e1c3991cdb0adc Mon Sep 17 00:00:00 2001

Add a sample live patching module.

Signed-off-by Seth Jennings <[email protected]>

---
 MAINTAINERS                          |  1 +
 samples/Kconfig                      |  7 +++
 samples/livepatch/Makefile           |  1 +
 samples/livepatch/livepatch-sample.c | 87 ++++++++++++++++++++++++++++++++++++
 4 files changed, 96 insertions(+)
 create mode 100644 samples/livepatch/Makefile
 create mode 100644 samples/livepatch/livepatch-sample.c

Quoted 11 lines · Seth Jennings

Add a sample live patching module.

Signed-off-by Seth Jennings <[email protected]>

---
 MAINTAINERS                          |  1 +
 samples/Kconfig                      |  7 +++
 samples/livepatch/Makefile           |  1 +
 samples/livepatch/livepatch-sample.c | 87 ++++++++++++++++++++++++++++++++++++
 4 files changed, 96 insertions(+)
 create mode 100644 samples/livepatch/Makefile
 create mode 100644 samples/livepatch/livepatch-sample.c

[...]

samples/livepatch/livepatch-sample.c

Quoted 10 lines

samples/livepatch/Makefile new file +1 -0

new file mode 100644
index 0000000..7f1cdc1
@@ -0,0 +1 @@
+obj-$(CONFIG_SAMPLE_LIVE_PATCHING) += livepatch-sample.o

samples/livepatch/livepatch-sample.c new file +0 -0

new file mode 100644
index 0000000..21f159d

The Makefile is ignored because there is missing:

samples/Makefile +0 -0

index 1a60c62e2045..f00257bcc5a7 100644

Best Regards,
Petr
--
To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Masami's IPMODIFY patch is heading for -next via your tree. Once it arrives, I'll rebase and make the change to set IPMODIFY. Do not pull this for -next yet. This version (v4) is for review and gathering acks.

Quoted 3 lines · Seth Jennings

Masami's IPMODIFY patch is heading for -next via your tree. Once it arrives, I'll rebase and make the change to set IPMODIFY. Do not pull this for -next yet. This version (v4) is for review and gathering acks.

Thanks for sending out v4 and incorporating the feedback, I really appreciate your responsiveness!

Anyway, I don't think targetting 3.19 is realistic, given we're currently already past 3.18-rc6 ... even if we rush it into -next in the coming days, it will get close to zero exposure in there before the merge window opens.

I'd like to do quite some more testing and still finish some pending portions of code reviews on our side (especially to make sure that this can be easily extended to support any consistency model in the future).

Once we start collecting Reviewed-by's / Acked-by's on this patchset, I can establish a tree on git.kernel.org that we can use to collect any followup patches during 3.20 development cycle and send a pull request to Linus during 3.20 merge window .. if everybody agrees with this course of action, obviously.

Thanks,

Signature · 2 lines

Jiri Kosina
SUSE Labs

Quoted 3 lines

I'd like to do quite some more testing and still finish some pending portions of code reviews on our side (especially to make sure that this can be easily extended to support any consistency model in the future).

Without knowing how that consistency code will look, how can we "make sure" that this code can be easily extended to support it?

Quoted 5 lines

Once we start collecting Reviewed-by's / Acked-by's on this patchset, I can establish a tree on git.kernel.org that we can use to collect any followup patches during 3.20 development cycle and send a pull request to Linus during 3.20 merge window .. if everybody agrees with this course of action, obviously.

I was hoping this first step would go into next via Steve's tree and go upstream for 3.20 (hopefully) from there. I would be against anything that tries to expand the feature set before this base functionality gets upstream.

However, if we want to have a tree to gather fixes before 3.20, which I think is what you are suggesting, that works for me.

We would need to agree explicitly that, in this tree, patches would need both a RH and SUSE ack to be accepted.

Note to Steve:
Masami's IPMODIFY patch is heading for -next via your tree. Once it arrives, I'll rebase and make the change to set IPMODIFY. Do not pull this for -next yet. This version (v4) is for review and gathering acks.

Quoted 4 lines

Note to Steve:
Masami's IPMODIFY patch is heading for -next via your tree. Once it arrives, I'll rebase and make the change to set IPMODIFY. Do not pull this for -next yet. This version (v4) is for review and gathering acks.

BTW, as we discussed IPMODIFY is an exclusive flag. So if we allocate ftrace_ops for each function in each patch, it could be conflict each other.

Maybe we need to have another ops hashtable to find such conflict and new handler to handle it.

What I'd propose instead is to make sure that we always have just a ftrace_ops per function entry, and only update the pointers there

Quoted 8 lines · Masami Hiramatsu

Quoted 4 lines

Note to Steve:
Masami's IPMODIFY patch is heading for -next via your tree. Once it arrives, I'll rebase and make the change to set IPMODIFY. Do not pull this for -next yet. This version (v4) is for review and gathering acks.

BTW, as we discussed IPMODIFY is an exclusive flag. So if we allocate ftrace_ops for each function in each patch, it could be conflict each other.

Yup, this corresponds to what Petr brought up yesterday. There are cases where all solutions (kpatch, kgraft, klp) would allocate multiple ftrace_ops for a single function entry (think of patching one function multiple times in a row).

So it's not as easy as just setting the flag.

Quoted 2 lines

Maybe we need to have another ops hashtable to find such conflict and new handler to handle it.

If I understand your proposal correctly, that would sound like a hackish workaround, trying to basically trick the IPMODIFY flag semantics you just implemented :)

What I'd propose instead is to make sure that we always have just a ftrace_ops per function entry, and only update the pointers there as necessary. Fortunately we can do the switch atomically, by making use of ->private.

Quoted 21 lines · Jiri Kosina

Quoted 8 lines · Masami Hiramatsu

Quoted 4 lines

Note to Steve:
Masami's IPMODIFY patch is heading for -next via your tree. Once it arrives, I'll rebase and make the change to set IPMODIFY. Do not pull this for -next yet. This version (v4) is for review and gathering acks.

BTW, as we discussed IPMODIFY is an exclusive flag. So if we allocate ftrace_ops for each function in each patch, it could be conflict each other.

Yup, this corresponds to what Petr brought up yesterday. There are cases where all solutions (kpatch, kgraft, klp) would allocate multiple ftrace_ops for a single function entry (think of patching one function multiple times in a row).

So it's not as easy as just setting the flag.

Quoted 2 lines

Maybe we need to have another ops hashtable to find such conflict and new handler to handle it.

Quoted 3 lines

If I understand your proposal correctly, that would sound like a hackish workaround, trying to basically trick the IPMODIFY flag semantics you just implemented :)

I think Masami may be proposing something similar to what we do in kpatch today. We have a single ftrace_ops and handler which is used for all functions. The handler accesses a global hash of kpatch_func structs which is indexed by the original function's IP address.

It actually works out pretty well because it nicely encapsulates the knowledge about which functions are patched in a single place. And it makes it easy to track function versions (for incremental patching and rollback).

Quoted 4 lines

What I'd propose instead is to make sure that we always have just a ftrace_ops per function entry, and only update the pointers there as necessary. Fortunately we can do the switch atomically, by making use of ->private.

But how would you update multiple functions atomically, to enforce per-thread consistency?

Quoted 22 lines · Josh Poimboeuf

Quoted 21 lines · Jiri Kosina

Quoted 8 lines · Masami Hiramatsu

Quoted 4 lines

Note to Steve:
Masami's IPMODIFY patch is heading for -next via your tree. Once it arrives, I'll rebase and make the change to set IPMODIFY. Do not pull this for -next yet. This version (v4) is for review and gathering acks.

BTW, as we discussed IPMODIFY is an exclusive flag. So if we allocate ftrace_ops for each function in each patch, it could be conflict each other.

Yup, this corresponds to what Petr brought up yesterday. There are cases where all solutions (kpatch, kgraft, klp) would allocate multiple ftrace_ops for a single function entry (think of patching one function multiple times in a row).

So it's not as easy as just setting the flag.

Quoted 2 lines

Maybe we need to have another ops hashtable to find such conflict and new handler to handle it.

Quoted 8 lines

Quoted 3 lines

If I understand your proposal correctly, that would sound like a hackish workaround, trying to basically trick the IPMODIFY flag semantics you just implemented :)

I think Masami may be proposing something similar to what we do in kpatch today. We have a single ftrace_ops and handler which is used for all functions. The handler accesses a global hash of kpatch_func structs which is indexed by the original function's IP address.

Hmm, I think both is OK. kpatch method is less memory consuming and will have a bigger overhead. However, as Steven talked at Plumbers Conf., he will introduce a direct code modifying interface for ftrace. After that is introduced, we don't need to care about performance degradation by patching :)

Quoted 10 lines

It actually works out pretty well because it nicely encapsulates the knowledge about which functions are patched in a single place. And it makes it easy to track function versions (for incremental patching and rollback).

Quoted 4 lines

What I'd propose instead is to make sure that we always have just a ftrace_ops per function entry, and only update the pointers there as necessary. Fortunately we can do the switch atomically, by making use of ->private.

Quoted 2 lines

But how would you update multiple functions atomically, to enforce per-thread consistency?

At this point, both can do it atomically. We just need an atomic flag for applying patches.

Quoted 32 lines · Masami Hiramatsu

Quoted 22 lines · Josh Poimboeuf

Quoted 21 lines · Jiri Kosina

Quoted 8 lines · Masami Hiramatsu

Quoted 4 lines

Note to Steve:
Masami's IPMODIFY patch is heading for -next via your tree. Once it arrives, I'll rebase and make the change to set IPMODIFY. Do not pull this for -next yet. This version (v4) is for review and gathering acks.

BTW, as we discussed IPMODIFY is an exclusive flag. So if we allocate ftrace_ops for each function in each patch, it could be conflict each other.

Yup, this corresponds to what Petr brought up yesterday. There are cases where all solutions (kpatch, kgraft, klp) would allocate multiple ftrace_ops for a single function entry (think of patching one function multiple times in a row).

So it's not as easy as just setting the flag.

Quoted 2 lines

Maybe we need to have another ops hashtable to find such conflict and new handler to handle it.

Quoted 8 lines

Quoted 3 lines

If I understand your proposal correctly, that would sound like a hackish workaround, trying to basically trick the IPMODIFY flag semantics you just implemented :)

I think Masami may be proposing something similar to what we do in kpatch today. We have a single ftrace_ops and handler which is used for all functions. The handler accesses a global hash of kpatch_func structs which is indexed by the original function's IP address.

Quoted 5 lines

Hmm, I think both is OK. kpatch method is less memory consuming and will have a bigger overhead. However, as Steven talked at Plumbers Conf., he will introduce a direct code modifying interface for ftrace. After that is introduced, we don't need to care about performance degradation by patching :)

Yup, I would prefer to have ftrace_ops per (original) function entry. I mean that new patches will reuse the existing ftrace_ops for already patched functions. They will just create new ftrace_ops from the not-yet-patched symbols.

Using a single ftrace_ops everywhere would kill the win from Steven's direct ftrace optimization.

Quoted 10 lines

Quoted 10 lines

It actually works out pretty well because it nicely encapsulates the knowledge about which functions are patched in a single place. And it makes it easy to track function versions (for incremental patching and rollback).

Quoted 4 lines

What I'd propose instead is to make sure that we always have just a ftrace_ops per function entry, and only update the pointers there as necessary. Fortunately we can do the switch atomically, by making use of ->private.

Quoted 5 lines

Quoted 2 lines

But how would you update multiple functions atomically, to enforce per-thread consistency?

At this point, both can do it atomically. We just need an atomic flag for applying patches.

By other words, we would need something like the "kgr_immutable" flag from kGraft. It will make sure that everybody stays with the current code until all function entries are updated.

Best Regards,
Petr

Quoted 8 lines · Masami Hiramatsu

Quoted 4 lines

Note to Steve:
Masami's IPMODIFY patch is heading for -next via your tree. Once it arrives, I'll rebase and make the change to set IPMODIFY. Do not pull this for -next yet. This version (v4) is for review and gathering acks.

BTW, as we discussed IPMODIFY is an exclusive flag. So if we allocate ftrace_ops for each function in each patch, it could be conflict each other.

Yup, this corresponds to what Petr brought up yesterday. There are cases where all solutions (kpatch, kgraft, klp) would allocate multiple ftrace_ops for a single function entry (think of patching one function multiple times in a row).

So it's not as easy as just setting the flag.

Quoted 2 lines

Maybe we need to have another ops hashtable to find such conflict and new handler to handle it.

If I understand your proposal correctly, that would sound like a hackish workaround, trying to basically trick the IPMODIFY flag semantics you just implemented :)

What I'd propose instead is to make sure that we always have just a ftrace_ops per function entry, and only update the pointers there as necessary. Fortunately we can do the switch atomically, by making use of ->private.

Quoted 4 lines · Seth Jennings

Note to Steve:
Masami's IPMODIFY patch is heading for -next via your tree. Once it arrives, I'll rebase and make the change to set IPMODIFY. Do not pull this for -next yet. This version (v4) is for review and gathering acks.

BTW, as we discussed IPMODIFY is an exclusive flag. So if we allocate ftrace_ops for each function in each patch, it could be conflict each other. Maybe we need to have another ops hashtable to find such conflict and new handler to handle it.