LinuxLists.cc - [PATCHv7 0/3] Kernel Live Patching

[permalink] [raw]

Subject: [PATCHv7 1/3] kernel: add TAINT_LIVEPATCH

This adds a new taint flag to indicate when the kernel or a kernel
module has been live patched. This will provide a clean indication in
bug reports that live patching was used.

Additionally, if the crash occurs in a live patched function, the live
patch module will appear beside the patched function in the backtrace.

Signed-off-by: Seth Jennings <[email protected]>
---
Documentation/oops-tracing.txt | 2 ++
Documentation/sysctl/kernel.txt | 1 +
include/linux/kernel.h | 1 +
kernel/panic.c | 2 ++
4 files changed, 6 insertions(+)

diff --git a/Documentation/oops-tracing.txt b/Documentation/oops-tracing.txt
index beefb9f..f3ac05c 100644
--- a/Documentation/oops-tracing.txt
+++ b/Documentation/oops-tracing.txt
@@ -270,6 +270,8 @@ characters, each representing a particular tainted value.

15: 'L' if a soft lockup has previously occurred on the system.

+ 16: 'K' if the kernel has been live patched.
+
The primary reason for the 'Tainted: ' string is to tell kernel
debuggers if this is a clean kernel or if anything unusual has
occurred. Tainting is permanent: even if an offending module is
diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 75511ef..83ab256 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -843,6 +843,7 @@ can be ORed together:
8192 - An unsigned module has been loaded in a kernel supporting module
signature.
16384 - A soft lockup has previously occurred on the system.
+32768 - The kernel has been live patched.

==============================================================

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 5449d2f..d03e3de 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -471,6 +471,7 @@ extern enum system_states {
#define TAINT_OOT_MODULE 12
#define TAINT_UNSIGNED_MODULE 13
#define TAINT_SOFTLOCKUP 14
+#define TAINT_LIVEPATCH 15

extern const char hex_asc[];
#define hex_asc_lo(x) hex_asc[((x) & 0x0f)]
diff --git a/kernel/panic.c b/kernel/panic.c
index 4d8d6f9..8136ad7 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -226,6 +226,7 @@ static const struct tnt tnts[] = {
{ TAINT_OOT_MODULE, 'O', ' ' },
{ TAINT_UNSIGNED_MODULE, 'E', ' ' },
{ TAINT_SOFTLOCKUP, 'L', ' ' },
+ { TAINT_LIVEPATCH, 'K', ' ' },
};

/**
@@ -246,6 +247,7 @@ static const struct tnt tnts[] = {
* 'O' - Out-of-tree module has been loaded.
* 'E' - Unsigned module has been loaded.
* 'L' - A soft lockup has previously occurred.
+ * 'K' - Kernel has been live patched.
*
* The string is overwritten by the next call to print_tainted().
*/
--
2.1.0

2014-12-16 17:59:19

[permalink] [raw]

Subject: [PATCHv7 2/3] kernel: add support for 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.

Signed-off-by: Seth Jennings <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
Documentation/ABI/testing/sysfs-kernel-livepatch | 44 ++
MAINTAINERS | 13 +
arch/x86/Kconfig | 3 +
arch/x86/include/asm/livepatch.h | 36 +
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/livepatch.c | 89 +++
include/linux/livepatch.h | 132 ++++
kernel/Makefile | 1 +
kernel/livepatch/Kconfig | 18 +
kernel/livepatch/Makefile | 3 +
kernel/livepatch/core.c | 929 +++++++++++++++++++++++
11 files changed, 1269 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-kernel-livepatch
create mode 100644 arch/x86/include/asm/livepatch.h
create mode 100644 arch/x86/kernel/livepatch.c
create mode 100644 include/linux/livepatch.h
create mode 100644 kernel/livepatch/Kconfig
create mode 100644 kernel/livepatch/Makefile
create mode 100644 kernel/livepatch/core.c

diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch b/Documentation/ABI/testing/sysfs-kernel-livepatch
new file mode 100644
index 0000000..5bf42a8
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-kernel-livepatch
@@ -0,0 +1,44 @@
+What: /sys/kernel/livepatch
+Date: Nov 2014
+KernelVersion: 3.19.0
+Contact: [email protected]
+Description:
+ Interface for kernel live patching
+
+ The /sys/kernel/livepatch directory contains subdirectories for
+ each loaded live patch module.
+
+What: /sys/kernel/livepatch/<patch>
+Date: Nov 2014
+KernelVersion: 3.19.0
+Contact: [email protected]
+Description:
+ The patch directory contains subdirectories for each kernel
+ object (vmlinux or a module) in which it patched functions.
+
+What: /sys/kernel/livepatch/<patch>/enabled
+Date: Nov 2014
+KernelVersion: 3.19.0
+Contact: [email protected]
+Description:
+ A writable attribute that indicates whether the patched
+ code is currently applied. Writing 0 will disable the patch
+ while writing 1 will re-enable the patch.
+
+What: /sys/kernel/livepatch/<patch>/<object>
+Date: Nov 2014
+KernelVersion: 3.19.0
+Contact: [email protected]
+Description:
+ The object directory contains subdirectories for each function
+ that is patched within the object.
+
+What: /sys/kernel/livepatch/<patch>/<object>/<function>
+Date: Nov 2014
+KernelVersion: 3.19.0
+Contact: [email protected]
+Description:
+ The function directory contains attributes regarding the
+ properties and state of the patched function.
+
+ There are currently no such attributes.
diff --git a/MAINTAINERS b/MAINTAINERS
index 06d5b4b..94f9f69 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5777,6 +5777,19 @@ F: Documentation/misc-devices/lis3lv02d
F: drivers/misc/lis3lv02d/
F: drivers/platform/x86/hp_accel.c

+LIVE PATCHING
+M: Josh Poimboeuf <[email protected]>
+M: Seth Jennings <[email protected]>
+M: Jiri Kosina <[email protected]>
+M: Vojtech Pavlik <[email protected]>
+S: Maintained
+F: kernel/livepatch/
+F: include/linux/livepatch.h
+F: arch/x86/include/asm/livepatch.h
+F: arch/x86/kernel/livepatch.c
+F: Documentation/ABI/testing/sysfs-kernel-livepatch
+L: [email protected]
+
LLC (802.2)
M: Arnaldo Carvalho de Melo <[email protected]>
S: Maintained
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ce8dcdf..5c57181 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -17,6 +17,7 @@ config X86_64
depends on 64BIT
select X86_DEV_DMA_OPS
select ARCH_USE_CMPXCHG_LOCKREF
+ select ARCH_HAVE_LIVE_PATCHING

### Arch settings
config X86
@@ -1986,6 +1987,8 @@ config CMDLINE_OVERRIDE
This is used to work around broken boot loaders. This should
be set to 'N' under normal conditions.

+source "kernel/livepatch/Kconfig"
+
endmenu

config ARCH_ENABLE_MEMORY_HOTPLUG
diff --git a/arch/x86/include/asm/livepatch.h b/arch/x86/include/asm/livepatch.h
new file mode 100644
index 0000000..c2ae592
--- /dev/null
+++ b/arch/x86/include/asm/livepatch.h
@@ -0,0 +1,36 @@
+/*
+ * 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
+extern int klp_write_module_reloc(struct module *mod, unsigned long type,
+ unsigned long loc, unsigned long value);
+
+#else
+#error Live patching support is disabled; check CONFIG_LIVE_PATCHING
+#endif
+
+#endif /* _ASM_X86_LIVEPATCH_H */
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 5d4502c..316b34e 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_X86_MPPARSE) += mpparse.o
obj-y += apic/
obj-$(CONFIG_X86_REBOOTFIXUPS) += reboot_fixups_32.o
obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o
+obj-$(CONFIG_LIVE_PATCHING) += livepatch.o
obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o
obj-$(CONFIG_FTRACE_SYSCALLS) += ftrace.o
obj-$(CONFIG_X86_TSC) += trace_clock.o
diff --git a/arch/x86/kernel/livepatch.c b/arch/x86/kernel/livepatch.c
new file mode 100644
index 0000000..4b0ed7b
--- /dev/null
+++ b/arch/x86/kernel/livepatch.c
@@ -0,0 +1,89 @@
+/*
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/module.h>
+#include <linux/uaccess.h>
+#include <asm/cacheflush.h>
+#include <asm/page_types.h>
+#include <asm/elf.h>
+#include <asm/livepatch.h>
+
+/**
+ * klp_write_module_reloc() - write a relocation in a module
+ * @mod: module in which the section to be modified is found
+ * @type: ELF relocation type (see asm/elf.h)
+ * @loc: address that the relocation should be written to
+ * @value: relocation value (sym address + addend)
+ *
+ * This function writes a relocation to the specified location for
+ * a particular module.
+ */
+int klp_write_module_reloc(struct module *mod, unsigned long type,
+ unsigned long loc, unsigned long value)
+{
+ int ret, numpages, size = 4;
+ bool readonly;
+ 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;
+ }
+
+ if (loc < core || loc >= core + core_size)
+ /* loc does not point to any symbol inside the module */
+ return -EINVAL;
+
+ if (loc < core + core_ro_size)
+ readonly = true;
+ else
+ readonly = false;
+
+ /* determine if the relocation spans a page boundary */
+ 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;
+}
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
new file mode 100644
index 0000000..b61fe74
--- /dev/null
+++ b/include/linux/livepatch.h
@@ -0,0 +1,132 @@
+/*
+ * 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)
+ * @kobj: kobject for sysfs resources
+ * @fops: ftrace operations structure
+ * @state: tracks function-level patch application state
+ */
+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
+ * @loc: address where the relocation will be written
+ * @val: 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 loc;
+ unsigned long val;
+ 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
+ * @kobj: kobject for sysfs resources
+ * @mod: kernel module associated with the patched object
+ * (NULL for vmlinux)
+ * @state: tracks object-level patch application state
+ */
+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
+ * @list: list node for global list of registered patches
+ * @kobj: kobject for sysfs resources
+ * @state: tracks patch-level application state
+ */
+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_ */
diff --git a/kernel/Makefile b/kernel/Makefile
index a59481a..616994f 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -26,6 +26,7 @@ obj-y += power/
obj-y += printk/
obj-y += irq/
obj-y += rcu/
+obj-y += livepatch/

obj-$(CONFIG_CHECKPOINT_RESTORE) += kcmp.o
obj-$(CONFIG_FREEZER) += freezer.o
diff --git a/kernel/livepatch/Kconfig b/kernel/livepatch/Kconfig
new file mode 100644
index 0000000..96da00f
--- /dev/null
+++ b/kernel/livepatch/Kconfig
@@ -0,0 +1,18 @@
+config ARCH_HAVE_LIVE_PATCHING
+ boolean
+ help
+ Arch supports kernel live patching
+
+config LIVE_PATCHING
+ boolean "Kernel Live Patching"
+ depends on DYNAMIC_FTRACE_WITH_REGS
+ depends on MODULES
+ depends on SYSFS
+ depends on KALLSYMS_ALL
+ depends on ARCH_HAVE_LIVE_PATCHING
+ help
+ Say Y here if you want to support kernel live patching.
+ This option has no runtime impact until a kernel "patch"
+ module uses the interface provided by this option to register
+ a patch, causing calls to patched functions to be redirected
+ to new function code contained in the patch module.
diff --git a/kernel/livepatch/Makefile b/kernel/livepatch/Makefile
new file mode 100644
index 0000000..7c1f008
--- /dev/null
+++ b/kernel/livepatch/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_LIVE_PATCHING) += livepatch.o
+
+livepatch-objs := core.o
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
new file mode 100644
index 0000000..0004a71
--- /dev/null
+++ b/kernel/livepatch/core.c
@@ -0,0 +1,929 @@
+/*
+ * 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;
+
+static bool klp_is_module(struct klp_object *obj)
+{
+ return obj->name;
+}
+
+static bool klp_is_object_loaded(struct klp_object *obj)
+{
+ return !obj->name || obj->mod;
+}
+
+/* sets obj->mod if object is not vmlinux and module is found */
+static void klp_find_object_module(struct klp_object *obj)
+{
+ if (!klp_is_module(obj))
+ return;
+
+ 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);
+}
+
+/* klp_mutex must be held by caller */
+static bool klp_is_patch_registered(struct klp_patch *patch)
+{
+ struct klp_patch *mypatch;
+
+ list_for_each_entry(mypatch, &klp_patches, list)
+ if (mypatch == patch)
+ return true;
+
+ return false;
+}
+
+static bool klp_initialized(void)
+{
+ return klp_root_kobj;
+}
+
+struct klp_find_arg {
+ const char *objname;
+ const char *name;
+ unsigned long addr;
+ /*
+ * If count == 0, the symbol was not found. If count == 1, a unique
+ * match was found and addr is set. If count > 1, there is
+ * unresolvable ambiguity among "count" number of symbols with the same
+ * name in the same object.
+ */
+ unsigned long count;
+};
+
+static int klp_find_callback(void *data, const char *name,
+ struct module *mod, unsigned long addr)
+{
+ struct klp_find_arg *args = data;
+
+ if ((mod && !args->objname) || (!mod && args->objname))
+ return 0;
+
+ if (strcmp(args->name, name))
+ return 0;
+
+ if (args->objname && strcmp(args->objname, mod->name))
+ return 0;
+
+ /*
+ * args->addr might be overwritten if another match is found
+ * but klp_find_object_symbol() handles this and only returns the
+ * addr if count == 1.
+ */
+ args->addr = addr;
+ args->count++;
+
+ return 0;
+}
+
+static int klp_find_object_symbol(const char *objname, const char *name,
+ unsigned long *addr)
+{
+ struct klp_find_arg args = {
+ .objname = objname,
+ .name = name,
+ .addr = 0,
+ .count = 0
+ };
+
+ kallsyms_on_each_symbol(klp_find_callback, &args);
+
+ if (args.count == 0)
+ pr_err("symbol '%s' not found in symbol table\n", name);
+ else if (args.count > 1)
+ pr_err("unresolvable ambiguity (%lu matches) on symbol '%s' in object '%s'\n",
+ args.count, name, objname);
+ else {
+ *addr = args.addr;
+ return 0;
+ }
+
+ *addr = 0;
+ return -EINVAL;
+}
+
+struct klp_verify_args {
+ const char *name;
+ const unsigned long addr;
+};
+
+static int klp_verify_callback(void *data, const char *name,
+ struct module *mod, unsigned long addr)
+{
+ struct klp_verify_args *args = data;
+
+ if (!mod &&
+ !strcmp(args->name, name) &&
+ args->addr == addr)
+ return 1;
+
+ return 0;
+}
+
+static int klp_verify_vmlinux_symbol(const char *name, unsigned long addr)
+{
+ struct klp_verify_args args = {
+ .name = name,
+ .addr = addr,
+ };
+
+ if (kallsyms_on_each_symbol(klp_verify_callback, &args))
+ return 0;
+
+ pr_err("symbol '%s' not found at specified address 0x%016lx, kernel mismatch?",
+ name, addr);
+ return -EINVAL;
+}
+
+static int klp_find_verify_func_addr(struct klp_object *obj,
+ struct klp_func *func)
+{
+ int ret;
+
+#if defined(CONFIG_RANDOMIZE_BASE)
+ /* KASLR is enabled, disregard old_addr from user */
+ func->old_addr = 0;
+#endif
+
+ if (!func->old_addr || klp_is_module(obj))
+ ret = klp_find_object_symbol(obj->name, func->old_name,
+ &func->old_addr);
+ else
+ ret = klp_verify_vmlinux_symbol(func->old_name,
+ func->old_addr);
+
+ return ret;
+}
+
+/*
+ * external symbols are located outside the parent object (where the parent
+ * object is either vmlinux or the kmod being patched).
+ */
+static int klp_find_external_symbol(struct module *pmod, const char *name,
+ unsigned long *addr)
+{
+ const struct kernel_symbol *sym;
+
+ /* first, check if it's an exported symbol */
+ preempt_disable();
+ sym = find_symbol(name, NULL, NULL, true, true);
+ preempt_enable();
+ if (sym) {
+ *addr = sym->value;
+ return 0;
+ }
+
+ /* otherwise check if it's in another .o within the patch module */
+ return klp_find_object_symbol(pmod->name, name, addr);
+}
+
+static int klp_write_object_relocations(struct module *pmod,
+ struct klp_object *obj)
+{
+ int ret;
+ struct klp_reloc *reloc;
+
+ if (WARN_ON(!klp_is_object_loaded(obj)))
+ return -EINVAL;
+
+ if (WARN_ON(!obj->relocs))
+ return -EINVAL;
+
+ for (reloc = obj->relocs; reloc->name; reloc++) {
+ if (!klp_is_module(obj)) {
+ ret = klp_verify_vmlinux_symbol(reloc->name,
+ reloc->val);
+ if (ret)
+ return ret;
+ } else {
+ /* module, reloc->val needs to be discovered */
+ if (reloc->external)
+ ret = klp_find_external_symbol(pmod,
+ reloc->name,
+ &reloc->val);
+ else
+ ret = klp_find_object_symbol(obj->mod->name,
+ reloc->name,
+ &reloc->val);
+ if (ret)
+ return ret;
+ }
+ ret = klp_write_module_reloc(pmod, reloc->type, reloc->loc,
+ reloc->val + reloc->addend);
+ if (ret) {
+ pr_err("relocation failed for symbol '%s' at 0x%016lx (%d)\n",
+ reloc->name, reloc->val, ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static void notrace klp_ftrace_handler(unsigned long ip,
+ unsigned long parent_ip,
+ struct ftrace_ops *ops,
+ struct pt_regs *regs)
+{
+ struct klp_func *func = ops->private;
+
+ regs->ip = (unsigned long)func->new_func;
+}
+
+static int klp_disable_func(struct klp_func *func)
+{
+ int ret;
+
+ if (WARN_ON(func->state != KLP_ENABLED))
+ return -EINVAL;
+
+ if (WARN_ON(!func->old_addr))
+ return -EINVAL;
+
+ ret = unregister_ftrace_function(func->fops);
+ if (ret) {
+ pr_err("failed to unregister ftrace handler for function '%s' (%d)\n",
+ func->old_name, ret);
+ return ret;
+ }
+
+ ret = ftrace_set_filter_ip(func->fops, func->old_addr, 1, 0);
+ if (ret)
+ pr_warn("function unregister succeeded but failed to clear the filter\n");
+
+ func->state = KLP_DISABLED;
+
+ return 0;
+}
+
+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",
+ func->old_name, ret);
+ return ret;
+ }
+
+ ret = register_ftrace_function(func->fops);
+ if (ret) {
+ pr_err("failed to register ftrace handler for function '%s' (%d)\n",
+ func->old_name, ret);
+ ftrace_set_filter_ip(func->fops, func->old_addr, 1, 0);
+ } else {
+ func->state = KLP_ENABLED;
+ }
+
+ return ret;
+}
+
+static int klp_disable_object(struct klp_object *obj)
+{
+ struct klp_func *func;
+ int ret;
+
+ for (func = obj->funcs; func->old_name; func++) {
+ if (func->state != KLP_ENABLED)
+ continue;
+
+ ret = klp_disable_func(func);
+ if (ret)
+ return ret;
+ }
+
+ obj->state = KLP_DISABLED;
+
+ return 0;
+}
+
+static int klp_enable_object(struct klp_object *obj)
+{
+ struct klp_func *func;
+ int ret;
+
+ if (WARN_ON(obj->state != KLP_DISABLED))
+ return -EINVAL;
+
+ if (WARN_ON(!klp_is_object_loaded(obj)))
+ return -EINVAL;
+
+ for (func = obj->funcs; func->old_name; func++) {
+ ret = klp_enable_func(func);
+ if (ret)
+ goto unregister;
+ }
+ obj->state = KLP_ENABLED;
+
+ return 0;
+
+unregister:
+ WARN_ON(klp_disable_object(obj));
+ return ret;
+}
+
+static int __klp_disable_patch(struct klp_patch *patch)
+{
+ struct klp_object *obj;
+ int ret;
+
+ 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
+ *
+ * Unregisters the patched functions from ftrace.
+ *
+ * Return: 0 on success, otherwise error
+ */
+int klp_disable_patch(struct klp_patch *patch)
+{
+ int ret;
+
+ mutex_lock(&klp_mutex);
+
+ if (!klp_is_patch_registered(patch)) {
+ ret = -EINVAL;
+ goto err;
+ }
+
+ if (patch->state == KLP_DISABLED) {
+ ret = -EINVAL;
+ goto err;
+ }
+
+ ret = __klp_disable_patch(patch);
+
+err:
+ 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++) {
+ klp_find_object_module(obj);
+
+ if (!klp_is_object_loaded(obj))
+ continue;
+
+ ret = klp_enable_object(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;
+
+ mutex_lock(&klp_mutex);
+
+ if (!klp_is_patch_registered(patch)) {
+ ret = -EINVAL;
+ goto err;
+ }
+
+ ret = __klp_enable_patch(patch);
+
+err:
+ mutex_unlock(&klp_mutex);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(klp_enable_patch);
+
+/*
+ * Sysfs Interface
+ *
+ * /sys/kernel/livepatch
+ * /sys/kernel/livepatch/<patch>
+ * /sys/kernel/livepatch/<patch>/enabled
+ * /sys/kernel/livepatch/<patch>/<object>
+ * /sys/kernel/livepatch/<patch>/<object>/<func>
+ */
+
+static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct klp_patch *patch;
+ int ret;
+ unsigned long val;
+
+ ret = kstrtoul(buf, 10, &val);
+ if (ret)
+ return -EINVAL;
+
+ if (val != KLP_DISABLED && val != KLP_ENABLED)
+ return -EINVAL;
+
+ patch = container_of(kobj, struct klp_patch, kobj);
+
+ mutex_lock(&klp_mutex);
+
+ if (val == patch->state) {
+ /* already in requested state */
+ ret = -EINVAL;
+ goto err;
+ }
+
+ if (val == KLP_ENABLED) {
+ ret = __klp_enable_patch(patch);
+ if (ret)
+ goto err;
+ } else {
+ ret = __klp_disable_patch(patch);
+ if (ret)
+ goto err;
+ }
+
+ mutex_unlock(&klp_mutex);
+
+ return count;
+
+err:
+ mutex_unlock(&klp_mutex);
+ return ret;
+}
+
+static ssize_t enabled_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct klp_patch *patch;
+
+ patch = container_of(kobj, struct klp_patch, kobj);
+ return snprintf(buf, PAGE_SIZE-1, "%d\n", patch->state);
+}
+
+static struct kobj_attribute enabled_kobj_attr = __ATTR_RW(enabled);
+static struct attribute *klp_patch_attrs[] = {
+ &enabled_kobj_attr.attr,
+ NULL
+};
+
+static void klp_kobj_release_patch(struct kobject *kobj)
+{
+ /*
+ * Once we have a consistency model we'll need to module_put() the
+ * patch module here. See klp_register_patch() for more details.
+ */
+}
+
+static struct kobj_type klp_ktype_patch = {
+ .release = klp_kobj_release_patch,
+ .sysfs_ops = &kobj_sysfs_ops,
+ .default_attrs = klp_patch_attrs,
+};
+
+static void klp_kobj_release_func(struct kobject *kobj)
+{
+ struct klp_func *func;
+
+ func = container_of(kobj, struct klp_func, kobj);
+ kfree(func->fops);
+}
+
+static struct kobj_type klp_ktype_func = {
+ .release = klp_kobj_release_func,
+ .sysfs_ops = &kobj_sysfs_ops,
+};
+
+/*
+ * Free all functions' kobjects in the array up to some limit. When limit is
+ * NULL, all kobjects are freed.
+ */
+static void klp_free_funcs_limited(struct klp_object *obj,
+ struct klp_func *limit)
+{
+ struct klp_func *func;
+
+ for (func = obj->funcs; func->old_name && func != limit; func++)
+ kobject_put(&func->kobj);
+}
+
+/* Clean up when a patched object is unloaded */
+static void klp_free_object_loaded(struct klp_object *obj)
+{
+ struct klp_func *func;
+
+ obj->mod = NULL;
+
+ for (func = obj->funcs; func->old_name; func++)
+ func->old_addr = 0;
+}
+
+/*
+ * Free all objects' kobjects in the array up to some limit. When limit is
+ * NULL, all kobjects are freed.
+ */
+static void klp_free_objects_limited(struct klp_patch *patch,
+ struct klp_object *limit)
+{
+ struct klp_object *obj;
+
+ for (obj = patch->objs; obj->funcs && obj != limit; obj++) {
+ klp_free_funcs_limited(obj, NULL);
+ kobject_put(obj->kobj);
+ }
+}
+
+static void klp_free_patch(struct klp_patch *patch)
+{
+ klp_free_objects_limited(patch, NULL);
+ if (!list_empty(&patch->list))
+ list_del(&patch->list);
+ kobject_put(&patch->kobj);
+}
+
+static int klp_init_func(struct klp_object *obj, struct klp_func *func)
+{
+ struct ftrace_ops *ops;
+ int ret;
+
+ ops = kzalloc(sizeof(*ops), GFP_KERNEL);
+ if (!ops)
+ return -ENOMEM;
+
+ ops->private = func;
+ ops->func = klp_ftrace_handler;
+ ops->flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC;
+ func->fops = ops;
+ func->state = KLP_DISABLED;
+
+ ret = kobject_init_and_add(&func->kobj, &klp_ktype_func,
+ obj->kobj, func->old_name);
+ if (ret) {
+ kfree(func->fops);
+ return ret;
+ }
+
+ return 0;
+}
+
+/* parts of the initialization that is done only when the object is loaded */
+static int klp_init_object_loaded(struct klp_patch *patch,
+ struct klp_object *obj)
+{
+ struct klp_func *func;
+ int ret;
+
+ if (obj->relocs) {
+ ret = klp_write_object_relocations(patch->mod, obj);
+ if (ret)
+ return ret;
+ }
+
+ for (func = obj->funcs; func->old_name; func++) {
+ ret = klp_find_verify_func_addr(obj, func);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
+{
+ struct klp_func *func;
+ int ret;
+ const char *name;
+
+ if (!obj->funcs)
+ return -EINVAL;
+
+ obj->state = KLP_DISABLED;
+
+ klp_find_object_module(obj);
+
+ name = klp_is_module(obj) ? obj->name : "vmlinux";
+ obj->kobj = kobject_create_and_add(name, &patch->kobj);
+ if (!obj->kobj)
+ return -ENOMEM;
+
+ for (func = obj->funcs; func->old_name; func++) {
+ ret = klp_init_func(obj, func);
+ if (ret)
+ goto free;
+ }
+
+ if (klp_is_object_loaded(obj)) {
+ ret = klp_init_object_loaded(patch, obj);
+ if (ret)
+ goto free;
+ }
+
+ return 0;
+
+free:
+ klp_free_funcs_limited(obj, func);
+ kobject_put(obj->kobj);
+ return ret;
+}
+
+static int klp_init_patch(struct klp_patch *patch)
+{
+ struct klp_object *obj;
+ int ret;
+
+ if (!patch->objs)
+ return -EINVAL;
+
+ mutex_lock(&klp_mutex);
+
+ patch->state = KLP_DISABLED;
+
+ ret = kobject_init_and_add(&patch->kobj, &klp_ktype_patch,
+ klp_root_kobj, patch->mod->name);
+ if (ret)
+ goto unlock;
+
+ for (obj = patch->objs; obj->funcs; obj++) {
+ ret = klp_init_object(patch, obj);
+ if (ret)
+ goto free;
+ }
+
+ list_add(&patch->list, &klp_patches);
+
+ mutex_unlock(&klp_mutex);
+
+ return 0;
+
+free:
+ klp_free_objects_limited(patch, obj);
+ kobject_put(&patch->kobj);
+unlock:
+ mutex_unlock(&klp_mutex);
+ return ret;
+}
+
+/**
+ * klp_unregister_patch() - unregisters a patch
+ * @patch: Disabled patch to be unregistered
+ *
+ * Frees the data structures and removes the sysfs interface.
+ *
+ * Return: 0 on success, otherwise error
+ */
+int klp_unregister_patch(struct klp_patch *patch)
+{
+ int ret = 0;
+
+ mutex_lock(&klp_mutex);
+
+ if (!klp_is_patch_registered(patch)) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (patch->state == KLP_ENABLED) {
+ ret = -EBUSY;
+ goto out;
+ }
+
+ klp_free_patch(patch);
+
+out:
+ mutex_unlock(&klp_mutex);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(klp_unregister_patch);
+
+/**
+ * klp_register_patch() - registers a patch
+ * @patch: Patch to be registered
+ *
+ * Initializes the data structure associated with the patch and
+ * creates the sysfs interface.
+ *
+ * Return: 0 on success, otherwise error
+ */
+int klp_register_patch(struct klp_patch *patch)
+{
+ int ret;
+
+ if (!klp_initialized())
+ return -ENODEV;
+
+ if (!patch || !patch->mod)
+ return -EINVAL;
+
+ /*
+ * 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.
+ */
+ if (!try_module_get(patch->mod))
+ return -ENODEV;
+
+ ret = klp_init_patch(patch);
+ if (ret)
+ module_put(patch->mod);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(klp_register_patch);
+
+static void klp_module_notify_coming(struct klp_patch *patch,
+ struct klp_object *obj)
+{
+ struct module *pmod = patch->mod;
+ struct module *mod = obj->mod;
+ int ret;
+
+ ret = klp_init_object_loaded(patch, obj);
+ if (ret)
+ goto err;
+
+ if (patch->state == KLP_DISABLED)
+ return;
+
+ pr_notice("applying patch '%s' to loading module '%s'\n",
+ pmod->name, mod->name);
+
+ ret = klp_enable_object(obj);
+ if (!ret)
+ return;
+
+err:
+ pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
+ pmod->name, mod->name, ret);
+}
+
+static void klp_module_notify_going(struct klp_patch *patch,
+ struct klp_object *obj)
+{
+ struct module *pmod = patch->mod;
+ struct module *mod = obj->mod;
+ int ret;
+
+ if (patch->state == KLP_DISABLED)
+ goto disabled;
+
+ 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);
+
+disabled:
+ klp_free_object_loaded(obj);
+}
+
+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) {
+ for (obj = patch->objs; obj->funcs; obj++) {
+ if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
+ continue;
+
+ if (action == MODULE_STATE_COMING) {
+ obj->mod = mod;
+ klp_module_notify_coming(patch, obj);
+ } else /* MODULE_STATE_GOING */
+ klp_module_notify_going(patch, obj);
+
+ break;
+ }
+ }
+
+ mutex_unlock(&klp_mutex);
+
+ return 0;
+}
+
+static struct notifier_block klp_module_nb = {
+ .notifier_call = klp_module_notify,
+ .priority = INT_MIN+1, /* called late but before ftrace notifier */
+};
+
+static int klp_init(void)
+{
+ int ret;
+
+ ret = register_module_notifier(&klp_module_nb);
+ if (ret)
+ return ret;
+
+ klp_root_kobj = kobject_create_and_add("livepatch", kernel_kobj);
+ if (!klp_root_kobj) {
+ ret = -ENOMEM;
+ goto unregister;
+ }
+
+ return 0;
+
+unregister:
+ unregister_module_notifier(&klp_module_nb);
+ return ret;
+}
+
+module_init(klp_init);
--
2.1.0

2014-12-16 17:59:52

[permalink] [raw]

Subject: [PATCHv7 3/3] samples: add sample live patching module

Add a sample live patching module.

Signed-off-by: Seth Jennings <[email protected]>
---
MAINTAINERS | 1 +
samples/Kconfig | 7 +++
samples/Makefile | 2 +-
samples/livepatch/Makefile | 1 +
samples/livepatch/livepatch-sample.c | 87 ++++++++++++++++++++++++++++++++++++
5 files changed, 97 insertions(+), 1 deletion(-)
create mode 100644 samples/livepatch/Makefile
create mode 100644 samples/livepatch/livepatch-sample.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 94f9f69..15c0929d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5788,6 +5788,7 @@ F: include/linux/livepatch.h
F: arch/x86/include/asm/livepatch.h
F: arch/x86/kernel/livepatch.c
F: Documentation/ABI/testing/sysfs-kernel-livepatch
+F: samples/livepatch/
L: [email protected]

LLC (802.2)
diff --git a/samples/Kconfig b/samples/Kconfig
index 6181c2c..0aed20d 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -63,4 +63,11 @@ config SAMPLE_RPMSG_CLIENT
to communicate with an AMP-configured remote processor over
the rpmsg bus.

+config SAMPLE_LIVE_PATCHING
+ tristate "Build live patching sample -- loadable modules only"
+ depends on LIVE_PATCHING && m
+ help
+ Builds a sample live patch that replaces the procfs handler
+ for /proc/cmdline to print "this has been live patched".
+
endif # SAMPLES
diff --git a/samples/Makefile b/samples/Makefile
index 1a60c62..f00257b 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -1,4 +1,4 @@
# Makefile for Linux samples code

-obj-$(CONFIG_SAMPLES) += kobject/ kprobes/ trace_events/ \
+obj-$(CONFIG_SAMPLES) += kobject/ kprobes/ trace_events/ livepatch/ \
hw_breakpoint/ kfifo/ kdb/ hidraw/ rpmsg/ seccomp/
diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
new file mode 100644
index 0000000..7f1cdc1
--- /dev/null
+++ b/samples/livepatch/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SAMPLE_LIVE_PATCHING) += livepatch-sample.o
diff --git a/samples/livepatch/livepatch-sample.c b/samples/livepatch/livepatch-sample.c
new file mode 100644
index 0000000..21f159d
--- /dev/null
+++ b/samples/livepatch/livepatch-sample.c
@@ -0,0 +1,87 @@
+/*
+ * livepatch-sample.c - Kernel Live Patching Sample Module
+ *
+ * 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/>.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/livepatch.h>
+
+/*
+ * This (dumb) live patch overrides the function that prints the
+ * kernel boot cmdline when /proc/cmdline is read.
+ *
+ * Example:
+ * $ cat /proc/cmdline
+ * <your cmdline>
+ * $ insmod livepatch-sample.ko
+ * $ cat /proc/cmdline
+ * this has been live patched
+ * $ echo 0 > /sys/kernel/livepatch/klp_sample/enabled
+ * <your cmdline>
+ */
+
+#include <linux/seq_file.h>
+static int livepatch_cmdline_proc_show(struct seq_file *m, void *v)
+{
+ seq_printf(m, "%s\n", "this has been live patched");
+ return 0;
+}
+
+static struct klp_func funcs[] = {
+ {
+ .old_name = "cmdline_proc_show",
+ .new_func = livepatch_cmdline_proc_show,
+ }, { }
+};
+
+static struct klp_object objs[] = {
+ {
+ /* name being NULL means vmlinux */
+ .funcs = funcs,
+ }, { }
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+};
+
+static int livepatch_init(void)
+{
+ int ret;
+
+ 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_exit(void)
+{
+ WARN_ON(klp_disable_patch(&patch));
+ WARN_ON(klp_unregister_patch(&patch));
+}
+
+module_init(livepatch_init);
+module_exit(livepatch_exit);
+MODULE_LICENSE("GPL");
--
2.1.0

2014-12-16 18:15:15

[permalink] [raw]

Subject: Re: [PATCHv7 0/3] Kernel Live Patching

On Tue, Dec 16, 2014 at 11:28 PM, Seth Jennings <[email protected]> wrote:
>
> Changelog:
>
> Thanks for all the feedback!
>

Could you describe what this does to signing? I presume the patched
module should cause a taint on module signing?

Balbir Singh

2014-12-16 18:46:21

[permalink] [raw]

Subject: Re: [PATCHv7 2/3] kernel: add support for live patching

On Tue, Dec 16, 2014 at 11:28 PM, Seth Jennings <[email protected]> wrote:
> 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]>
> Signed-off-by: Josh Poimboeuf <[email protected]>

snip

> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index ce8dcdf..5c57181 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -17,6 +17,7 @@ config X86_64
> depends on 64BIT
> select X86_DEV_DMA_OPS
> select ARCH_USE_CMPXCHG_LOCKREF
> + select ARCH_HAVE_LIVE_PATCHING
>
> ### Arch settings
> config X86
> @@ -1986,6 +1987,8 @@ config CMDLINE_OVERRIDE
> This is used to work around broken boot loaders. This should
> be set to 'N' under normal conditions.
>
> +source "kernel/livepatch/Kconfig"
> +
> endmenu
>
> config ARCH_ENABLE_MEMORY_HOTPLUG
> diff --git a/arch/x86/include/asm/livepatch.h b/arch/x86/include/asm/livepatch.h
> new file mode 100644
> index 0000000..c2ae592
> --- /dev/null
> +++ b/arch/x86/include/asm/livepatch.h
> @@ -0,0 +1,36 @@
> +/*
> + * 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
> +extern int klp_write_module_reloc(struct module *mod, unsigned long type,
> + unsigned long loc, unsigned long value);
> +
> +#else
> +#error Live patching support is disabled; check CONFIG_LIVE_PATCHING
> +#endif
> +
> +#endif /* _ASM_X86_LIVEPATCH_H */
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 5d4502c..316b34e 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -63,6 +63,7 @@ obj-$(CONFIG_X86_MPPARSE) += mpparse.o
> obj-y += apic/
> obj-$(CONFIG_X86_REBOOTFIXUPS) += reboot_fixups_32.o
> obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o
> +obj-$(CONFIG_LIVE_PATCHING) += livepatch.o
> obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o
> obj-$(CONFIG_FTRACE_SYSCALLS) += ftrace.o
> obj-$(CONFIG_X86_TSC) += trace_clock.o
> diff --git a/arch/x86/kernel/livepatch.c b/arch/x86/kernel/livepatch.c
> new file mode 100644
> index 0000000..4b0ed7b
> --- /dev/null
> +++ b/arch/x86/kernel/livepatch.c
> @@ -0,0 +1,89 @@
> +/*
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/uaccess.h>
> +#include <asm/cacheflush.h>
> +#include <asm/page_types.h>
> +#include <asm/elf.h>
> +#include <asm/livepatch.h>
> +
> +/**
> + * klp_write_module_reloc() - write a relocation in a module
> + * @mod: module in which the section to be modified is found
> + * @type: ELF relocation type (see asm/elf.h)
> + * @loc: address that the relocation should be written to
> + * @value: relocation value (sym address + addend)
> + *
> + * This function writes a relocation to the specified location for
> + * a particular module.

This is more of the what then why?

> + */
> +int klp_write_module_reloc(struct module *mod, unsigned long type,
> + unsigned long loc, unsigned long value)
> +{
> + int ret, numpages, size = 4;
> + bool readonly;
> + 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;
> + }
> +
> + if (loc < core || loc >= core + core_size)
> + /* loc does not point to any symbol inside the module */
> + return -EINVAL;
> +
> + if (loc < core + core_ro_size)
> + readonly = true;
> + else
> + readonly = false;
> +
> + /* determine if the relocation spans a page boundary */
> + 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;
> +}
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> new file mode 100644
> index 0000000..b61fe74
> --- /dev/null
> +++ b/include/linux/livepatch.h
> @@ -0,0 +1,132 @@
> +/*
> + * 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)
> + * @kobj: kobject for sysfs resources
> + * @fops: ftrace operations structure
> + * @state: tracks function-level patch application state
> + */
> +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
> + * @loc: address where the relocation will be written
> + * @val: 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 loc;
> + unsigned long val;
> + 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
> + * @kobj: kobject for sysfs resources
> + * @mod: kernel module associated with the patched object
> + * (NULL for vmlinux)
> + * @state: tracks object-level patch application state
> + */
> +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
> + * @list: list node for global list of registered patches
> + * @kobj: kobject for sysfs resources
> + * @state: tracks patch-level application state
> + */
> +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_ */
> diff --git a/kernel/Makefile b/kernel/Makefile
> index a59481a..616994f 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -26,6 +26,7 @@ obj-y += power/
> obj-y += printk/
> obj-y += irq/
> obj-y += rcu/
> +obj-y += livepatch/
>
> obj-$(CONFIG_CHECKPOINT_RESTORE) += kcmp.o
> obj-$(CONFIG_FREEZER) += freezer.o
> diff --git a/kernel/livepatch/Kconfig b/kernel/livepatch/Kconfig
> new file mode 100644
> index 0000000..96da00f
> --- /dev/null
> +++ b/kernel/livepatch/Kconfig
> @@ -0,0 +1,18 @@
> +config ARCH_HAVE_LIVE_PATCHING
> + boolean
> + help
> + Arch supports kernel live patching
> +
> +config LIVE_PATCHING
> + boolean "Kernel Live Patching"
> + depends on DYNAMIC_FTRACE_WITH_REGS
> + depends on MODULES
> + depends on SYSFS
> + depends on KALLSYMS_ALL
> + depends on ARCH_HAVE_LIVE_PATCHING
> + help
> + Say Y here if you want to support kernel live patching.
> + This option has no runtime impact until a kernel "patch"
> + module uses the interface provided by this option to register
> + a patch, causing calls to patched functions to be redirected
> + to new function code contained in the patch module.
> diff --git a/kernel/livepatch/Makefile b/kernel/livepatch/Makefile
> new file mode 100644
> index 0000000..7c1f008
> --- /dev/null
> +++ b/kernel/livepatch/Makefile
> @@ -0,0 +1,3 @@
> +obj-$(CONFIG_LIVE_PATCHING) += livepatch.o
> +
> +livepatch-objs := core.o
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> new file mode 100644
> index 0000000..0004a71
> --- /dev/null
> +++ b/kernel/livepatch/core.c
> @@ -0,0 +1,929 @@
> +/*
> + * 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;
> +
> +static bool klp_is_module(struct klp_object *obj)
> +{
> + return obj->name;
> +}
> +
> +static bool klp_is_object_loaded(struct klp_object *obj)
> +{
> + return !obj->name || obj->mod;
> +}
> +
> +/* sets obj->mod if object is not vmlinux and module is found */
> +static void klp_find_object_module(struct klp_object *obj)
> +{
> + if (!klp_is_module(obj))
> + return;
> +
> + 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);
> +}
> +
> +/* klp_mutex must be held by caller */
> +static bool klp_is_patch_registered(struct klp_patch *patch)
> +{
> + struct klp_patch *mypatch;
> +
> + list_for_each_entry(mypatch, &klp_patches, list)
> + if (mypatch == patch)
> + return true;
> +
> + return false;
> +}
> +
> +static bool klp_initialized(void)
> +{
> + return klp_root_kobj;
> +}
> +
> +struct klp_find_arg {
> + const char *objname;
> + const char *name;
> + unsigned long addr;
> + /*
> + * If count == 0, the symbol was not found. If count == 1, a unique
> + * match was found and addr is set. If count > 1, there is
> + * unresolvable ambiguity among "count" number of symbols with the same
> + * name in the same object.
> + */
> + unsigned long count;
> +};
> +
> +static int klp_find_callback(void *data, const char *name,
> + struct module *mod, unsigned long addr)
> +{
> + struct klp_find_arg *args = data;
> +
> + if ((mod && !args->objname) || (!mod && args->objname))
> + return 0;
> +
> + if (strcmp(args->name, name))
> + return 0;
> +
> + if (args->objname && strcmp(args->objname, mod->name))
> + return 0;
> +
> + /*
> + * args->addr might be overwritten if another match is found
> + * but klp_find_object_symbol() handles this and only returns the
> + * addr if count == 1.
> + */
> + args->addr = addr;
> + args->count++;
> +
> + return 0;
> +}
> +
> +static int klp_find_object_symbol(const char *objname, const char *name,
> + unsigned long *addr)
> +{
> + struct klp_find_arg args = {
> + .objname = objname,
> + .name = name,
> + .addr = 0,
> + .count = 0
> + };
> +
> + kallsyms_on_each_symbol(klp_find_callback, &args);
> +
> + if (args.count == 0)
> + pr_err("symbol '%s' not found in symbol table\n", name);
> + else if (args.count > 1)
> + pr_err("unresolvable ambiguity (%lu matches) on symbol '%s' in object '%s'\n",
> + args.count, name, objname);
> + else {
> + *addr = args.addr;
> + return 0;
> + }
> +
> + *addr = 0;
> + return -EINVAL;
> +}
> +
> +struct klp_verify_args {
> + const char *name;
> + const unsigned long addr;
> +};
> +
> +static int klp_verify_callback(void *data, const char *name,
> + struct module *mod, unsigned long addr)
> +{
> + struct klp_verify_args *args = data;
> +
> + if (!mod &&
> + !strcmp(args->name, name) &&
> + args->addr == addr)
> + return 1;
> +
> + return 0;
> +}
> +
> +static int klp_verify_vmlinux_symbol(const char *name, unsigned long addr)
> +{
> + struct klp_verify_args args = {
> + .name = name,
> + .addr = addr,
> + };
> +
> + if (kallsyms_on_each_symbol(klp_verify_callback, &args))
> + return 0;
> +
> + pr_err("symbol '%s' not found at specified address 0x%016lx, kernel mismatch?",
> + name, addr);
> + return -EINVAL;
> +}
> +
> +static int klp_find_verify_func_addr(struct klp_object *obj,
> + struct klp_func *func)
> +{
> + int ret;
> +
> +#if defined(CONFIG_RANDOMIZE_BASE)
> + /* KASLR is enabled, disregard old_addr from user */
> + func->old_addr = 0;
> +#endif
> +
> + if (!func->old_addr || klp_is_module(obj))
> + ret = klp_find_object_symbol(obj->name, func->old_name,
> + &func->old_addr);
> + else
> + ret = klp_verify_vmlinux_symbol(func->old_name,
> + func->old_addr);
> +
> + return ret;
> +}
> +
> +/*
> + * external symbols are located outside the parent object (where the parent
> + * object is either vmlinux or the kmod being patched).
> + */
> +static int klp_find_external_symbol(struct module *pmod, const char *name,
> + unsigned long *addr)
> +{
> + const struct kernel_symbol *sym;
> +
> + /* first, check if it's an exported symbol */
> + preempt_disable();
> + sym = find_symbol(name, NULL, NULL, true, true);
> + preempt_enable();
> + if (sym) {
> + *addr = sym->value;
> + return 0;
> + }
> +
> + /* otherwise check if it's in another .o within the patch module */
> + return klp_find_object_symbol(pmod->name, name, addr);
> +}
> +

Shouldn't the code above be asbtracted out for anyone searching for
kernel symbols? Its only KLP at the moment, but I am sure these are
reusable bits

> +static int klp_write_object_relocations(struct module *pmod,
> + struct klp_object *obj)
> +{
> + int ret;
> + struct klp_reloc *reloc;
> +
> + if (WARN_ON(!klp_is_object_loaded(obj)))
> + return -EINVAL;
> +
> + if (WARN_ON(!obj->relocs))
> + return -EINVAL;
> +
> + for (reloc = obj->relocs; reloc->name; reloc++) {
> + if (!klp_is_module(obj)) {
> + ret = klp_verify_vmlinux_symbol(reloc->name,
> + reloc->val);
> + if (ret)
> + return ret;
> + } else {
> + /* module, reloc->val needs to be discovered */
> + if (reloc->external)
> + ret = klp_find_external_symbol(pmod,
> + reloc->name,
> + &reloc->val);
> + else
> + ret = klp_find_object_symbol(obj->mod->name,
> + reloc->name,
> + &reloc->val);
> + if (ret)
> + return ret;
> + }
> + ret = klp_write_module_reloc(pmod, reloc->type, reloc->loc,
> + reloc->val + reloc->addend);

An iterative loop can be expensive -- state transitions from page
rw/ro frequently! Can we optimize?

> + if (ret) {
> + pr_err("relocation failed for symbol '%s' at 0x%016lx (%d)\n",
> + reloc->name, reloc->val, ret);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static void notrace klp_ftrace_handler(unsigned long ip,
> + unsigned long parent_ip,
> + struct ftrace_ops *ops,
> + struct pt_regs *regs)
> +{
> + struct klp_func *func = ops->private;
> +
> + regs->ip = (unsigned long)func->new_func;
> +}
> +
> +static int klp_disable_func(struct klp_func *func)
> +{
> + int ret;
> +
> + if (WARN_ON(func->state != KLP_ENABLED))
> + return -EINVAL;
> +
> + if (WARN_ON(!func->old_addr))
> + return -EINVAL;
> +
> + ret = unregister_ftrace_function(func->fops);
> + if (ret) {
> + pr_err("failed to unregister ftrace handler for function '%s' (%d)\n",
> + func->old_name, ret);
> + return ret;
> + }
> +
> + ret = ftrace_set_filter_ip(func->fops, func->old_addr, 1, 0);
> + if (ret)
> + pr_warn("function unregister succeeded but failed to clear the filter\n");
> +
> + func->state = KLP_DISABLED;
> +
> + return 0;
> +}
> +
> +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",
> + func->old_name, ret);
> + return ret;
> + }
> +
> + ret = register_ftrace_function(func->fops);
> + if (ret) {
> + pr_err("failed to register ftrace handler for function '%s' (%d)\n",
> + func->old_name, ret);
> + ftrace_set_filter_ip(func->fops, func->old_addr, 1, 0);
> + } else {
> + func->state = KLP_ENABLED;
> + }
> +
> + return ret;
> +}
> +
> +static int klp_disable_object(struct klp_object *obj)
> +{
> + struct klp_func *func;
> + int ret;
> +
> + for (func = obj->funcs; func->old_name; func++) {
> + if (func->state != KLP_ENABLED)
> + continue;
> +
> + ret = klp_disable_func(func);
> + if (ret)
> + return ret;
> + }
> +
> + obj->state = KLP_DISABLED;
> +
> + return 0;
> +}
> +
> +static int klp_enable_object(struct klp_object *obj)
> +{
> + struct klp_func *func;
> + int ret;
> +
> + if (WARN_ON(obj->state != KLP_DISABLED))
> + return -EINVAL;
> +
> + if (WARN_ON(!klp_is_object_loaded(obj)))
> + return -EINVAL;
> +
> + for (func = obj->funcs; func->old_name; func++) {
> + ret = klp_enable_func(func);
> + if (ret)
> + goto unregister;
> + }
> + obj->state = KLP_ENABLED;
> +
> + return 0;
> +
> +unregister:
> + WARN_ON(klp_disable_object(obj));
> + return ret;
> +}
> +
> +static int __klp_disable_patch(struct klp_patch *patch)
> +{
> + struct klp_object *obj;
> + int ret;
> +
> + 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
> + *
> + * Unregisters the patched functions from ftrace.
> + *
> + * Return: 0 on success, otherwise error
> + */
> +int klp_disable_patch(struct klp_patch *patch)
> +{
> + int ret;
> +
> + mutex_lock(&klp_mutex);
> +
> + if (!klp_is_patch_registered(patch)) {
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + if (patch->state == KLP_DISABLED) {
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + ret = __klp_disable_patch(patch);
> +
> +err:
> + 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++) {
> + klp_find_object_module(obj);
> +
> + if (!klp_is_object_loaded(obj))
> + continue;
> +
> + ret = klp_enable_object(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;
> +
> + mutex_lock(&klp_mutex);
> +
> + if (!klp_is_patch_registered(patch)) {
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + ret = __klp_enable_patch(patch);
> +
> +err:
> + mutex_unlock(&klp_mutex);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(klp_enable_patch);
> +
> +/*
> + * Sysfs Interface
> + *
> + * /sys/kernel/livepatch
> + * /sys/kernel/livepatch/<patch>
> + * /sys/kernel/livepatch/<patch>/enabled
> + * /sys/kernel/livepatch/<patch>/<object>
> + * /sys/kernel/livepatch/<patch>/<object>/<func>
> + */
> +
> +static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct klp_patch *patch;
> + int ret;
> + unsigned long val;
> +
> + ret = kstrtoul(buf, 10, &val);
> + if (ret)
> + return -EINVAL;
> +
> + if (val != KLP_DISABLED && val != KLP_ENABLED)
> + return -EINVAL;
> +
> + patch = container_of(kobj, struct klp_patch, kobj);
> +
> + mutex_lock(&klp_mutex);
> +
> + if (val == patch->state) {
> + /* already in requested state */
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + if (val == KLP_ENABLED) {
> + ret = __klp_enable_patch(patch);
> + if (ret)
> + goto err;
> + } else {
> + ret = __klp_disable_patch(patch);
> + if (ret)
> + goto err;
> + }
> +
> + mutex_unlock(&klp_mutex);
> +
> + return count;
> +
> +err:
> + mutex_unlock(&klp_mutex);
> + return ret;
> +}
> +
> +static ssize_t enabled_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct klp_patch *patch;
> +
> + patch = container_of(kobj, struct klp_patch, kobj);
> + return snprintf(buf, PAGE_SIZE-1, "%d\n", patch->state);
> +}
> +
> +static struct kobj_attribute enabled_kobj_attr = __ATTR_RW(enabled);
> +static struct attribute *klp_patch_attrs[] = {
> + &enabled_kobj_attr.attr,
> + NULL
> +};
> +
> +static void klp_kobj_release_patch(struct kobject *kobj)
> +{
> + /*
> + * Once we have a consistency model we'll need to module_put() the
> + * patch module here. See klp_register_patch() for more details.
> + */
> +}
> +
> +static struct kobj_type klp_ktype_patch = {
> + .release = klp_kobj_release_patch,
> + .sysfs_ops = &kobj_sysfs_ops,
> + .default_attrs = klp_patch_attrs,
> +};
> +
> +static void klp_kobj_release_func(struct kobject *kobj)
> +{
> + struct klp_func *func;
> +
> + func = container_of(kobj, struct klp_func, kobj);
> + kfree(func->fops);
> +}
> +
> +static struct kobj_type klp_ktype_func = {
> + .release = klp_kobj_release_func,
> + .sysfs_ops = &kobj_sysfs_ops,
> +};
> +
> +/*
> + * Free all functions' kobjects in the array up to some limit. When limit is
> + * NULL, all kobjects are freed.
> + */
> +static void klp_free_funcs_limited(struct klp_object *obj,
> + struct klp_func *limit)
> +{
> + struct klp_func *func;
> +
> + for (func = obj->funcs; func->old_name && func != limit; func++)
> + kobject_put(&func->kobj);
> +}
> +
> +/* Clean up when a patched object is unloaded */
> +static void klp_free_object_loaded(struct klp_object *obj)
> +{
> + struct klp_func *func;
> +
> + obj->mod = NULL;
> +
> + for (func = obj->funcs; func->old_name; func++)
> + func->old_addr = 0;
> +}
> +
> +/*
> + * Free all objects' kobjects in the array up to some limit. When limit is
> + * NULL, all kobjects are freed.
> + */
> +static void klp_free_objects_limited(struct klp_patch *patch,
> + struct klp_object *limit)
> +{
> + struct klp_object *obj;
> +
> + for (obj = patch->objs; obj->funcs && obj != limit; obj++) {
> + klp_free_funcs_limited(obj, NULL);
> + kobject_put(obj->kobj);
> + }
> +}
> +
> +static void klp_free_patch(struct klp_patch *patch)
> +{
> + klp_free_objects_limited(patch, NULL);
> + if (!list_empty(&patch->list))
> + list_del(&patch->list);
> + kobject_put(&patch->kobj);
> +}
> +
> +static int klp_init_func(struct klp_object *obj, struct klp_func *func)
> +{
> + struct ftrace_ops *ops;
> + int ret;
> +
> + ops = kzalloc(sizeof(*ops), GFP_KERNEL);
> + if (!ops)
> + return -ENOMEM;
> +
> + ops->private = func;
> + ops->func = klp_ftrace_handler;
> + ops->flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC;
> + func->fops = ops;
> + func->state = KLP_DISABLED;
> +
> + ret = kobject_init_and_add(&func->kobj, &klp_ktype_func,
> + obj->kobj, func->old_name);
> + if (ret) {
> + kfree(func->fops);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/* parts of the initialization that is done only when the object is loaded */
> +static int klp_init_object_loaded(struct klp_patch *patch,
> + struct klp_object *obj)
> +{
> + struct klp_func *func;
> + int ret;
> +
> + if (obj->relocs) {
> + ret = klp_write_object_relocations(patch->mod, obj);
> + if (ret)
> + return ret;
> + }
> +
> + for (func = obj->funcs; func->old_name; func++) {
> + ret = klp_find_verify_func_addr(obj, func);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
> +{
> + struct klp_func *func;
> + int ret;
> + const char *name;
> +
> + if (!obj->funcs)
> + return -EINVAL;
> +
> + obj->state = KLP_DISABLED;
> +
> + klp_find_object_module(obj);
> +
> + name = klp_is_module(obj) ? obj->name : "vmlinux";
> + obj->kobj = kobject_create_and_add(name, &patch->kobj);
> + if (!obj->kobj)
> + return -ENOMEM;
> +
> + for (func = obj->funcs; func->old_name; func++) {
> + ret = klp_init_func(obj, func);
> + if (ret)
> + goto free;
> + }
> +
> + if (klp_is_object_loaded(obj)) {
> + ret = klp_init_object_loaded(patch, obj);
> + if (ret)
> + goto free;
> + }
> +
> + return 0;
> +
> +free:
> + klp_free_funcs_limited(obj, func);
> + kobject_put(obj->kobj);
> + return ret;
> +}
> +
> +static int klp_init_patch(struct klp_patch *patch)
> +{
> + struct klp_object *obj;
> + int ret;
> +
> + if (!patch->objs)
> + return -EINVAL;
> +
> + mutex_lock(&klp_mutex);
> +
> + patch->state = KLP_DISABLED;
> +
> + ret = kobject_init_and_add(&patch->kobj, &klp_ktype_patch,
> + klp_root_kobj, patch->mod->name);
> + if (ret)
> + goto unlock;
> +
> + for (obj = patch->objs; obj->funcs; obj++) {
> + ret = klp_init_object(patch, obj);
> + if (ret)
> + goto free;
> + }
> +
> + list_add(&patch->list, &klp_patches);
> +
> + mutex_unlock(&klp_mutex);
> +
> + return 0;
> +
> +free:
> + klp_free_objects_limited(patch, obj);
> + kobject_put(&patch->kobj);
> +unlock:
> + mutex_unlock(&klp_mutex);
> + return ret;
> +}
> +
> +/**
> + * klp_unregister_patch() - unregisters a patch
> + * @patch: Disabled patch to be unregistered
> + *
> + * Frees the data structures and removes the sysfs interface.
> + *
> + * Return: 0 on success, otherwise error
> + */
> +int klp_unregister_patch(struct klp_patch *patch)
> +{
> + int ret = 0;
> +
> + mutex_lock(&klp_mutex);
> +
> + if (!klp_is_patch_registered(patch)) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + if (patch->state == KLP_ENABLED) {
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + klp_free_patch(patch);
> +
> +out:
> + mutex_unlock(&klp_mutex);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(klp_unregister_patch);
> +
> +/**
> + * klp_register_patch() - registers a patch
> + * @patch: Patch to be registered
> + *
> + * Initializes the data structure associated with the patch and
> + * creates the sysfs interface.
> + *
> + * Return: 0 on success, otherwise error
> + */
> +int klp_register_patch(struct klp_patch *patch)
> +{
> + int ret;
> +
> + if (!klp_initialized())
> + return -ENODEV;
> +
> + if (!patch || !patch->mod)
> + return -EINVAL;
> +
> + /*
> + * 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.
> + */
> + if (!try_module_get(patch->mod))
> + return -ENODEV;
> +
> + ret = klp_init_patch(patch);
> + if (ret)
> + module_put(patch->mod);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(klp_register_patch);
> +
> +static void klp_module_notify_coming(struct klp_patch *patch,
> + struct klp_object *obj)
> +{
> + struct module *pmod = patch->mod;
> + struct module *mod = obj->mod;
> + int ret;
> +
> + ret = klp_init_object_loaded(patch, obj);
> + if (ret)
> + goto err;
> +
> + if (patch->state == KLP_DISABLED)
> + return;
> +
> + pr_notice("applying patch '%s' to loading module '%s'\n",
> + pmod->name, mod->name);
> +
> + ret = klp_enable_object(obj);
> + if (!ret)
> + return;
> +
> +err:
> + pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
> + pmod->name, mod->name, ret);
> +}
> +
> +static void klp_module_notify_going(struct klp_patch *patch,
> + struct klp_object *obj)
> +{
> + struct module *pmod = patch->mod;
> + struct module *mod = obj->mod;
> + int ret;
> +
> + if (patch->state == KLP_DISABLED)
> + goto disabled;
> +
> + 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);
> +
> +disabled:
> + klp_free_object_loaded(obj);
> +}
> +
> +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) {
> + for (obj = patch->objs; obj->funcs; obj++) {
> + if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
> + continue;
> +
> + if (action == MODULE_STATE_COMING) {
> + obj->mod = mod;
> + klp_module_notify_coming(patch, obj);
> + } else /* MODULE_STATE_GOING */
> + klp_module_notify_going(patch, obj);
> +
> + break;
> + }
> + }
> +
> + mutex_unlock(&klp_mutex);
> +
> + return 0;
> +}
> +
> +static struct notifier_block klp_module_nb = {
> + .notifier_call = klp_module_notify,
> + .priority = INT_MIN+1, /* called late but before ftrace notifier */
> +};
> +
> +static int klp_init(void)
> +{
> + int ret;
> +
> + ret = register_module_notifier(&klp_module_nb);
> + if (ret)
> + return ret;
> +
> + klp_root_kobj = kobject_create_and_add("livepatch", kernel_kobj);
> + if (!klp_root_kobj) {
> + ret = -ENOMEM;
> + goto unregister;
> + }
> +
> + return 0;
> +
> +unregister:
> + unregister_module_notifier(&klp_module_nb);
> + return ret;
> +}
> +
> +module_init(klp_init);

Looks good otherwise

Reviewed-by: Balbir Singh <[email protected]>

2014-12-16 19:06:22

[permalink] [raw]

Subject: Re: [PATCHv7 0/3] Kernel Live Patching

On Tue, Dec 16, 2014 at 11:45:12PM +0530, Balbir Singh wrote:
> On Tue, Dec 16, 2014 at 11:28 PM, Seth Jennings <[email protected]> wrote:
> >
> > Changelog:
> >
> > Thanks for all the feedback!
> >
>
> Could you describe what this does to signing? I presume the patched
> module should cause a taint on module signing?

The patch module can be signed to avoid the taint of being unsigned,
assuming you have the signing key for the kernel you are running.
However we do taint with a new taint flag (see 1/3) to indicate
that the kernel has been patched.

Seth

>
> Balbir Singh

2014-12-16 19:41:53

[permalink] [raw]

Subject: Re: [PATCHv7 2/3] kernel: add support for live patching

On Wed, Dec 17, 2014 at 12:16:18AM +0530, Balbir Singh wrote:
> On Tue, Dec 16, 2014 at 11:28 PM, Seth Jennings <[email protected]> wrote:
> > 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]>
> > Signed-off-by: Josh Poimboeuf <[email protected]>
>
> snip
>
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index ce8dcdf..5c57181 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -17,6 +17,7 @@ config X86_64
> > depends on 64BIT
> > select X86_DEV_DMA_OPS
> > select ARCH_USE_CMPXCHG_LOCKREF
> > + select ARCH_HAVE_LIVE_PATCHING
> >
> > ### Arch settings
> > config X86
> > @@ -1986,6 +1987,8 @@ config CMDLINE_OVERRIDE
> > This is used to work around broken boot loaders. This should
> > be set to 'N' under normal conditions.
> >
> > +source "kernel/livepatch/Kconfig"
> > +
> > endmenu
> >
> > config ARCH_ENABLE_MEMORY_HOTPLUG
> > diff --git a/arch/x86/include/asm/livepatch.h b/arch/x86/include/asm/livepatch.h
> > new file mode 100644
> > index 0000000..c2ae592
> > --- /dev/null
> > +++ b/arch/x86/include/asm/livepatch.h
> > @@ -0,0 +1,36 @@
> > +/*
> > + * 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
> > +extern int klp_write_module_reloc(struct module *mod, unsigned long type,
> > + unsigned long loc, unsigned long value);
> > +
> > +#else
> > +#error Live patching support is disabled; check CONFIG_LIVE_PATCHING
> > +#endif
> > +
> > +#endif /* _ASM_X86_LIVEPATCH_H */
> > diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> > index 5d4502c..316b34e 100644
> > --- a/arch/x86/kernel/Makefile
> > +++ b/arch/x86/kernel/Makefile
> > @@ -63,6 +63,7 @@ obj-$(CONFIG_X86_MPPARSE) += mpparse.o
> > obj-y += apic/
> > obj-$(CONFIG_X86_REBOOTFIXUPS) += reboot_fixups_32.o
> > obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o
> > +obj-$(CONFIG_LIVE_PATCHING) += livepatch.o
> > obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o
> > obj-$(CONFIG_FTRACE_SYSCALLS) += ftrace.o
> > obj-$(CONFIG_X86_TSC) += trace_clock.o
> > diff --git a/arch/x86/kernel/livepatch.c b/arch/x86/kernel/livepatch.c
> > new file mode 100644
> > index 0000000..4b0ed7b
> > --- /dev/null
> > +++ b/arch/x86/kernel/livepatch.c
> > @@ -0,0 +1,89 @@
> > +/*
> > + * 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.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/uaccess.h>
> > +#include <asm/cacheflush.h>
> > +#include <asm/page_types.h>
> > +#include <asm/elf.h>
> > +#include <asm/livepatch.h>
> > +
> > +/**
> > + * klp_write_module_reloc() - write a relocation in a module
> > + * @mod: module in which the section to be modified is found
> > + * @type: ELF relocation type (see asm/elf.h)
> > + * @loc: address that the relocation should be written to
> > + * @value: relocation value (sym address + addend)
> > + *
> > + * This function writes a relocation to the specified location for
> > + * a particular module.
>
> This is more of the what then why?

I can take note of it and make a patch with a better comment.

>
> > + */
> > +int klp_write_module_reloc(struct module *mod, unsigned long type,
> > + unsigned long loc, unsigned long value)
> > +{
> > + int ret, numpages, size = 4;
> > + bool readonly;
> > + 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;
> > + }
> > +
> > + if (loc < core || loc >= core + core_size)
> > + /* loc does not point to any symbol inside the module */
> > + return -EINVAL;
> > +
> > + if (loc < core + core_ro_size)
> > + readonly = true;
> > + else
> > + readonly = false;
> > +
> > + /* determine if the relocation spans a page boundary */
> > + 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;
> > +}
> > diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> > new file mode 100644
> > index 0000000..b61fe74
> > --- /dev/null
> > +++ b/include/linux/livepatch.h
> > @@ -0,0 +1,132 @@
> > +/*
> > + * 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)
> > + * @kobj: kobject for sysfs resources
> > + * @fops: ftrace operations structure
> > + * @state: tracks function-level patch application state
> > + */
> > +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
> > + * @loc: address where the relocation will be written
> > + * @val: 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 loc;
> > + unsigned long val;
> > + 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
> > + * @kobj: kobject for sysfs resources
> > + * @mod: kernel module associated with the patched object
> > + * (NULL for vmlinux)
> > + * @state: tracks object-level patch application state
> > + */
> > +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
> > + * @list: list node for global list of registered patches
> > + * @kobj: kobject for sysfs resources
> > + * @state: tracks patch-level application state
> > + */
> > +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_ */
> > diff --git a/kernel/Makefile b/kernel/Makefile
> > index a59481a..616994f 100644
> > --- a/kernel/Makefile
> > +++ b/kernel/Makefile
> > @@ -26,6 +26,7 @@ obj-y += power/
> > obj-y += printk/
> > obj-y += irq/
> > obj-y += rcu/
> > +obj-y += livepatch/
> >
> > obj-$(CONFIG_CHECKPOINT_RESTORE) += kcmp.o
> > obj-$(CONFIG_FREEZER) += freezer.o
> > diff --git a/kernel/livepatch/Kconfig b/kernel/livepatch/Kconfig
> > new file mode 100644
> > index 0000000..96da00f
> > --- /dev/null
> > +++ b/kernel/livepatch/Kconfig
> > @@ -0,0 +1,18 @@
> > +config ARCH_HAVE_LIVE_PATCHING
> > + boolean
> > + help
> > + Arch supports kernel live patching
> > +
> > +config LIVE_PATCHING
> > + boolean "Kernel Live Patching"
> > + depends on DYNAMIC_FTRACE_WITH_REGS
> > + depends on MODULES
> > + depends on SYSFS
> > + depends on KALLSYMS_ALL
> > + depends on ARCH_HAVE_LIVE_PATCHING
> > + help
> > + Say Y here if you want to support kernel live patching.
> > + This option has no runtime impact until a kernel "patch"
> > + module uses the interface provided by this option to register
> > + a patch, causing calls to patched functions to be redirected
> > + to new function code contained in the patch module.
> > diff --git a/kernel/livepatch/Makefile b/kernel/livepatch/Makefile
> > new file mode 100644
> > index 0000000..7c1f008
> > --- /dev/null
> > +++ b/kernel/livepatch/Makefile
> > @@ -0,0 +1,3 @@
> > +obj-$(CONFIG_LIVE_PATCHING) += livepatch.o
> > +
> > +livepatch-objs := core.o
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > new file mode 100644
> > index 0000000..0004a71
> > --- /dev/null
> > +++ b/kernel/livepatch/core.c
> > @@ -0,0 +1,929 @@
> > +/*
> > + * 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;
> > +
> > +static bool klp_is_module(struct klp_object *obj)
> > +{
> > + return obj->name;
> > +}
> > +
> > +static bool klp_is_object_loaded(struct klp_object *obj)
> > +{
> > + return !obj->name || obj->mod;
> > +}
> > +
> > +/* sets obj->mod if object is not vmlinux and module is found */
> > +static void klp_find_object_module(struct klp_object *obj)
> > +{
> > + if (!klp_is_module(obj))
> > + return;
> > +
> > + 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);
> > +}
> > +
> > +/* klp_mutex must be held by caller */
> > +static bool klp_is_patch_registered(struct klp_patch *patch)
> > +{
> > + struct klp_patch *mypatch;
> > +
> > + list_for_each_entry(mypatch, &klp_patches, list)
> > + if (mypatch == patch)
> > + return true;
> > +
> > + return false;
> > +}
> > +
> > +static bool klp_initialized(void)
> > +{
> > + return klp_root_kobj;
> > +}
> > +
> > +struct klp_find_arg {
> > + const char *objname;
> > + const char *name;
> > + unsigned long addr;
> > + /*
> > + * If count == 0, the symbol was not found. If count == 1, a unique
> > + * match was found and addr is set. If count > 1, there is
> > + * unresolvable ambiguity among "count" number of symbols with the same
> > + * name in the same object.
> > + */
> > + unsigned long count;
> > +};
> > +
> > +static int klp_find_callback(void *data, const char *name,
> > + struct module *mod, unsigned long addr)
> > +{
> > + struct klp_find_arg *args = data;
> > +
> > + if ((mod && !args->objname) || (!mod && args->objname))
> > + return 0;
> > +
> > + if (strcmp(args->name, name))
> > + return 0;
> > +
> > + if (args->objname && strcmp(args->objname, mod->name))
> > + return 0;
> > +
> > + /*
> > + * args->addr might be overwritten if another match is found
> > + * but klp_find_object_symbol() handles this and only returns the
> > + * addr if count == 1.
> > + */
> > + args->addr = addr;
> > + args->count++;
> > +
> > + return 0;
> > +}
> > +
> > +static int klp_find_object_symbol(const char *objname, const char *name,
> > + unsigned long *addr)
> > +{
> > + struct klp_find_arg args = {
> > + .objname = objname,
> > + .name = name,
> > + .addr = 0,
> > + .count = 0
> > + };
> > +
> > + kallsyms_on_each_symbol(klp_find_callback, &args);
> > +
> > + if (args.count == 0)
> > + pr_err("symbol '%s' not found in symbol table\n", name);
> > + else if (args.count > 1)
> > + pr_err("unresolvable ambiguity (%lu matches) on symbol '%s' in object '%s'\n",
> > + args.count, name, objname);
> > + else {
> > + *addr = args.addr;
> > + return 0;
> > + }
> > +
> > + *addr = 0;
> > + return -EINVAL;
> > +}
> > +
> > +struct klp_verify_args {
> > + const char *name;
> > + const unsigned long addr;
> > +};
> > +
> > +static int klp_verify_callback(void *data, const char *name,
> > + struct module *mod, unsigned long addr)
> > +{
> > + struct klp_verify_args *args = data;
> > +
> > + if (!mod &&
> > + !strcmp(args->name, name) &&
> > + args->addr == addr)
> > + return 1;
> > +
> > + return 0;
> > +}
> > +
> > +static int klp_verify_vmlinux_symbol(const char *name, unsigned long addr)
> > +{
> > + struct klp_verify_args args = {
> > + .name = name,
> > + .addr = addr,
> > + };
> > +
> > + if (kallsyms_on_each_symbol(klp_verify_callback, &args))
> > + return 0;
> > +
> > + pr_err("symbol '%s' not found at specified address 0x%016lx, kernel mismatch?",
> > + name, addr);
> > + return -EINVAL;
> > +}
> > +
> > +static int klp_find_verify_func_addr(struct klp_object *obj,
> > + struct klp_func *func)
> > +{
> > + int ret;
> > +
> > +#if defined(CONFIG_RANDOMIZE_BASE)
> > + /* KASLR is enabled, disregard old_addr from user */
> > + func->old_addr = 0;
> > +#endif
> > +
> > + if (!func->old_addr || klp_is_module(obj))
> > + ret = klp_find_object_symbol(obj->name, func->old_name,
> > + &func->old_addr);
> > + else
> > + ret = klp_verify_vmlinux_symbol(func->old_name,
> > + func->old_addr);
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > + * external symbols are located outside the parent object (where the parent
> > + * object is either vmlinux or the kmod being patched).
> > + */
> > +static int klp_find_external_symbol(struct module *pmod, const char *name,
> > + unsigned long *addr)
> > +{
> > + const struct kernel_symbol *sym;
> > +
> > + /* first, check if it's an exported symbol */
> > + preempt_disable();
> > + sym = find_symbol(name, NULL, NULL, true, true);
> > + preempt_enable();
> > + if (sym) {
> > + *addr = sym->value;
> > + return 0;
> > + }
> > +
> > + /* otherwise check if it's in another .o within the patch module */
> > + return klp_find_object_symbol(pmod->name, name, addr);
> > +}
> > +
>
> Shouldn't the code above be asbtracted out for anyone searching for
> kernel symbols? Its only KLP at the moment, but I am sure these are
> reusable bits

The kallsyms API is not the best, probably because it is hard to predict
how people are going to want to use it. Right now, I can't think of a
reason that another kernel user would want to do this. However, if
someone does need the same functionality later, I could be in favor of
abstracting it in kallsyms to avoid code duplication. I just don't
think we need to do that proactively.

>
> > +static int klp_write_object_relocations(struct module *pmod,
> > + struct klp_object *obj)
> > +{
> > + int ret;
> > + struct klp_reloc *reloc;
> > +
> > + if (WARN_ON(!klp_is_object_loaded(obj)))
> > + return -EINVAL;
> > +
> > + if (WARN_ON(!obj->relocs))
> > + return -EINVAL;
> > +
> > + for (reloc = obj->relocs; reloc->name; reloc++) {
> > + if (!klp_is_module(obj)) {
> > + ret = klp_verify_vmlinux_symbol(reloc->name,
> > + reloc->val);
> > + if (ret)
> > + return ret;
> > + } else {
> > + /* module, reloc->val needs to be discovered */
> > + if (reloc->external)
> > + ret = klp_find_external_symbol(pmod,
> > + reloc->name,
> > + &reloc->val);
> > + else
> > + ret = klp_find_object_symbol(obj->mod->name,
> > + reloc->name,
> > + &reloc->val);
> > + if (ret)
> > + return ret;
> > + }
> > + ret = klp_write_module_reloc(pmod, reloc->type, reloc->loc,
> > + reloc->val + reloc->addend);
>
> An iterative loop can be expensive -- state transitions from page
> rw/ro frequently! Can we optimize?

It is expensive, but it is only done once at patch or module-to-be-patched
load time. Optimizing it would probably require sorting the relocations
by address and then doing all the relocations in each page all at once.

If the patches every get so large that this becomes a problem, we can do
something about it at that point in time. No need to add complexity
optimizing something that is done once and won't make a measurable
difference with the patch sizes we anticipate.

>
> > + if (ret) {
> > + pr_err("relocation failed for symbol '%s' at 0x%016lx (%d)\n",
> > + reloc->name, reloc->val, ret);
> > + return ret;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void notrace klp_ftrace_handler(unsigned long ip,
> > + unsigned long parent_ip,
> > + struct ftrace_ops *ops,
> > + struct pt_regs *regs)
> > +{
> > + struct klp_func *func = ops->private;
> > +
> > + regs->ip = (unsigned long)func->new_func;
> > +}
> > +
> > +static int klp_disable_func(struct klp_func *func)
> > +{
> > + int ret;
> > +
> > + if (WARN_ON(func->state != KLP_ENABLED))
> > + return -EINVAL;
> > +
> > + if (WARN_ON(!func->old_addr))
> > + return -EINVAL;
> > +
> > + ret = unregister_ftrace_function(func->fops);
> > + if (ret) {
> > + pr_err("failed to unregister ftrace handler for function '%s' (%d)\n",
> > + func->old_name, ret);
> > + return ret;
> > + }
> > +
> > + ret = ftrace_set_filter_ip(func->fops, func->old_addr, 1, 0);
> > + if (ret)
> > + pr_warn("function unregister succeeded but failed to clear the filter\n");
> > +
> > + func->state = KLP_DISABLED;
> > +
> > + return 0;
> > +}
> > +
> > +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",
> > + func->old_name, ret);
> > + return ret;
> > + }
> > +
> > + ret = register_ftrace_function(func->fops);
> > + if (ret) {
> > + pr_err("failed to register ftrace handler for function '%s' (%d)\n",
> > + func->old_name, ret);
> > + ftrace_set_filter_ip(func->fops, func->old_addr, 1, 0);
> > + } else {
> > + func->state = KLP_ENABLED;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int klp_disable_object(struct klp_object *obj)
> > +{
> > + struct klp_func *func;
> > + int ret;
> > +
> > + for (func = obj->funcs; func->old_name; func++) {
> > + if (func->state != KLP_ENABLED)
> > + continue;
> > +
> > + ret = klp_disable_func(func);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + obj->state = KLP_DISABLED;
> > +
> > + return 0;
> > +}
> > +
> > +static int klp_enable_object(struct klp_object *obj)
> > +{
> > + struct klp_func *func;
> > + int ret;
> > +
> > + if (WARN_ON(obj->state != KLP_DISABLED))
> > + return -EINVAL;
> > +
> > + if (WARN_ON(!klp_is_object_loaded(obj)))
> > + return -EINVAL;
> > +
> > + for (func = obj->funcs; func->old_name; func++) {
> > + ret = klp_enable_func(func);
> > + if (ret)
> > + goto unregister;
> > + }
> > + obj->state = KLP_ENABLED;
> > +
> > + return 0;
> > +
> > +unregister:
> > + WARN_ON(klp_disable_object(obj));
> > + return ret;
> > +}
> > +
> > +static int __klp_disable_patch(struct klp_patch *patch)
> > +{
> > + struct klp_object *obj;
> > + int ret;
> > +
> > + 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
> > + *
> > + * Unregisters the patched functions from ftrace.
> > + *
> > + * Return: 0 on success, otherwise error
> > + */
> > +int klp_disable_patch(struct klp_patch *patch)
> > +{
> > + int ret;
> > +
> > + mutex_lock(&klp_mutex);
> > +
> > + if (!klp_is_patch_registered(patch)) {
> > + ret = -EINVAL;
> > + goto err;
> > + }
> > +
> > + if (patch->state == KLP_DISABLED) {
> > + ret = -EINVAL;
> > + goto err;
> > + }
> > +
> > + ret = __klp_disable_patch(patch);
> > +
> > +err:
> > + 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++) {
> > + klp_find_object_module(obj);
> > +
> > + if (!klp_is_object_loaded(obj))
> > + continue;
> > +
> > + ret = klp_enable_object(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;
> > +
> > + mutex_lock(&klp_mutex);
> > +
> > + if (!klp_is_patch_registered(patch)) {
> > + ret = -EINVAL;
> > + goto err;
> > + }
> > +
> > + ret = __klp_enable_patch(patch);
> > +
> > +err:
> > + mutex_unlock(&klp_mutex);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(klp_enable_patch);
> > +
> > +/*
> > + * Sysfs Interface
> > + *
> > + * /sys/kernel/livepatch
> > + * /sys/kernel/livepatch/<patch>
> > + * /sys/kernel/livepatch/<patch>/enabled
> > + * /sys/kernel/livepatch/<patch>/<object>
> > + * /sys/kernel/livepatch/<patch>/<object>/<func>
> > + */
> > +
> > +static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct klp_patch *patch;
> > + int ret;
> > + unsigned long val;
> > +
> > + ret = kstrtoul(buf, 10, &val);
> > + if (ret)
> > + return -EINVAL;
> > +
> > + if (val != KLP_DISABLED && val != KLP_ENABLED)
> > + return -EINVAL;
> > +
> > + patch = container_of(kobj, struct klp_patch, kobj);
> > +
> > + mutex_lock(&klp_mutex);
> > +
> > + if (val == patch->state) {
> > + /* already in requested state */
> > + ret = -EINVAL;
> > + goto err;
> > + }
> > +
> > + if (val == KLP_ENABLED) {
> > + ret = __klp_enable_patch(patch);
> > + if (ret)
> > + goto err;
> > + } else {
> > + ret = __klp_disable_patch(patch);
> > + if (ret)
> > + goto err;
> > + }
> > +
> > + mutex_unlock(&klp_mutex);
> > +
> > + return count;
> > +
> > +err:
> > + mutex_unlock(&klp_mutex);
> > + return ret;
> > +}
> > +
> > +static ssize_t enabled_show(struct kobject *kobj,
> > + struct kobj_attribute *attr, char *buf)
> > +{
> > + struct klp_patch *patch;
> > +
> > + patch = container_of(kobj, struct klp_patch, kobj);
> > + return snprintf(buf, PAGE_SIZE-1, "%d\n", patch->state);
> > +}
> > +
> > +static struct kobj_attribute enabled_kobj_attr = __ATTR_RW(enabled);
> > +static struct attribute *klp_patch_attrs[] = {
> > + &enabled_kobj_attr.attr,
> > + NULL
> > +};
> > +
> > +static void klp_kobj_release_patch(struct kobject *kobj)
> > +{
> > + /*
> > + * Once we have a consistency model we'll need to module_put() the
> > + * patch module here. See klp_register_patch() for more details.
> > + */
> > +}
> > +
> > +static struct kobj_type klp_ktype_patch = {
> > + .release = klp_kobj_release_patch,
> > + .sysfs_ops = &kobj_sysfs_ops,
> > + .default_attrs = klp_patch_attrs,
> > +};
> > +
> > +static void klp_kobj_release_func(struct kobject *kobj)
> > +{
> > + struct klp_func *func;
> > +
> > + func = container_of(kobj, struct klp_func, kobj);
> > + kfree(func->fops);
> > +}
> > +
> > +static struct kobj_type klp_ktype_func = {
> > + .release = klp_kobj_release_func,
> > + .sysfs_ops = &kobj_sysfs_ops,
> > +};
> > +
> > +/*
> > + * Free all functions' kobjects in the array up to some limit. When limit is
> > + * NULL, all kobjects are freed.
> > + */
> > +static void klp_free_funcs_limited(struct klp_object *obj,
> > + struct klp_func *limit)
> > +{
> > + struct klp_func *func;
> > +
> > + for (func = obj->funcs; func->old_name && func != limit; func++)
> > + kobject_put(&func->kobj);
> > +}
> > +
> > +/* Clean up when a patched object is unloaded */
> > +static void klp_free_object_loaded(struct klp_object *obj)
> > +{
> > + struct klp_func *func;
> > +
> > + obj->mod = NULL;
> > +
> > + for (func = obj->funcs; func->old_name; func++)
> > + func->old_addr = 0;
> > +}
> > +
> > +/*
> > + * Free all objects' kobjects in the array up to some limit. When limit is
> > + * NULL, all kobjects are freed.
> > + */
> > +static void klp_free_objects_limited(struct klp_patch *patch,
> > + struct klp_object *limit)
> > +{
> > + struct klp_object *obj;
> > +
> > + for (obj = patch->objs; obj->funcs && obj != limit; obj++) {
> > + klp_free_funcs_limited(obj, NULL);
> > + kobject_put(obj->kobj);
> > + }
> > +}
> > +
> > +static void klp_free_patch(struct klp_patch *patch)
> > +{
> > + klp_free_objects_limited(patch, NULL);
> > + if (!list_empty(&patch->list))
> > + list_del(&patch->list);
> > + kobject_put(&patch->kobj);
> > +}
> > +
> > +static int klp_init_func(struct klp_object *obj, struct klp_func *func)
> > +{
> > + struct ftrace_ops *ops;
> > + int ret;
> > +
> > + ops = kzalloc(sizeof(*ops), GFP_KERNEL);
> > + if (!ops)
> > + return -ENOMEM;
> > +
> > + ops->private = func;
> > + ops->func = klp_ftrace_handler;
> > + ops->flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC;
> > + func->fops = ops;
> > + func->state = KLP_DISABLED;
> > +
> > + ret = kobject_init_and_add(&func->kobj, &klp_ktype_func,
> > + obj->kobj, func->old_name);
> > + if (ret) {
> > + kfree(func->fops);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/* parts of the initialization that is done only when the object is loaded */
> > +static int klp_init_object_loaded(struct klp_patch *patch,
> > + struct klp_object *obj)
> > +{
> > + struct klp_func *func;
> > + int ret;
> > +
> > + if (obj->relocs) {
> > + ret = klp_write_object_relocations(patch->mod, obj);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + for (func = obj->funcs; func->old_name; func++) {
> > + ret = klp_find_verify_func_addr(obj, func);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
> > +{
> > + struct klp_func *func;
> > + int ret;
> > + const char *name;
> > +
> > + if (!obj->funcs)
> > + return -EINVAL;
> > +
> > + obj->state = KLP_DISABLED;
> > +
> > + klp_find_object_module(obj);
> > +
> > + name = klp_is_module(obj) ? obj->name : "vmlinux";
> > + obj->kobj = kobject_create_and_add(name, &patch->kobj);
> > + if (!obj->kobj)
> > + return -ENOMEM;
> > +
> > + for (func = obj->funcs; func->old_name; func++) {
> > + ret = klp_init_func(obj, func);
> > + if (ret)
> > + goto free;
> > + }
> > +
> > + if (klp_is_object_loaded(obj)) {
> > + ret = klp_init_object_loaded(patch, obj);
> > + if (ret)
> > + goto free;
> > + }
> > +
> > + return 0;
> > +
> > +free:
> > + klp_free_funcs_limited(obj, func);
> > + kobject_put(obj->kobj);
> > + return ret;
> > +}
> > +
> > +static int klp_init_patch(struct klp_patch *patch)
> > +{
> > + struct klp_object *obj;
> > + int ret;
> > +
> > + if (!patch->objs)
> > + return -EINVAL;
> > +
> > + mutex_lock(&klp_mutex);
> > +
> > + patch->state = KLP_DISABLED;
> > +
> > + ret = kobject_init_and_add(&patch->kobj, &klp_ktype_patch,
> > + klp_root_kobj, patch->mod->name);
> > + if (ret)
> > + goto unlock;
> > +
> > + for (obj = patch->objs; obj->funcs; obj++) {
> > + ret = klp_init_object(patch, obj);
> > + if (ret)
> > + goto free;
> > + }
> > +
> > + list_add(&patch->list, &klp_patches);
> > +
> > + mutex_unlock(&klp_mutex);
> > +
> > + return 0;
> > +
> > +free:
> > + klp_free_objects_limited(patch, obj);
> > + kobject_put(&patch->kobj);
> > +unlock:
> > + mutex_unlock(&klp_mutex);
> > + return ret;
> > +}
> > +
> > +/**
> > + * klp_unregister_patch() - unregisters a patch
> > + * @patch: Disabled patch to be unregistered
> > + *
> > + * Frees the data structures and removes the sysfs interface.
> > + *
> > + * Return: 0 on success, otherwise error
> > + */
> > +int klp_unregister_patch(struct klp_patch *patch)
> > +{
> > + int ret = 0;
> > +
> > + mutex_lock(&klp_mutex);
> > +
> > + if (!klp_is_patch_registered(patch)) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + if (patch->state == KLP_ENABLED) {
> > + ret = -EBUSY;
> > + goto out;
> > + }
> > +
> > + klp_free_patch(patch);
> > +
> > +out:
> > + mutex_unlock(&klp_mutex);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(klp_unregister_patch);
> > +
> > +/**
> > + * klp_register_patch() - registers a patch
> > + * @patch: Patch to be registered
> > + *
> > + * Initializes the data structure associated with the patch and
> > + * creates the sysfs interface.
> > + *
> > + * Return: 0 on success, otherwise error
> > + */
> > +int klp_register_patch(struct klp_patch *patch)
> > +{
> > + int ret;
> > +
> > + if (!klp_initialized())
> > + return -ENODEV;
> > +
> > + if (!patch || !patch->mod)
> > + return -EINVAL;
> > +
> > + /*
> > + * 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.
> > + */
> > + if (!try_module_get(patch->mod))
> > + return -ENODEV;
> > +
> > + ret = klp_init_patch(patch);
> > + if (ret)
> > + module_put(patch->mod);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(klp_register_patch);
> > +
> > +static void klp_module_notify_coming(struct klp_patch *patch,
> > + struct klp_object *obj)
> > +{
> > + struct module *pmod = patch->mod;
> > + struct module *mod = obj->mod;
> > + int ret;
> > +
> > + ret = klp_init_object_loaded(patch, obj);
> > + if (ret)
> > + goto err;
> > +
> > + if (patch->state == KLP_DISABLED)
> > + return;
> > +
> > + pr_notice("applying patch '%s' to loading module '%s'\n",
> > + pmod->name, mod->name);
> > +
> > + ret = klp_enable_object(obj);
> > + if (!ret)
> > + return;
> > +
> > +err:
> > + pr_warn("failed to apply patch '%s' to module '%s' (%d)\n",
> > + pmod->name, mod->name, ret);
> > +}
> > +
> > +static void klp_module_notify_going(struct klp_patch *patch,
> > + struct klp_object *obj)
> > +{
> > + struct module *pmod = patch->mod;
> > + struct module *mod = obj->mod;
> > + int ret;
> > +
> > + if (patch->state == KLP_DISABLED)
> > + goto disabled;
> > +
> > + 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);
> > +
> > +disabled:
> > + klp_free_object_loaded(obj);
> > +}
> > +
> > +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) {
> > + for (obj = patch->objs; obj->funcs; obj++) {
> > + if (!klp_is_module(obj) || strcmp(obj->name, mod->name))
> > + continue;
> > +
> > + if (action == MODULE_STATE_COMING) {
> > + obj->mod = mod;
> > + klp_module_notify_coming(patch, obj);
> > + } else /* MODULE_STATE_GOING */
> > + klp_module_notify_going(patch, obj);
> > +
> > + break;
> > + }
> > + }
> > +
> > + mutex_unlock(&klp_mutex);
> > +
> > + return 0;
> > +}
> > +
> > +static struct notifier_block klp_module_nb = {
> > + .notifier_call = klp_module_notify,
> > + .priority = INT_MIN+1, /* called late but before ftrace notifier */
> > +};
> > +
> > +static int klp_init(void)
> > +{
> > + int ret;
> > +
> > + ret = register_module_notifier(&klp_module_nb);
> > + if (ret)
> > + return ret;
> > +
> > + klp_root_kobj = kobject_create_and_add("livepatch", kernel_kobj);
> > + if (!klp_root_kobj) {
> > + ret = -ENOMEM;
> > + goto unregister;
> > + }
> > +
> > + return 0;
> > +
> > +unregister:
> > + unregister_module_notifier(&klp_module_nb);
> > + return ret;
> > +}
> > +
> > +module_init(klp_init);
>
> Looks good otherwise

Thanks a lot for the review!

Seth

>
> Reviewed-by: Balbir Singh <[email protected]>

2014-12-16 20:14:42

[permalink] [raw]

Subject: Re: [PATCHv7 0/3] Kernel Live Patching

On Tue, 16 Dec 2014, Balbir Singh wrote:

> Could you describe what this does to signing? I presume the patched
> module should cause a taint on module signing?

Hmm, why should it?

- if module signatures are enforced on the system in question, the module
with the patch itself has to be signed as well, otherwise it will not be
loaded by the kernel at all in the first place

- after the trusted (signed) module with the patch is loaded, this is in
principle no way different than other self-modifications the kernel is
performing all the time (static keys, alternatives, kprobes, ...)

Yes, we are tainting a kernel, but for reasons completely unrelated to
module signing.

I actually think that module signing doesn't play any role whatsoever in
what this patchset is doing.

Thanks,

--
Jiri Kosina
SUSE Labs

2014-12-17 03:42:12

[permalink] [raw]

Subject: Re: [PATCHv7 0/3] Kernel Live Patching

On Wed, Dec 17, 2014 at 12:35 AM, Seth Jennings <[email protected]> wrote:
> On Tue, Dec 16, 2014 at 11:45:12PM +0530, Balbir Singh wrote:
>> On Tue, Dec 16, 2014 at 11:28 PM, Seth Jennings <[email protected]> wrote:
>> >
>> > Changelog:
>> >
>> > Thanks for all the feedback!
>> >
>>
>> Could you describe what this does to signing? I presume the patched
>> module should cause a taint on module signing?
>
> The patch module can be signed to avoid the taint of being unsigned,
> assuming you have the signing key for the kernel you are running.
> However we do taint with a new taint flag (see 1/3) to indicate
> that the kernel has been patched.
>

I see, so the assumption is that the module and patch would be signed
with the same key?

Thanks,
Balbir Singh.

2014-12-17 03:43:30

[permalink] [raw]

Subject: Re: [PATCHv7 0/3] Kernel Live Patching

On Wed, Dec 17, 2014 at 1:44 AM, Jiri Kosina <[email protected]> wrote:
> On Tue, 16 Dec 2014, Balbir Singh wrote:
>
>> Could you describe what this does to signing? I presume the patched
>> module should cause a taint on module signing?
>
> Hmm, why should it?
>

I wanted to clarify it from a different perspective

If the base image is signed by X and the patched module is signed by
Y, is that supported. What does it imply in the case of live-patching?

Balbir Singh

2014-12-17 06:46:15

[permalink] [raw]

Subject: Re: [PATCHv7 0/3] Kernel Live Patching

On Wed, 17 Dec 2014, Balbir Singh wrote:

> >> Could you describe what this does to signing? I presume the patched
> >> module should cause a taint on module signing?
> >
> > Hmm, why should it?
>
> I wanted to clarify it from a different perspective
>
> If the base image is signed by X and the patched module is signed by
> Y, is that supported. What does it imply in the case of live-patching?

Why should that matter? Both are signed by keys that kernel is configured
to trust, which makes them equal (even though they are technically
different).

--
Jiri Kosina
SUSE Labs

2014-12-17 07:52:24

[permalink] [raw]

Subject: Re: [PATCHv7 0/3] Kernel Live Patching

On Wed, Dec 17, 2014 at 12:16 PM, Jiri Kosina <[email protected]> wrote:
> On Wed, 17 Dec 2014, Balbir Singh wrote:
>
>> >> Could you describe what this does to signing? I presume the patched
>> >> module should cause a taint on module signing?
>> >
>> > Hmm, why should it?
>>
>> I wanted to clarify it from a different perspective
>>
>> If the base image is signed by X and the patched module is signed by
>> Y, is that supported. What does it imply in the case of live-patching?
>
> Why should that matter? Both are signed by keys that kernel is configured
> to trust, which makes them equal (even though they are technically
> different).
>

I am not sure they are equal, others can comment

Balbir Singh

2014-12-17 12:41:04

by Vojtech Pavlik

[permalink] [raw]

Subject: Re: [PATCHv7 0/3] Kernel Live Patching

On Wed, Dec 17, 2014 at 01:22:21PM +0530, Balbir Singh wrote:
> On Wed, Dec 17, 2014 at 12:16 PM, Jiri Kosina <[email protected]> wrote:
> > On Wed, 17 Dec 2014, Balbir Singh wrote:
> >
> >> >> Could you describe what this does to signing? I presume the patched
> >> >> module should cause a taint on module signing?
> >> >
> >> > Hmm, why should it?
> >>
> >> I wanted to clarify it from a different perspective
> >>
> >> If the base image is signed by X and the patched module is signed by
> >> Y, is that supported. What does it imply in the case of live-patching?
> >
> > Why should that matter? Both are signed by keys that kernel is configured
> > to trust, which makes them equal (even though they are technically
> > different).
> >
>
> I am not sure they are equal, others can comment

Since any loaded kernel module can do virtually anything on a machine,
there can only be one level of trust. As such, all trusted keys are
equally trusted.

--
Vojtech Pavlik
Director SUSE Labs

2014-12-17 14:06:24

by Miroslav Benes

[permalink] [raw]

Subject: Re: [PATCHv7 1/3] kernel: add TAINT_LIVEPATCH

On Tue, 16 Dec 2014, Seth Jennings wrote:

> This adds a new taint flag to indicate when the kernel or a kernel
> module has been live patched. This will provide a clean indication in
> bug reports that live patching was used.
>
> Additionally, if the crash occurs in a live patched function, the live
> patch module will appear beside the patched function in the backtrace.
>
> Signed-off-by: Seth Jennings <[email protected]>

Reviewed-by: Miroslav Benes <[email protected]>

Thanks,
--
Miroslav Benes
SUSE Labs

2014-12-17 14:07:00

by Miroslav Benes

[permalink] [raw]

Subject: Re: [PATCHv7 3/3] samples: add sample live patching module

On Tue, 16 Dec 2014, Seth Jennings wrote:

> Add a sample live patching module.
>
> Signed-off-by: Seth Jennings <[email protected]>

Reviewed-by: Miroslav Benes <[email protected]>

Thanks,
--
Miroslav Benes
SUSE Labs

2014-12-17 14:09:05

by Miroslav Benes

[permalink] [raw]

Subject: Re: [PATCHv7 2/3] kernel: add support for live patching

On Tue, 16 Dec 2014, Seth Jennings wrote:

> 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]>
> Signed-off-by: Josh Poimboeuf <[email protected]>

No problem there for me. So, for my patches you have

Signed-off-by: Miroslav Benes <[email protected]>

and for the rest

Reviewed-by: Miroslav Benes <[email protected]>

Thanks. Great work!
--
Miroslav Benes
SUSE Labs

2014-12-17 15:15:12

[permalink] [raw]

Subject: Re: [PATCHv7 1/3] kernel: add TAINT_LIVEPATCH

On Tue 2014-12-16 11:58:18, Seth Jennings wrote:
> This adds a new taint flag to indicate when the kernel or a kernel
> module has been live patched. This will provide a clean indication in
> bug reports that live patching was used.
>
> Additionally, if the crash occurs in a live patched function, the live
> patch module will appear beside the patched function in the backtrace.
>
> Signed-off-by: Seth Jennings <[email protected]>

Reviewed-by: Petr Mladek <[email protected]>

Best Regards,
Petr

2014-12-17 15:23:25

[permalink] [raw]

Subject: Re: [PATCHv7 3/3] samples: add sample live patching module

On Tue 2014-12-16 11:58:20, Seth Jennings wrote:
> Add a sample live patching module.
>
> Signed-off-by: Seth Jennings <[email protected]>

Reviewed-by: Petr Mladek <[email protected]>

Best Regards,
Petr

2014-12-17 17:15:08

[permalink] [raw]

Subject: Re: [PATCHv7 0/3] Kernel Live Patching

On Tue, Dec 16, 2014 at 11:58:17AM -0600, Seth Jennings wrote:
> Changelog:
>
> Thanks for all the feedback!
>
> I think that we have something that is workable for everyone now. Barring
> functional defects, I think we should put a hold on any nits to avoid churn for
> the moment and start gathering acks so that we are in a position to go into
> -next when the 3.19 window closes.
>
> Also, I don't think patches 1/3 and 3/3 have changed since v5, so if you reviewed
> them then, they haven't changed.

For the set:

Reviewed-by: Josh Poimboeuf <[email protected]>

--
Josh

2014-12-18 11:44:12

[permalink] [raw]

Subject: Re: [PATCHv7 1/3] kernel: add TAINT_LIVEPATCH

(2014/12/17 2:58), Seth Jennings wrote:
> This adds a new taint flag to indicate when the kernel or a kernel
> module has been live patched. This will provide a clean indication in
> bug reports that live patching was used.
>
> Additionally, if the crash occurs in a live patched function, the live
> patch module will appear beside the patched function in the backtrace.
>
> Signed-off-by: Seth Jennings <[email protected]>

Looks good to me.

Reviewed-by: Masami Hiramatsu <[email protected]>

Thanks!

> ---
> Documentation/oops-tracing.txt | 2 ++
> Documentation/sysctl/kernel.txt | 1 +
> include/linux/kernel.h | 1 +
> kernel/panic.c | 2 ++
> 4 files changed, 6 insertions(+)
>
> diff --git a/Documentation/oops-tracing.txt b/Documentation/oops-tracing.txt
> index beefb9f..f3ac05c 100644
> --- a/Documentation/oops-tracing.txt
> +++ b/Documentation/oops-tracing.txt
> @@ -270,6 +270,8 @@ characters, each representing a particular tainted value.
>
> 15: 'L' if a soft lockup has previously occurred on the system.
>
> + 16: 'K' if the kernel has been live patched.
> +
> The primary reason for the 'Tainted: ' string is to tell kernel
> debuggers if this is a clean kernel or if anything unusual has
> occurred. Tainting is permanent: even if an offending module is
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index 75511ef..83ab256 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -843,6 +843,7 @@ can be ORed together:
> 8192 - An unsigned module has been loaded in a kernel supporting module
> signature.
> 16384 - A soft lockup has previously occurred on the system.
> +32768 - The kernel has been live patched.
>
> ==============================================================
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 5449d2f..d03e3de 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -471,6 +471,7 @@ extern enum system_states {
> #define TAINT_OOT_MODULE 12
> #define TAINT_UNSIGNED_MODULE 13
> #define TAINT_SOFTLOCKUP 14
> +#define TAINT_LIVEPATCH 15
>
> extern const char hex_asc[];
> #define hex_asc_lo(x) hex_asc[((x) & 0x0f)]
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 4d8d6f9..8136ad7 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -226,6 +226,7 @@ static const struct tnt tnts[] = {
> { TAINT_OOT_MODULE, 'O', ' ' },
> { TAINT_UNSIGNED_MODULE, 'E', ' ' },
> { TAINT_SOFTLOCKUP, 'L', ' ' },
> + { TAINT_LIVEPATCH, 'K', ' ' },
> };
>
> /**
> @@ -246,6 +247,7 @@ static const struct tnt tnts[] = {
> * 'O' - Out-of-tree module has been loaded.
> * 'E' - Unsigned module has been loaded.
> * 'L' - A soft lockup has previously occurred.
> + * 'K' - Kernel has been live patched.
> *
> * The string is overwritten by the next call to print_tainted().
> */
>

--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

2014-12-18 11:55:31

[permalink] [raw]

Subject: Re: [PATCHv7 0/3] Kernel Live Patching

(2014/12/17 2:58), Seth Jennings wrote:
> Changelog:
>
> Thanks for all the feedback!
>
> I think that we have something that is workable for everyone now. Barring
> functional defects, I think we should put a hold on any nits to avoid churn for
> the moment and start gathering acks so that we are in a position to go into
> -next when the 3.19 window closes.
>
> Also, I don't think patches 1/3 and 3/3 have changed since v5, so if you reviewed
> them then, they haven't changed.
>
> changes in v7:
> - TODO: set IPMODIFY (not a blocker to moving forward)

Why don't you set this?
IPMODIFY series are not completely applied yet, but you can already
use the flag. All you need is just set it :)

Thank you,

--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

2014-12-18 13:36:59

[permalink] [raw]

Subject: Re: [PATCHv7 2/3] kernel: add support for live patching

On Tue 2014-12-16 11:58:19, Seth Jennings wrote:
> 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]>
> Signed-off-by: Josh Poimboeuf <[email protected]>

I like the current state. Thanks a lot for the effort.

Reviewed-by: Petr Mladek <[email protected]>

and for my changes:

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

Best Regards,
Petr

2014-12-18 15:50:20

[permalink] [raw]

Subject: [PATCH] livepatch: use FTRACE_OPS_FL_IPMODIFY

On Thu, Dec 18, 2014 at 08:55:21PM +0900, Masami Hiramatsu wrote:
> (2014/12/17 2:58), Seth Jennings wrote:
> > changes in v7:
> > - TODO: set IPMODIFY (not a blocker to moving forward)
>
> Why don't you set this?
> IPMODIFY series are not completely applied yet, but you can already
> use the flag. All you need is just set it :)

Yeah, I don't see any reason why we can't start using this flag now.
How about we add this patch to the queue?

-->8--

Subject: livepatch: use FTRACE_OPS_FL_IPMODIFY

Use the FTRACE_OPS_FL_IPMODIFY flag to prevent conflicts with other
ftrace users who also modify regs->ip.

Signed-off-by: Josh Poimboeuf <[email protected]>
---
kernel/livepatch/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 0004a71..bdd99975 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -640,7 +640,8 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func)

ops->private = func;
ops->func = klp_ftrace_handler;
- ops->flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC;
+ ops->flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC |
+ FTRACE_FL_IPMODIFY;
func->fops = ops;
func->state = KLP_DISABLED;

--
2.1.0

2014-12-18 15:52:09

[permalink] [raw]

Subject: Re: [PATCHv7 1/3] kernel: add TAINT_LIVEPATCH

On Thu, 18 Dec 2014, Masami Hiramatsu wrote:

> > This adds a new taint flag to indicate when the kernel or a kernel
> > module has been live patched. This will provide a clean indication in
> > bug reports that live patching was used.
> >
> > Additionally, if the crash occurs in a live patched function, the live
> > patch module will appear beside the patched function in the backtrace.
> >
> > Signed-off-by: Seth Jennings <[email protected]>
>
> Looks good to me.
>
> Reviewed-by: Masami Hiramatsu <[email protected]>

Masami, just for completness -- is this just for 1/3, or for the whole
series? (so that I get it right once I am applying the patches to git
tree).

Thanks,

--
Jiri Kosina
SUSE Labs

2014-12-18 15:57:23

[permalink] [raw]

Subject: Re: [PATCHv7 0/3] Kernel Live Patching

On Wed, Dec 17, 2014 at 10:21:47AM -0600, Josh Poimboeuf wrote:
> On Tue, Dec 16, 2014 at 11:58:17AM -0600, Seth Jennings wrote:
> > Changelog:
> >
> > Thanks for all the feedback!
> >
> > I think that we have something that is workable for everyone now. Barring
> > functional defects, I think we should put a hold on any nits to avoid churn for
> > the moment and start gathering acks so that we are in a position to go into
> > -next when the 3.19 window closes.
> >
> > Also, I don't think patches 1/3 and 3/3 have changed since v5, so if you reviewed
> > them then, they haven't changed.
>
> For the set:
>
> Reviewed-by: Josh Poimboeuf <[email protected]>

Hrm, since I'm listed as a maintainer, I should probably change this to:

Acked-by: Josh Poimboeuf <[email protected]>

--
Josh

2014-12-19 03:35:03

[permalink] [raw]

Subject: Re: [PATCH] livepatch: use FTRACE_OPS_FL_IPMODIFY

(2014/12/19 0:49), Josh Poimboeuf wrote:
> On Thu, Dec 18, 2014 at 08:55:21PM +0900, Masami Hiramatsu wrote:
>> (2014/12/17 2:58), Seth Jennings wrote:
>>> changes in v7:
>>> - TODO: set IPMODIFY (not a blocker to moving forward)
>>
>> Why don't you set this?
>> IPMODIFY series are not completely applied yet, but you can already
>> use the flag. All you need is just set it :)
>
> Yeah, I don't see any reason why we can't start using this flag now.
> How about we add this patch to the queue?
>
> -->8--
>
> Subject: livepatch: use FTRACE_OPS_FL_IPMODIFY
>
> Use the FTRACE_OPS_FL_IPMODIFY flag to prevent conflicts with other
> ftrace users who also modify regs->ip.
>
> Signed-off-by: Josh Poimboeuf <[email protected]>

Looks good to me :)

Acked-by: Masami Hiramatsu <[email protected]>

Thanks!

> ---
> kernel/livepatch/core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 0004a71..bdd99975 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -640,7 +640,8 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func)
>
> ops->private = func;
> ops->func = klp_ftrace_handler;
> - ops->flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC;
> + ops->flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC |
> + FTRACE_FL_IPMODIFY;
> func->fops = ops;
> func->state = KLP_DISABLED;
>
>

--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

2014-12-19 03:36:12

[permalink] [raw]

Subject: Re: Re: [PATCHv7 1/3] kernel: add TAINT_LIVEPATCH

(2014/12/19 0:52), Jiri Kosina wrote:
> On Thu, 18 Dec 2014, Masami Hiramatsu wrote:
>
>>> This adds a new taint flag to indicate when the kernel or a kernel
>>> module has been live patched. This will provide a clean indication in
>>> bug reports that live patching was used.
>>>
>>> Additionally, if the crash occurs in a live patched function, the live
>>> patch module will appear beside the patched function in the backtrace.
>>>
>>> Signed-off-by: Seth Jennings <[email protected]>
>>
>> Looks good to me.
>>
>> Reviewed-by: Masami Hiramatsu <[email protected]>
>
> Masami, just for completness -- is this just for 1/3, or for the whole
> series? (so that I get it right once I am applying the patches to git
> tree).

Yeah, Now all the patches in this series are OK to me :)

Feel free to add my Reviewed-by.

Reviewed-by: Masami Hiramatsu <[email protected]>

for this series :)

Thank you!

>
> Thanks,
>

--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

2014-12-19 05:37:51

by Li Bin

[permalink] [raw]

Subject: [kpatch] [PATCH] livepatch v7: move x86 specific ftrace handler code to arch/x86

The execution flow redirection related implemention in the livepatch
ftrace handler is depended on the specific architecture. This patch
introduces klp_arch_set_pc(like kgdb_arch_set_pc) interface to change
the pt_regs.

Signed-off-by: Li Bin <[email protected]>
---
arch/x86/include/asm/livepatch.h | 5 +++++
kernel/livepatch/core.c | 2 +-
2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/livepatch.h b/arch/x86/include/asm/livepatch.h
index c2ae592..4cdec4e 100644
--- a/arch/x86/include/asm/livepatch.h
+++ b/arch/x86/include/asm/livepatch.h
@@ -21,6 +21,7 @@
#define _ASM_X86_LIVEPATCH_H

#include <linux/module.h>
+#include <linux/ftrace.h>

#ifdef CONFIG_LIVE_PATCHING
#ifndef CC_USING_FENTRY
@@ -29,6 +30,10 @@
extern int klp_write_module_reloc(struct module *mod, unsigned long type,
unsigned long loc, unsigned long value);

+static inline void klp_arch_set_pc(struct pt_regs *regs, unsigned long ip)
+{
+ regs->ip = ip;
+}
#else
#error Live patching support is disabled; check CONFIG_LIVE_PATCHING
#endif
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 0004a71..c4c04fd 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -271,7 +271,7 @@ static void notrace klp_ftrace_handler(unsigned long ip,
{
struct klp_func *func = ops->private;

- regs->ip = (unsigned long)func->new_func;
+ klp_arch_set_pc(regs, (unsigned long)func->new_func);
}

static int klp_disable_func(struct klp_func *func)
--
1.7.1

2014-12-19 06:11:42

by Li Bin

[permalink] [raw]

Subject: [kpatch] [PATCH] livepatch v7: move x86 specific ftrace handler code to arch/x86

2014-12-19 06:12:49

by Li Bin

[permalink] [raw]

Subject: Re: [kpatch] [PATCH] livepatch v7: move x86 specific ftrace handler code to arch/x86

Sorry! Bad format, please ignore this patch.

On 2014/12/19 13:37, Li Bin wrote:
> The execution flow redirection related implemention in the livepatch
> ftrace handler is depended on the specific architecture. This patch
> introduces klp_arch_set_pc(like kgdb_arch_set_pc) interface to change
> the pt_regs.
>
> Signed-off-by: Li Bin <[email protected]>

2014-12-19 07:31:44

[permalink] [raw]

Subject: Re: [kpatch] [PATCH] livepatch v7: move x86 specific ftrace handler code to arch/x86

On Fri, 19 Dec 2014, Li Bin wrote:

> The execution flow redirection related implemention in the livepatch
> ftrace handler is depended on the specific architecture. This patch
> introduces klp_arch_set_pc(like kgdb_arch_set_pc) interface to change
> the pt_regs.
>
> Signed-off-by: Li Bin <[email protected]>

Good catch, thanks.

Reviewed-by: Jiri Kosina <[email protected]>

--
Jiri Kosina
SUSE Labs

2014-12-19 07:40:01

[permalink] [raw]

Subject: Re: [PATCHv7 2/3] kernel: add support for live patching

On Tue, 16 Dec 2014, Seth Jennings wrote:

> 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]>
> Signed-off-by: Josh Poimboeuf <[email protected]>

I have finally finished reviewing this as well.

Reviewed-by: Jiri Kosina <[email protected]>

and

Signed-off-by: Jiri Kosina <[email protected]>

for the changes I contributed.

I'll wait a bit more to eventually gather more acks / review comments, and
will then push it to git.kernel.org repository (with SUSE copyright added
to livepatch.c) and have included in linux-next, as discussed before.

Thanks!

--
Jiri Kosina
SUSE Labs

2014-12-19 09:43:05

[permalink] [raw]

Subject: Re: [PATCH] livepatch: use FTRACE_OPS_FL_IPMODIFY

On Thu 2014-12-18 09:49:35, Josh Poimboeuf wrote:
> On Thu, Dec 18, 2014 at 08:55:21PM +0900, Masami Hiramatsu wrote:
> > (2014/12/17 2:58), Seth Jennings wrote:
> > > changes in v7:
> > > - TODO: set IPMODIFY (not a blocker to moving forward)
> >
> > Why don't you set this?
> > IPMODIFY series are not completely applied yet, but you can already
> > use the flag. All you need is just set it :)
>
> Yeah, I don't see any reason why we can't start using this flag now.
> How about we add this patch to the queue?
>
> -->8--
>
> Subject: livepatch: use FTRACE_OPS_FL_IPMODIFY
>
> Use the FTRACE_OPS_FL_IPMODIFY flag to prevent conflicts with other
> ftrace users who also modify regs->ip.
>
> Signed-off-by: Josh Poimboeuf <[email protected]>

It makes sense. The flag is available even in 3.19.

Reviewed-by: Petr Mladek <[email protected]>

> ---
> kernel/livepatch/core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 0004a71..bdd99975 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -640,7 +640,8 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func)
>
> ops->private = func;
> ops->func = klp_ftrace_handler;
> - ops->flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC;
> + ops->flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC |
> + FTRACE_FL_IPMODIFY;
> func->fops = ops;
> func->state = KLP_DISABLED;
>
> --
> 2.1.0
>

2014-12-19 14:24:08

[permalink] [raw]

Subject: Re: [kpatch] [PATCH] livepatch v7: move x86 specific ftrace handler code to arch/x86

On Fri, Dec 19, 2014 at 02:11:17PM +0800, Li Bin wrote:
> The execution flow redirection related implemention in the livepatch
> ftrace handler is depended on the specific architecture. This patch
> introduces klp_arch_set_pc(like kgdb_arch_set_pc) interface to change
> the pt_regs.
>
> Signed-off-by: Li Bin <[email protected]>
> ---
> arch/x86/include/asm/livepatch.h | 5 +++++
> kernel/livepatch/core.c | 2 +-
> 2 files changed, 6 insertions(+), 1 deletions(-)

Acked-by: Josh Poimboeuf <[email protected]>

--
Josh

2014-12-22 17:34:20

[permalink] [raw]

Subject: Re: [PATCH] livepatch: use FTRACE_OPS_FL_IPMODIFY

On Fri 2014-12-19 10:43:35, Petr Mladek wrote:
> On Thu 2014-12-18 09:49:35, Josh Poimboeuf wrote:
> > On Thu, Dec 18, 2014 at 08:55:21PM +0900, Masami Hiramatsu wrote:
> > > (2014/12/17 2:58), Seth Jennings wrote:
> > > > changes in v7:
> > > > - TODO: set IPMODIFY (not a blocker to moving forward)
> > >
> > > Why don't you set this?
> > > IPMODIFY series are not completely applied yet, but you can already
> > > use the flag. All you need is just set it :)
> >
> > Yeah, I don't see any reason why we can't start using this flag now.
> > How about we add this patch to the queue?
> >
> > -->8--
> >
> > Subject: livepatch: use FTRACE_OPS_FL_IPMODIFY
> >
> > Use the FTRACE_OPS_FL_IPMODIFY flag to prevent conflicts with other
> > ftrace users who also modify regs->ip.
> >
> > Signed-off-by: Josh Poimboeuf <[email protected]>
>
> It makes sense. The flag is available even in 3.19.
>
> Reviewed-by: Petr Mladek <[email protected]>

Feel free to keep the Reviewed-by if you fix the typo, see below.

> > ---
> > kernel/livepatch/core.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index 0004a71..bdd99975 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> > @@ -640,7 +640,8 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func)
> >
> > ops->private = func;
> > ops->func = klp_ftrace_handler;
> > - ops->flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC;
> > + ops->flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC |
> > + FTRACE_FL_IPMODIFY;

Heh, there is a typo. Please replace FTRACE_FL_IPMODIFY
with FTRACE_OPS_FL_IPMODIFY ;-)

Best Regards,
Petr

> > func->fops = ops;
> > func->state = KLP_DISABLED;
> >
> > --
> > 2.1.0
> >
> --
> 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

2014-12-22 17:41:55

[permalink] [raw]

Subject: Re: [PATCH] livepatch: use FTRACE_OPS_FL_IPMODIFY

On Mon, Dec 22, 2014 at 06:34:09PM +0100, Petr Mladek wrote:
> On Fri 2014-12-19 10:43:35, Petr Mladek wrote:
> > On Thu 2014-12-18 09:49:35, Josh Poimboeuf wrote:
> > > On Thu, Dec 18, 2014 at 08:55:21PM +0900, Masami Hiramatsu wrote:
> > > > (2014/12/17 2:58), Seth Jennings wrote:
> > > > > changes in v7:
> > > > > - TODO: set IPMODIFY (not a blocker to moving forward)
> > > >
> > > > Why don't you set this?
> > > > IPMODIFY series are not completely applied yet, but you can already
> > > > use the flag. All you need is just set it :)
> > >
> > > Yeah, I don't see any reason why we can't start using this flag now.
> > > How about we add this patch to the queue?
> > >
> > > -->8--
> > >
> > > Subject: livepatch: use FTRACE_OPS_FL_IPMODIFY
> > >
> > > Use the FTRACE_OPS_FL_IPMODIFY flag to prevent conflicts with other
> > > ftrace users who also modify regs->ip.
> > >
> > > Signed-off-by: Josh Poimboeuf <[email protected]>
> >
> > It makes sense. The flag is available even in 3.19.
> >
> > Reviewed-by: Petr Mladek <[email protected]>
>
> Feel free to keep the Reviewed-by if you fix the typo, see below.
>
> > > ---
> > > kernel/livepatch/core.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > > index 0004a71..bdd99975 100644
> > > --- a/kernel/livepatch/core.c
> > > +++ b/kernel/livepatch/core.c
> > > @@ -640,7 +640,8 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func)
> > >
> > > ops->private = func;
> > > ops->func = klp_ftrace_handler;
> > > - ops->flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC;
> > > + ops->flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_DYNAMIC |
> > > + FTRACE_FL_IPMODIFY;
>
> Heh, there is a typo. Please replace FTRACE_FL_IPMODIFY
> with FTRACE_OPS_FL_IPMODIFY ;-)

Argh! At least the commit message is correct ;-)

I'll send out a v2.

> Best Regards,
> Petr
>
> > > func->fops = ops;
> > > func->state = KLP_DISABLED;
> > >
> > > --
> > > 2.1.0
> > >
> > --
> > 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

--
Josh

2014-12-22 19:45:05