All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -repost 01/21] ftrace: Add function to find fentry of function
@ 2014-06-25 11:06 Jiri Slaby
  2014-06-25 11:06 ` [PATCH -repost 02/21] ftrace: Make ftrace_is_dead available globally Jiri Slaby
                   ` (19 more replies)
  0 siblings, 20 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Frederic Weisbecker

This is needed for kGraft to find a fentry location to be "ftraced".
We use this to find a place where to jump to a new/old code location.

Note that we use a O(n) algorithm to assert correctness (and
simplicity). This algorithm can be further optimized to be O(log(n))
using binary search, but care has to be taken about the first member
of each entries page. I.e. we cannot use 1:1 what is in
ftrace_location_range etc.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
---
 include/linux/ftrace.h |  1 +
 kernel/trace/ftrace.c  | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 404a686a3644..c142816c2801 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -295,6 +295,7 @@ extern void
 unregister_ftrace_function_probe_func(char *glob, struct ftrace_probe_ops *ops);
 extern void unregister_ftrace_function_probe_all(char *glob);
 
+extern unsigned long ftrace_function_to_fentry(unsigned long addr);
 extern int ftrace_text_reserved(const void *start, const void *end);
 
 extern int ftrace_nr_registered_ops(void);
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 5b372e3ed675..f4da441c0125 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1422,6 +1422,36 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip, void *regs)
 		}				\
 	}
 
+/**
+ * ftrace_function_to_fentry -- lookup fentry location for a function
+ * @addr: function address to find a fentry in
+ *
+ * Perform a lookup in a list of fentry callsites to find one that fits a
+ * specified function @addr. It returns the corresponding fentry callsite or
+ * zero on failure.
+ */
+unsigned long ftrace_function_to_fentry(unsigned long addr)
+{
+	const struct dyn_ftrace *rec;
+	const struct ftrace_page *pg;
+	unsigned long ret = 0;
+
+	mutex_lock(&ftrace_lock);
+	do_for_each_ftrace_rec(pg, rec) {
+		unsigned long off;
+
+		if (!kallsyms_lookup_size_offset(rec->ip, NULL, &off))
+			continue;
+		if (addr + off == rec->ip) {
+			ret = rec->ip;
+			goto end;
+		}
+	} while_for_each_ftrace_rec()
+end:
+	mutex_unlock(&ftrace_lock);
+
+	return ret;
+}
 
 static int ftrace_cmp_recs(const void *a, const void *b)
 {
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 02/21] ftrace: Make ftrace_is_dead available globally
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
@ 2014-06-25 11:06 ` Jiri Slaby
  2014-06-25 11:06 ` [PATCH -repost 03/21] kgr: initial code Jiri Slaby
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Frederic Weisbecker

Kgr wants to test whether ftrace is OK with patching. If not, we just
bail out and will not initialize at all.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
---
 include/linux/ftrace.h | 3 +++
 kernel/trace/trace.h   | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index c142816c2801..7ba30d4b4909 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -140,6 +140,8 @@ enum ftrace_tracing_type_t {
 /* Current tracing type, default is FTRACE_TYPE_ENTER */
 extern enum ftrace_tracing_type_t ftrace_tracing_type;
 
+extern int ftrace_is_dead(void);
+
 /**
  * ftrace_stop - stop function tracer.
  *
@@ -241,6 +243,7 @@ static inline int ftrace_nr_registered_ops(void)
 	return 0;
 }
 static inline void clear_ftrace_function(void) { }
+static inline int ftrace_is_dead(void) { return 0; }
 static inline void ftrace_kill(void) { }
 static inline void ftrace_stop(void) { }
 static inline void ftrace_start(void) { }
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 9258f5a815db..0d96b8990a97 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -824,7 +824,6 @@ static inline int ftrace_trace_task(struct task_struct *task)
 
 	return test_tsk_trace_trace(task);
 }
-extern int ftrace_is_dead(void);
 int ftrace_create_function_files(struct trace_array *tr,
 				 struct dentry *parent);
 void ftrace_destroy_function_files(struct trace_array *tr);
@@ -837,7 +836,6 @@ static inline int ftrace_trace_task(struct task_struct *task)
 {
 	return 1;
 }
-static inline int ftrace_is_dead(void) { return 0; }
 static inline int
 ftrace_create_function_files(struct trace_array *tr,
 			     struct dentry *parent)
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 03/21] kgr: initial code
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
  2014-06-25 11:06 ` [PATCH -repost 02/21] ftrace: Make ftrace_is_dead available globally Jiri Slaby
@ 2014-06-25 11:06 ` Jiri Slaby
  2014-06-25 11:06 ` [PATCH -repost 04/21] kgr: add testing kgraft patch Jiri Slaby
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Frederic Weisbecker

From: Jiri Kosina <jkosina@suse.cz>

Provide initial implementation. We are now able to do ftrace-based
runtime patching of the kernel code.

In addition to that, we will provide a kgr_patcher module in the next
patch to test the functionality.

Note that the per-process flag dismisses in later patches where it is
converted to a single bit in the thread_info.

Limitations/TODOs:

- rmmod of the module that provides the patch is not possible yet
  (it'd be nice if that'd cause reverse application of the patch)
- x86_64 only

Additional squashes to this patch:
jk: add missing Kconfig.kgr
jk: fixup a header bug
jk: cleanup comments
js: port to new mcount infrastructure
js: order includes
js: fix for non-KGR (prototype and Kconfig fixes)
js: fix potential lock imbalance in kgr_patch_code
js: use insn helper for jmp generation
js: add \n to a printk
jk: externally_visible attribute warning fix
jk: symbol lookup failure handling
jk: fix race between patching and setting a flag (thanks to bpetkov)
js: add more sanity checking
js: handle missing kallsyms gracefully
js: use correct name, not alias
js: fix index in cleanup path
js: clear kgr_in_progress for all syscall paths
js: cleanup
js: do the checking in the process context
js: call kgr_mark_processes outside loop and locks
jk: convert from raw patching to ftrace API
jk: depend on regs-saving ftrace
js: make kgr_init an init_call
js: use correct offset for stub
js: use pr_debug
js: use IS_ENABLED
js: fix potential memory leak
js: change names from kgr -> kGraft
js: fix error handling and return values
js: use bitops to be atomic
jk: helpers for task's kgr_in_progress
js: remove copies of stubs, have only a single instance

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Andi Kleen <andi@firstfloor.org>
---
 arch/x86/Kconfig                   |   2 +
 arch/x86/include/asm/kgraft.h      |  27 +++
 arch/x86/include/asm/thread_info.h |   1 +
 arch/x86/kernel/asm-offsets.c      |   1 +
 arch/x86/kernel/entry_64.S         |   3 +
 include/linux/kgraft.h             |  85 +++++++++
 kernel/Kconfig.kgraft              |   7 +
 kernel/Makefile                    |   1 +
 kernel/kgraft.c                    | 346 +++++++++++++++++++++++++++++++++++++
 9 files changed, 473 insertions(+)
 create mode 100644 arch/x86/include/asm/kgraft.h
 create mode 100644 include/linux/kgraft.h
 create mode 100644 kernel/Kconfig.kgraft
 create mode 100644 kernel/kgraft.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a8f749ef0fdc..90c45b15b08b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -131,6 +131,7 @@ config X86
 	select HAVE_CC_STACKPROTECTOR
 	select GENERIC_CPU_AUTOPROBE
 	select HAVE_ARCH_AUDITSYSCALL
+	select HAVE_KGRAFT
 
 config INSTRUCTION_DECODER
 	def_bool y
@@ -267,6 +268,7 @@ config FIX_EARLYCON_MEM
 
 source "init/Kconfig"
 source "kernel/Kconfig.freezer"
+source "kernel/Kconfig.kgraft"
 
 menu "Processor type and features"
 
diff --git a/arch/x86/include/asm/kgraft.h b/arch/x86/include/asm/kgraft.h
new file mode 100644
index 000000000000..5e40ba1a0753
--- /dev/null
+++ b/arch/x86/include/asm/kgraft.h
@@ -0,0 +1,27 @@
+/*
+ * kGraft Online Kernel Patching
+ *
+ *  Copyright (c) 2013-2014 SUSE
+ *   Authors: Jiri Kosina
+ *	      Vojtech Pavlik
+ *	      Jiri Slaby
+ */
+
+/*
+ * 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.
+ */
+
+#ifndef ASM_KGR_H
+#define ASM_KGR_H
+
+#include <asm/ptrace.h>
+
+static inline void kgr_set_regs_ip(struct pt_regs *regs, unsigned long ip)
+{
+	regs->ip = ip;
+}
+
+#endif
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 854053889d4d..e44c8fda9c43 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -35,6 +35,7 @@ struct thread_info {
 	void __user		*sysenter_return;
 	unsigned int		sig_on_uaccess_error:1;
 	unsigned int		uaccess_err:1;	/* uaccess failed */
+	unsigned long		kgr_in_progress;
 };
 
 #define INIT_THREAD_INFO(tsk)			\
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index 9f6b9341950f..0db0437967a2 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -32,6 +32,7 @@ void common(void) {
 	OFFSET(TI_flags, thread_info, flags);
 	OFFSET(TI_status, thread_info, status);
 	OFFSET(TI_addr_limit, thread_info, addr_limit);
+	OFFSET(TI_kgr_in_progress, thread_info, kgr_in_progress);
 
 	BLANK();
 	OFFSET(crypto_tfm_ctx_offset, crypto_tfm, __crt_ctx);
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index b25ca969edd2..a7c570abc918 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -409,6 +409,7 @@ GLOBAL(system_call_after_swapgs)
 	movq  %rax,ORIG_RAX-ARGOFFSET(%rsp)
 	movq  %rcx,RIP-ARGOFFSET(%rsp)
 	CFI_REL_OFFSET rip,RIP-ARGOFFSET
+	movq $0, TI_kgr_in_progress+THREAD_INFO(%rsp,RIP-ARGOFFSET)
 	testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
 	jnz tracesys
 system_call_fastpath:
@@ -433,6 +434,7 @@ sysret_check:
 	LOCKDEP_SYS_EXIT
 	DISABLE_INTERRUPTS(CLBR_NONE)
 	TRACE_IRQS_OFF
+	movq $0, TI_kgr_in_progress+THREAD_INFO(%rsp,RIP-ARGOFFSET)
 	movl TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET),%edx
 	andl %edi,%edx
 	jnz  sysret_careful
@@ -555,6 +557,7 @@ GLOBAL(int_ret_from_sys_call)
 GLOBAL(int_with_check)
 	LOCKDEP_SYS_EXIT_IRQ
 	GET_THREAD_INFO(%rcx)
+	movq $0, TI_kgr_in_progress(%rcx)
 	movl TI_flags(%rcx),%edx
 	andl %edi,%edx
 	jnz   int_careful
diff --git a/include/linux/kgraft.h b/include/linux/kgraft.h
new file mode 100644
index 000000000000..e87623fe74ad
--- /dev/null
+++ b/include/linux/kgraft.h
@@ -0,0 +1,85 @@
+/*
+ * kGraft Online Kernel Patching
+ *
+ *  Copyright (c) 2013-2014 SUSE
+ *   Authors: Jiri Kosina
+ *	      Vojtech Pavlik
+ *	      Jiri Slaby
+ */
+
+/*
+ * 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.
+ */
+
+#ifndef LINUX_KGR_H
+#define LINUX_KGR_H
+
+#include <linux/bitops.h>
+#include <linux/ftrace.h>
+#include <linux/sched.h>
+
+#if IS_ENABLED(CONFIG_KGRAFT)
+
+#include <asm/kgraft.h>
+
+#define KGR_TIMEOUT 30
+
+struct kgr_patch {
+	char reserved;
+	const struct kgr_patch_fun {
+		const char *name;
+		const char *new_name;
+
+		void *new_function;
+
+		struct ftrace_ops *ftrace_ops_slow;
+		struct ftrace_ops *ftrace_ops_fast;
+	} *patches[];
+};
+
+/*
+ * data structure holding locations of the source and target function
+ * fentry sites to avoid repeated lookups
+ */
+struct kgr_loc_caches {
+	unsigned long old;
+	unsigned long new;
+};
+
+#define KGR_PATCHED_FUNCTION(_name, _new_function)				\
+	static struct ftrace_ops __kgr_patch_ftrace_ops_slow_ ## _name = {	\
+		.flags = FTRACE_OPS_FL_SAVE_REGS,				\
+	};									\
+	static struct ftrace_ops __kgr_patch_ftrace_ops_fast_ ## _name = {	\
+		.flags = FTRACE_OPS_FL_SAVE_REGS,				\
+	};									\
+	static const struct kgr_patch_fun __kgr_patch_ ## _name = {		\
+		.name = #_name,							\
+		.new_name = #_new_function,					\
+		.new_function = _new_function,					\
+		.ftrace_ops_slow = &__kgr_patch_ftrace_ops_slow_ ## _name,	\
+		.ftrace_ops_fast = &__kgr_patch_ftrace_ops_fast_ ## _name,	\
+	};
+
+#define KGR_PATCH(name)		&__kgr_patch_ ## name
+#define KGR_PATCH_END		NULL
+
+extern int kgr_start_patching(const struct kgr_patch *);
+
+static inline void kgr_mark_task_in_progress(struct task_struct *p)
+{
+	/* This is replaced by thread_flag later. */
+	set_bit(0, &task_thread_info(p)->kgr_in_progress);
+}
+
+static inline bool kgr_task_in_progress(struct task_struct *p)
+{
+	return test_bit(0, &task_thread_info(p)->kgr_in_progress);
+}
+
+#endif /* IS_ENABLED(CONFIG_KGRAFT) */
+
+#endif /* LINUX_KGR_H */
diff --git a/kernel/Kconfig.kgraft b/kernel/Kconfig.kgraft
new file mode 100644
index 000000000000..f38d82c06580
--- /dev/null
+++ b/kernel/Kconfig.kgraft
@@ -0,0 +1,7 @@
+config HAVE_KGRAFT
+	bool
+
+config KGRAFT
+	bool "kGraft infrastructure"
+	depends on DYNAMIC_FTRACE_WITH_REGS
+	depends on HAVE_KGRAFT
diff --git a/kernel/Makefile b/kernel/Makefile
index f2a8b6246ce9..3b81542a839d 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -28,6 +28,7 @@ obj-y += printk/
 obj-y += irq/
 obj-y += rcu/
 
+obj-$(CONFIG_KGRAFT) += kgraft.o
 obj-$(CONFIG_CHECKPOINT_RESTORE) += kcmp.o
 obj-$(CONFIG_FREEZER) += freezer.o
 obj-$(CONFIG_PROFILING) += profile.o
diff --git a/kernel/kgraft.c b/kernel/kgraft.c
new file mode 100644
index 000000000000..9b832419e0fd
--- /dev/null
+++ b/kernel/kgraft.c
@@ -0,0 +1,346 @@
+/*
+ * kGraft Online Kernel Patching
+ *
+ *  Copyright (c) 2013-2014 SUSE
+ *   Authors: Jiri Kosina
+ *	      Vojtech Pavlik
+ *	      Jiri Slaby
+ */
+
+/*
+ * 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.
+ */
+
+#include <linux/ftrace.h>
+#include <linux/kallsyms.h>
+#include <linux/kgraft.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/sort.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+
+static int kgr_patch_code(const struct kgr_patch_fun *patch_fun, bool final);
+static void kgr_work_fn(struct work_struct *work);
+
+static struct workqueue_struct *kgr_wq;
+static DECLARE_DELAYED_WORK(kgr_work, kgr_work_fn);
+static DEFINE_MUTEX(kgr_in_progress_lock);
+static bool kgr_in_progress;
+static bool kgr_initialized;
+static const struct kgr_patch *kgr_patch;
+
+/*
+ * The stub needs to modify the RIP value stored in struct pt_regs
+ * so that ftrace redirects the execution properly.
+ */
+static void kgr_stub_fast(unsigned long ip, unsigned long parent_ip,
+		struct ftrace_ops *ops, struct pt_regs *regs)
+{
+	struct kgr_loc_caches *c = ops->private;
+
+	pr_info("kgr: fast stub: calling new code at %lx\n", c->new);
+	kgr_set_regs_ip(regs, c->new);
+}
+
+static void kgr_stub_slow(unsigned long ip, unsigned long parent_ip,
+		struct ftrace_ops *ops, struct pt_regs *regs)
+{
+	struct kgr_loc_caches *c = ops->private;
+
+	if (kgr_task_in_progress(current) && current->mm) {
+		pr_info("kgr: slow stub: calling old code at %lx\n",
+				c->old);
+		kgr_set_regs_ip(regs, c->old + MCOUNT_INSN_SIZE);
+	} else {
+		pr_info("kgr: slow stub: calling new code at %lx\n",
+				c->new);
+		kgr_set_regs_ip(regs, c->new);
+	}
+}
+
+static bool kgr_still_patching(void)
+{
+	struct task_struct *p;
+	bool failed = false;
+
+	read_lock(&tasklist_lock);
+	for_each_process(p) {
+		/*
+		 * TODO
+		 *   kernel thread codepaths not supported and silently ignored
+		 */
+		if (kgr_task_in_progress(p) && p->mm) {
+			pr_info("pid %d (%s) still in kernel after timeout\n",
+					p->pid, p->comm);
+			failed = true;
+		}
+	}
+	read_unlock(&tasklist_lock);
+	return failed;
+}
+
+static void kgr_finalize(void)
+{
+	const struct kgr_patch_fun *const *patch_fun;
+
+	for (patch_fun = kgr_patch->patches; *patch_fun; patch_fun++) {
+		int ret = kgr_patch_code(*patch_fun, true);
+		/*
+		 * In case any of the symbol resolutions in the set
+		 * has failed, patch all the previously replaced fentry
+		 * callsites back to nops and fail with grace
+		 */
+		if (ret < 0)
+			pr_err("kgr: finalize for %s failed, trying to continue\n",
+					(*patch_fun)->name);
+	}
+}
+
+static void kgr_work_fn(struct work_struct *work)
+{
+	if (kgr_still_patching()) {
+		pr_info("kgr failed after timeout (%d), still in degraded mode\n",
+			KGR_TIMEOUT);
+		/* recheck again later */
+		queue_delayed_work(kgr_wq, &kgr_work, KGR_TIMEOUT * HZ);
+		return;
+	}
+
+	/*
+	 * victory, patching finished, put everything back in shape
+	 * with as less performance impact as possible again
+	 */
+	pr_info("kgr succeeded\n");
+	kgr_finalize();
+	mutex_lock(&kgr_in_progress_lock);
+	kgr_in_progress = false;
+	mutex_unlock(&kgr_in_progress_lock);
+}
+
+static void kgr_mark_processes(void)
+{
+	struct task_struct *p;
+
+	read_lock(&tasklist_lock);
+	for_each_process(p)
+		kgr_mark_task_in_progress(p);
+	read_unlock(&tasklist_lock);
+}
+
+static unsigned long kgr_get_fentry_loc(const char *f_name)
+{
+	unsigned long orig_addr, fentry_loc;
+	const char *check_name;
+	char check_buf[KSYM_SYMBOL_LEN];
+
+	orig_addr = kallsyms_lookup_name(f_name);
+	if (!orig_addr) {
+		pr_err("kgr: function %s not resolved\n", f_name);
+		return -ENOENT;
+	}
+
+	fentry_loc = ftrace_function_to_fentry(orig_addr);
+	if (!fentry_loc) {
+		pr_err("kgr: fentry_loc not properly resolved\n");
+		return -ENXIO;
+	}
+
+	check_name = kallsyms_lookup(fentry_loc, NULL, NULL, NULL, check_buf);
+	if (strcmp(check_name, f_name)) {
+		pr_err("kgr: we got out of bounds the intended function (%s -> %s)\n",
+				f_name, check_name);
+		return -EINVAL;
+	}
+
+	return fentry_loc;
+}
+
+static int kgr_init_ftrace_ops(const struct kgr_patch_fun *patch_fun)
+{
+	struct kgr_loc_caches *caches;
+	unsigned long fentry_loc;
+	int ret;
+
+	/*
+	 * Initialize the ftrace_ops->private with pointers to the fentry
+	 * sites of both old and new functions. This is used as a
+	 * redirection target in the per-arch stubs.
+	 *
+	 * Beware! -- freeing (once unloading will be implemented)
+	 * will require synchronize_sched() etc.
+	 */
+
+	caches = kmalloc(sizeof(*caches), GFP_KERNEL);
+	if (!caches) {
+		pr_debug("kgr: unable to allocate fentry caches\n");
+		return -ENOMEM;
+	}
+
+	fentry_loc = kgr_get_fentry_loc(patch_fun->new_name);
+	if (IS_ERR_VALUE(fentry_loc)) {
+		pr_debug("kgr: fentry location lookup failed\n");
+		ret = fentry_loc;
+		goto free_caches;
+	}
+	pr_debug("kgr: storing %lx to caches->new for %s\n",
+			fentry_loc, patch_fun->new_name);
+	caches->new = fentry_loc;
+
+	fentry_loc = kgr_get_fentry_loc(patch_fun->name);
+	if (IS_ERR_VALUE(fentry_loc)) {
+		pr_debug("kgr: fentry location lookup failed\n");
+		ret = fentry_loc;
+		goto free_caches;
+	}
+
+	pr_debug("kgr: storing %lx to caches->old for %s\n",
+			fentry_loc, patch_fun->name);
+	caches->old = fentry_loc;
+
+	patch_fun->ftrace_ops_fast->private = caches;
+	patch_fun->ftrace_ops_fast->func = kgr_stub_fast;
+	patch_fun->ftrace_ops_slow->private = caches;
+	patch_fun->ftrace_ops_slow->func = kgr_stub_slow;
+
+	return 0;
+free_caches:
+	kfree(caches);
+	return ret;
+}
+
+static int kgr_patch_code(const struct kgr_patch_fun *patch_fun, bool final)
+{
+	struct ftrace_ops *new_ops;
+	struct kgr_loc_caches *caches;
+	unsigned long fentry_loc;
+	int err;
+
+	/* Choose between slow and fast stub */
+	if (!final) {
+		err = kgr_init_ftrace_ops(patch_fun);
+		if (err)
+			return err;
+		pr_debug("kgr: patching %s to slow stub\n", patch_fun->name);
+		new_ops = patch_fun->ftrace_ops_slow;
+	} else {
+		pr_debug("kgr: patching %s to fast stub\n", patch_fun->name);
+		new_ops = patch_fun->ftrace_ops_fast;
+	}
+
+	/* Flip the switch */
+	caches = new_ops->private;
+	fentry_loc = caches->old;
+	err = ftrace_set_filter_ip(new_ops, fentry_loc, 0, 0);
+	if (err) {
+		pr_debug("kgr: setting filter for %lx (%s) failed\n",
+				caches->old, patch_fun->name);
+		return err;
+	}
+
+	err = register_ftrace_function(new_ops);
+	if (err) {
+		pr_debug("kgr: registering ftrace function for %lx (%s) failed\n",
+				caches->old, patch_fun->name);
+		return err;
+	}
+
+	/*
+	 * Get rid of the slow stub. Having two stubs in the interim is fine,
+	 * the last one always "wins", as it'll be dragged earlier from the
+	 * ftrace hashtable
+	 */
+	if (final) {
+		err = unregister_ftrace_function(patch_fun->ftrace_ops_slow);
+		if (err) {
+			pr_debug("kgr: unregistering ftrace function for %lx (%s) failed with %d\n",
+					fentry_loc, patch_fun->name, err);
+			/* don't fail: we are only slower */
+			return 0;
+		}
+	}
+	pr_debug("kgr: redirection for %lx (%s) done\n", fentry_loc,
+			patch_fun->name);
+
+	return 0;
+}
+
+/**
+ * kgr_start_patching -- the entry for a kgraft patch
+ * @patch: patch to be applied
+ *
+ * Start patching of code that is neither running in IRQ context nor
+ * kernel thread.
+ */
+int kgr_start_patching(const struct kgr_patch *patch)
+{
+	const struct kgr_patch_fun *const *patch_fun;
+
+	if (!kgr_initialized) {
+		pr_err("kgr: can't patch, not initialized\n");
+		return -EINVAL;
+	}
+
+	mutex_lock(&kgr_in_progress_lock);
+	if (kgr_in_progress) {
+		pr_err("kgr: can't patch, another patching not yet finalized\n");
+		mutex_unlock(&kgr_in_progress_lock);
+		return -EAGAIN;
+	}
+
+	for (patch_fun = patch->patches; *patch_fun; patch_fun++) {
+		int ret;
+
+		ret = kgr_patch_code(*patch_fun, false);
+		/*
+		 * In case any of the symbol resolutions in the set
+		 * has failed, patch all the previously replaced fentry
+		 * callsites back to nops and fail with grace
+		 */
+		if (ret < 0) {
+			for (patch_fun--; patch_fun >= patch->patches;
+					patch_fun--)
+				unregister_ftrace_function((*patch_fun)->ftrace_ops_slow);
+			mutex_unlock(&kgr_in_progress_lock);
+			return ret;
+		}
+	}
+	kgr_in_progress = true;
+	kgr_patch = patch;
+	mutex_unlock(&kgr_in_progress_lock);
+
+	kgr_mark_processes();
+
+	/*
+	 * give everyone time to exit kernel, and check after a while
+	 */
+	queue_delayed_work(kgr_wq, &kgr_work, KGR_TIMEOUT * HZ);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kgr_start_patching);
+
+static int __init kgr_init(void)
+{
+	if (ftrace_is_dead()) {
+		pr_warn("kgr: enabled, but no fentry locations found ... aborting\n");
+		return -ENODEV;
+	}
+
+	kgr_wq = create_singlethread_workqueue("kgraft");
+	if (!kgr_wq) {
+		pr_err("kgr: cannot allocate a work queue, aborting!\n");
+		return -ENOMEM;
+	}
+
+	kgr_initialized = true;
+	pr_info("kgr: successfully initialized\n");
+
+	return 0;
+}
+module_init(kgr_init);
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 04/21] kgr: add testing kgraft patch
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
  2014-06-25 11:06 ` [PATCH -repost 02/21] ftrace: Make ftrace_is_dead available globally Jiri Slaby
  2014-06-25 11:06 ` [PATCH -repost 03/21] kgr: initial code Jiri Slaby
@ 2014-06-25 11:06 ` Jiri Slaby
  2014-06-25 11:06 ` [PATCH -repost 05/21] kgr: update Kconfig documentation Jiri Slaby
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Frederic Weisbecker

This is intended to be a presentation of the kGraft engine, so it is
placed into samples/ directory.

It patches two chosen functions sys_iopl() and sys_capable() to print
a message in addition to the original functionality.

js: fix filename in Makefile (thanks mmarek)

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
---
 samples/Kconfig                 |  4 ++
 samples/Makefile                |  3 +-
 samples/kgraft/Makefile         |  1 +
 samples/kgraft/kgraft_patcher.c | 92 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 samples/kgraft/Makefile
 create mode 100644 samples/kgraft/kgraft_patcher.c

diff --git a/samples/Kconfig b/samples/Kconfig
index 6181c2cc9ca0..b33a397dfc58 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -55,6 +55,10 @@ config SAMPLE_KDB
 	  Build an example of how to dynamically add the hello
 	  command to the kdb shell.
 
+config SAMPLE_KGRAFT_PATCHER
+	tristate "Build kGraft patcher example -- loadable modules only"
+	depends on KGRAFT && m
+
 config SAMPLE_RPMSG_CLIENT
 	tristate "Build rpmsg client sample -- loadable modules only"
 	depends on RPMSG && m
diff --git a/samples/Makefile b/samples/Makefile
index 1a60c62e2045..a0d1626bd5bb 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -1,4 +1,5 @@
 # Makefile for Linux samples code
 
 obj-$(CONFIG_SAMPLES)	+= kobject/ kprobes/ trace_events/ \
-			   hw_breakpoint/ kfifo/ kdb/ hidraw/ rpmsg/ seccomp/
+			   hw_breakpoint/ kfifo/ kdb/ kgraft/ \
+			   hidraw/ rpmsg/ seccomp/
diff --git a/samples/kgraft/Makefile b/samples/kgraft/Makefile
new file mode 100644
index 000000000000..888a332c3148
--- /dev/null
+++ b/samples/kgraft/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SAMPLE_KGRAFT_PATCHER) += kgraft_patcher.o
diff --git a/samples/kgraft/kgraft_patcher.c b/samples/kgraft/kgraft_patcher.c
new file mode 100644
index 000000000000..abb0c05bf739
--- /dev/null
+++ b/samples/kgraft/kgraft_patcher.c
@@ -0,0 +1,92 @@
+/*
+ * kgraft_patcher -- just kick the kGraft infrastructure for test
+ *
+ * We patch two (arbitrarily chosen) functions at once...
+ *
+ *  Copyright (c) 2013-2014 SUSE
+ *   Authors: Jiri Kosina
+ *	      Vojtech Pavlik
+ *	      Jiri Slaby
+ */
+
+/*
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/kgraft.h>
+#include <linux/kallsyms.h>
+#include <linux/sched.h>
+#include <linux/types.h>
+#include <linux/capability.h>
+#include <linux/ptrace.h>
+
+#include <asm/processor.h>
+
+/*
+ * This all should be autogenerated from the patched sources
+ */
+
+asmlinkage long kgr_new_sys_iopl(unsigned int level)
+{
+	struct pt_regs *regs = current_pt_regs();
+	unsigned int old = (regs->flags >> 12) & 3;
+	struct thread_struct *t = &current->thread;
+
+	printk(KERN_DEBUG "kgr-patcher: this is a new sys_iopl()\n");
+
+	if (level > 3)
+		return -EINVAL;
+	/* Trying to gain more privileges? */
+	if (level > old) {
+		if (!capable(CAP_SYS_RAWIO))
+			return -EPERM;
+	}
+	regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) | (level << 12);
+	t->iopl = level << 12;
+	set_iopl_mask(t->iopl);
+
+	return 0;
+}
+KGR_PATCHED_FUNCTION(SyS_iopl, kgr_new_sys_iopl);
+
+static bool new_capable(int cap)
+{
+	printk(KERN_DEBUG "kgr-patcher: this is a new capable()\n");
+
+	return ns_capable(&init_user_ns, cap);
+}
+KGR_PATCHED_FUNCTION(capable, new_capable);
+
+static const struct kgr_patch patch = {
+	.patches = {
+		KGR_PATCH(SyS_iopl),
+		KGR_PATCH(capable),
+		KGR_PATCH_END
+	}
+};
+
+static int __init kgr_patcher_init(void)
+{
+	/* removing not supported */
+	__module_get(THIS_MODULE);
+	kgr_start_patching(&patch);
+	return 0;
+}
+
+static void __exit kgr_patcher_cleanup(void)
+{
+	/* extra care needs to be taken when freeing ftrace_ops->private */
+	pr_err("removing now buggy!\n");
+}
+
+module_init(kgr_patcher_init);
+module_exit(kgr_patcher_cleanup);
+
+MODULE_LICENSE("GPL");
+
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 05/21] kgr: update Kconfig documentation
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (2 preceding siblings ...)
  2014-06-25 11:06 ` [PATCH -repost 04/21] kgr: add testing kgraft patch Jiri Slaby
@ 2014-06-25 11:06 ` Jiri Slaby
  2014-06-25 12:42   ` One Thousand Gnomes
  2014-06-25 11:07 ` [PATCH -repost 06/21] kgr: add Documentation Jiri Slaby
                   ` (15 subsequent siblings)
  19 siblings, 1 reply; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Udo Seidel

This is based on Udo's text which was augmented in this patch.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Udo Seidel <udoseidel@gmx.de>
Cc: Vojtech Pavlik <vojtech@suse.cz>
---
 kernel/Kconfig.kgraft | 3 +++
 samples/Kconfig       | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/kernel/Kconfig.kgraft b/kernel/Kconfig.kgraft
index f38d82c06580..bead93646071 100644
--- a/kernel/Kconfig.kgraft
+++ b/kernel/Kconfig.kgraft
@@ -5,3 +5,6 @@ config KGRAFT
 	bool "kGraft infrastructure"
 	depends on DYNAMIC_FTRACE_WITH_REGS
 	depends on HAVE_KGRAFT
+	help
+	  Select this to enable kGraft online kernel patching. The
+	  runtime price is zero, so it is safe to say Y here.
diff --git a/samples/Kconfig b/samples/Kconfig
index b33a397dfc58..12848d1bd8c5 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -58,6 +58,10 @@ config SAMPLE_KDB
 config SAMPLE_KGRAFT_PATCHER
 	tristate "Build kGraft patcher example -- loadable modules only"
 	depends on KGRAFT && m
+	help
+	  Sample code to replace sys_iopl() and sys_capable() via
+	  kGraft. This is only for presentation purposes. It is safe to
+	  say Y here.
 
 config SAMPLE_RPMSG_CLIENT
 	tristate "Build rpmsg client sample -- loadable modules only"
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 06/21] kgr: add Documentation
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (3 preceding siblings ...)
  2014-06-25 11:06 ` [PATCH -repost 05/21] kgr: update Kconfig documentation Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 07/21] kgr: trigger the first check earlier Jiri Slaby
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Udo Seidel

This is a text provided by Udo and polished.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Udo Seidel <udoseidel@gmx.de>
---
 Documentation/kgraft.txt | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/kgraft.txt

diff --git a/Documentation/kgraft.txt b/Documentation/kgraft.txt
new file mode 100644
index 000000000000..476beaf35c62
--- /dev/null
+++ b/Documentation/kgraft.txt
@@ -0,0 +1,44 @@
+Live Kernel Patching with kGraft
+--------------------------------
+
+Written by Udo Seidel <udoseidel at gmx dot de>
+Based on the Blog entry by Vojtech Pavlik
+Updated by Jiri Slaby
+
+June 2014
+
+kGraft's developement was started by the SUSE Labs. kGraft builds on
+technologies and ideas that are already present in the kernel: ftrace [1] and
+its mcount-based reserved space in function headers [2], the INT3/IPI-NMI
+patching also used in jump labels [3], and RCU-like update of code that does
+not require stopping the kernel.
+
+A kGraft patch is a kernel module and fully relies on the in-kernel module
+loader to link the new code with the kernel. Thanks to all that, the design
+can be nicely minimalistic.
+
+While kGraft is, by choice, limited to replacing whole functions and constants
+they reference, this does not limit the set of code patches that can be
+applied significantly.
+
+Use
+---
+
+1) Build a kernel with CONFIG_KGRAFT enabled
+2) Create a module with a patch
+   * Look at samples/kgraft/kgraft_patcher.c for an example
+3) Insert the module from 2) into the booted kernel from 1)
+4) All processes need to enter the kernel to acknowledge the new state
+   * This can be done e.g. by sending a non-fatal signal to all processes
+   * Check /proc/*/kgr_in_progress to check who still needs to be poked
+5) You should see "kgr succeeded" in dmesg now
+
+Enjoy your patched system!
+
+
+References
+----------
+
+[1] Documentation/trace/ftrace.txt
+[2] Documentation/trace/ftrace-design.txt
+[3] Documentation/static-keys.txt
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 07/21] kgr: trigger the first check earlier
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (4 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 06/21] kgr: add Documentation Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 08/21] kgr: sched.h, introduce kgr_task_safe helper Jiri Slaby
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Frederic Weisbecker

In 5 seconds, not 30. This speeds up the whole process in most
scenarios.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
---
 kernel/kgraft.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/kgraft.c b/kernel/kgraft.c
index 9b832419e0fd..b0c712e8297c 100644
--- a/kernel/kgraft.c
+++ b/kernel/kgraft.c
@@ -319,7 +319,7 @@ int kgr_start_patching(const struct kgr_patch *patch)
 	/*
 	 * give everyone time to exit kernel, and check after a while
 	 */
-	queue_delayed_work(kgr_wq, &kgr_work, KGR_TIMEOUT * HZ);
+	queue_delayed_work(kgr_wq, &kgr_work, 5 * HZ);
 
 	return 0;
 }
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 08/21] kgr: sched.h, introduce kgr_task_safe helper
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (5 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 07/21] kgr: trigger the first check earlier Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 09/21] kgr: mark task_safe in some kthreads Jiri Slaby
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Frederic Weisbecker

To be used from some kthreads.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
---
 include/linux/sched.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 306f4f0c987a..6bc2d63a59c4 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2974,6 +2974,15 @@ static inline void mm_init_owner(struct mm_struct *mm, struct task_struct *p)
 }
 #endif /* CONFIG_MEMCG */
 
+#if IS_ENABLED(CONFIG_KGRAFT)
+static inline void kgr_task_safe(struct task_struct *p)
+{
+	clear_bit(0, &task_thread_info(p)->kgr_in_progress);
+}
+#else
+static inline void kgr_task_safe(struct task_struct *p) { }
+#endif /* IS_ENABLED(CONFIG_KGRAFT) */
+
 static inline unsigned long task_rlimit(const struct task_struct *tsk,
 		unsigned int limit)
 {
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 09/21] kgr: mark task_safe in some kthreads
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (6 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 08/21] kgr: sched.h, introduce kgr_task_safe helper Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 10/21] kgr: kthreads support Jiri Slaby
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Frederic Weisbecker, Theodore Ts'o, Dipankar Sarma

Before we enable a kthread support in kGraft, we must make sure all
kthreads mark themselves as kGraft-safe at some point explicitly.

We do this by injecting kgr_task_safe to the freezer test. There, we
assume that kthreads are in some predefined state and can expect
something bad to happen. Hence we switch the kGraft worlds there from
the old one to the new one. The optimal solution would be to convert
most of kthreads (that need not be kthreads actually) to workqeues as
suggested by Tejun. This is an upcoming work that will appear next.
But until we get there, we use freezer for kGraft that way as is
presented here.

Note that there are also some kthreads that do not utilize freezer, so
we use kgr_task_safe in them explicitly. This happens at locations
that appear to be safe for the kthreads to switch the worlds.

The end result after we migrate kthreads (that need not be kthreads)
to workqueues is: have only kthreads that contain kgr_task_safe
explicitly (or using some helper) and nothing else.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> [devtmpfs]
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> [rcu]
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Dipankar Sarma <dipankar@in.ibm.com>
Cc: Tejun Heo <tj@kernel.org>
---
 drivers/base/devtmpfs.c     |  1 +
 drivers/scsi/scsi_error.c   |  2 ++
 drivers/usb/core/hub.c      |  4 ++--
 fs/jbd2/journal.c           |  2 ++
 fs/notify/mark.c            |  5 ++++-
 include/linux/freezer.h     |  2 ++
 kernel/hung_task.c          |  5 ++++-
 kernel/kthread.c            |  3 +++
 kernel/rcu/tree.c           |  6 ++++--
 kernel/rcu/tree_plugin.h    | 10 ++++++++--
 kernel/smpboot.c            |  2 ++
 kernel/workqueue.c          |  3 +++
 mm/huge_memory.c            |  1 +
 net/bluetooth/rfcomm/core.c |  2 ++
 14 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index 25798db14553..c7d52d1b8c9c 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -387,6 +387,7 @@ static int devtmpfsd(void *p)
 	sys_chroot(".");
 	complete(&setup_done);
 	while (1) {
+		kgr_task_safe(current);
 		spin_lock(&req_lock);
 		while (requests) {
 			struct req *req = requests;
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index cbe38e5e7955..28bc61251e2a 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -2153,6 +2153,8 @@ int scsi_error_handler(void *data)
 	 * disables signal delivery for the created thread.
 	 */
 	while (!kthread_should_stop()) {
+		kgr_task_safe(current);
+
 		set_current_state(TASK_INTERRUPTIBLE);
 		if ((shost->host_failed == 0 && shost->host_eh_scheduled == 0) ||
 		    shost->host_failed != shost->host_busy) {
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 21b99b4b4082..85a53488ed3f 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5070,9 +5070,9 @@ static int hub_thread(void *__unused)
 
 	do {
 		hub_events();
-		wait_event_freezable(khubd_wait,
+		wait_event_freezable(khubd_wait, ({ kgr_task_safe(current);
 				!list_empty(&hub_event_list) ||
-				kthread_should_stop());
+				kthread_should_stop(); }));
 	} while (!kthread_should_stop() || !list_empty(&hub_event_list));
 
 	pr_debug("%s: khubd exiting\n", usbcore_name);
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 67b8e303946c..1b9c4c2e014a 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -43,6 +43,7 @@
 #include <linux/backing-dev.h>
 #include <linux/bitops.h>
 #include <linux/ratelimit.h>
+#include <linux/sched.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/jbd2.h>
@@ -260,6 +261,7 @@ loop:
 			write_lock(&journal->j_state_lock);
 		}
 		finish_wait(&journal->j_wait_commit, &wait);
+		kgr_task_safe(current);
 	}
 
 	jbd_debug(1, "kjournald2 wakes\n");
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index d90deaa08e78..d30a491cacf2 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -82,6 +82,7 @@
 #include <linux/kthread.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/srcu.h>
@@ -355,7 +356,9 @@ static int fsnotify_mark_destroy(void *ignored)
 			fsnotify_put_mark(mark);
 		}
 
-		wait_event_interruptible(destroy_waitq, !list_empty(&destroy_list));
+		wait_event_interruptible(destroy_waitq, ({
+					kgr_task_safe(current);
+					!list_empty(&destroy_list); }));
 	}
 
 	return 0;
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 7fd81b8c4897..e08c3bef251b 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -61,6 +61,8 @@ static inline bool try_to_freeze_unsafe(void)
 
 static inline bool try_to_freeze(void)
 {
+	kgr_task_safe(current);
+
 	if (!(current->flags & PF_NOFREEZE))
 		debug_check_no_locks_held();
 	return try_to_freeze_unsafe();
diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index 06db12434d72..3d59261a8e2c 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -14,6 +14,7 @@
 #include <linux/kthread.h>
 #include <linux/lockdep.h>
 #include <linux/export.h>
+#include <linux/sched.h>
 #include <linux/sysctl.h>
 #include <linux/utsname.h>
 #include <trace/events/sched.h>
@@ -229,8 +230,10 @@ static int watchdog(void *dummy)
 	for ( ; ; ) {
 		unsigned long timeout = sysctl_hung_task_timeout_secs;
 
-		while (schedule_timeout_interruptible(timeout_jiffies(timeout)))
+		while (schedule_timeout_interruptible(timeout_jiffies(timeout))) {
+			kgr_task_safe(current);
 			timeout = sysctl_hung_task_timeout_secs;
+		}
 
 		if (atomic_xchg(&reset_hung_task, 0))
 			continue;
diff --git a/kernel/kthread.c b/kernel/kthread.c
index c2390f41307b..099ff4a07753 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -111,6 +111,8 @@ bool kthread_freezable_should_stop(bool *was_frozen)
 {
 	bool frozen = false;
 
+	kgr_task_safe(current);
+
 	might_sleep();
 
 	if (unlikely(freezing(current)))
@@ -497,6 +499,7 @@ int kthreadd(void *unused)
 		if (list_empty(&kthread_create_list))
 			schedule();
 		__set_current_state(TASK_RUNNING);
+		kgr_task_safe(current);
 
 		spin_lock(&kthread_create_lock);
 		while (!list_empty(&kthread_create_list)) {
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index f1ba77363fbb..51acec198818 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1701,9 +1701,10 @@ static int __noreturn rcu_gp_kthread(void *arg)
 					       ACCESS_ONCE(rsp->gpnum),
 					       TPS("reqwait"));
 			rsp->gp_state = RCU_GP_WAIT_GPS;
-			wait_event_interruptible(rsp->gp_wq,
+			wait_event_interruptible(rsp->gp_wq, ({
+						 kgr_task_safe(current);
 						 ACCESS_ONCE(rsp->gp_flags) &
-						 RCU_GP_FLAG_INIT);
+						 RCU_GP_FLAG_INIT; }));
 			/* Locking provides needed memory barrier. */
 			if (rcu_gp_init(rsp))
 				break;
@@ -1735,6 +1736,7 @@ static int __noreturn rcu_gp_kthread(void *arg)
 					(!ACCESS_ONCE(rnp->qsmask) &&
 					 !rcu_preempt_blocked_readers_cgp(rnp)),
 					j);
+			kgr_task_safe(current);
 			/* Locking provides needed memory barriers. */
 			/* If grace period done, leave loop. */
 			if (!ACCESS_ONCE(rnp->qsmask) &&
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index cbc2c45265e2..ac012deb387b 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -27,6 +27,7 @@
 #include <linux/delay.h>
 #include <linux/gfp.h>
 #include <linux/oom.h>
+#include <linux/sched.h>
 #include <linux/smpboot.h>
 #include "../time/tick-internal.h"
 
@@ -1224,7 +1225,8 @@ static int rcu_boost_kthread(void *arg)
 	for (;;) {
 		rnp->boost_kthread_status = RCU_KTHREAD_WAITING;
 		trace_rcu_utilization(TPS("End boost kthread@rcu_wait"));
-		rcu_wait(rnp->boost_tasks || rnp->exp_tasks);
+		rcu_wait(({ kgr_task_safe(current);
+					rnp->boost_tasks || rnp->exp_tasks; }));
 		trace_rcu_utilization(TPS("Start boost kthread@rcu_wait"));
 		rnp->boost_kthread_status = RCU_KTHREAD_RUNNING;
 		more2boost = rcu_boost(rnp);
@@ -2227,11 +2229,15 @@ static int rcu_nocb_kthread(void *arg)
 
 	/* Each pass through this loop invokes one batch of callbacks */
 	for (;;) {
+		kgr_task_safe(current);
+
 		/* If not polling, wait for next batch of callbacks. */
 		if (!rcu_nocb_poll) {
 			trace_rcu_nocb_wake(rdp->rsp->name, rdp->cpu,
 					    TPS("Sleep"));
-			wait_event_interruptible(rdp->nocb_wq, rdp->nocb_head);
+			wait_event_interruptible(rdp->nocb_wq, ({
+						kgr_task_safe(current);
+						rdp->nocb_head; }));
 			/* Memory barrier provide by xchg() below. */
 		} else if (firsttime) {
 			firsttime = 0;
diff --git a/kernel/smpboot.c b/kernel/smpboot.c
index eb89e1807408..9764c83c0ef2 100644
--- a/kernel/smpboot.c
+++ b/kernel/smpboot.c
@@ -107,6 +107,8 @@ static int smpboot_thread_fn(void *data)
 	struct smp_hotplug_thread *ht = td->ht;
 
 	while (1) {
+		kgr_task_safe(current);
+
 		set_current_state(TASK_INTERRUPTIBLE);
 		preempt_disable();
 		if (kthread_should_stop()) {
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 6203d2900877..69222618d3e4 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2231,6 +2231,7 @@ sleep:
 	__set_current_state(TASK_INTERRUPTIBLE);
 	spin_unlock_irq(&pool->lock);
 	schedule();
+	kgr_task_safe(current);
 	goto woke_up;
 }
 
@@ -2272,6 +2273,8 @@ static int rescuer_thread(void *__rescuer)
 repeat:
 	set_current_state(TASK_INTERRUPTIBLE);
 
+	kgr_task_safe(current);
+
 	/*
 	 * By the time the rescuer is requested to stop, the workqueue
 	 * shouldn't have any work pending, but @wq->maydays may still have
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index e60837dc785c..fbcff9041fca 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2699,6 +2699,7 @@ static void khugepaged_do_scan(void)
 			break;
 
 		cond_resched();
+		kgr_task_safe(current);
 
 		if (unlikely(kthread_should_stop() || freezing(current)))
 			break;
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 754b6fe4f742..65c27adea565 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -2094,6 +2094,8 @@ static int rfcomm_run(void *unused)
 		if (kthread_should_stop())
 			break;
 
+		kgr_task_safe(current);
+
 		/* Process stuff */
 		rfcomm_process_sessions();
 
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 10/21] kgr: kthreads support
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (7 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 09/21] kgr: mark task_safe in some kthreads Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 11/21] kgr: handle irqs Jiri Slaby
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Frederic Weisbecker

Wake up kthreads so that they cycle through kgr_task_safe either
by an explicit call to it or implicitly via try_to_freeze. This
ensures nobody should use the old version of the code and kgraft core
can push everybody to use the new version by switching to the fast
path.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Tejun Heo <tj@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
---
 kernel/kgraft.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/kernel/kgraft.c b/kernel/kgraft.c
index b0c712e8297c..d21eaad8d48d 100644
--- a/kernel/kgraft.c
+++ b/kernel/kgraft.c
@@ -53,7 +53,7 @@ static void kgr_stub_slow(unsigned long ip, unsigned long parent_ip,
 {
 	struct kgr_loc_caches *c = ops->private;
 
-	if (kgr_task_in_progress(current) && current->mm) {
+	if (kgr_task_in_progress(current)) {
 		pr_info("kgr: slow stub: calling old code at %lx\n",
 				c->old);
 		kgr_set_regs_ip(regs, c->old + MCOUNT_INSN_SIZE);
@@ -71,11 +71,7 @@ static bool kgr_still_patching(void)
 
 	read_lock(&tasklist_lock);
 	for_each_process(p) {
-		/*
-		 * TODO
-		 *   kernel thread codepaths not supported and silently ignored
-		 */
-		if (kgr_task_in_progress(p) && p->mm) {
+		if (kgr_task_in_progress(p)) {
 			pr_info("pid %d (%s) still in kernel after timeout\n",
 					p->pid, p->comm);
 			failed = true;
@@ -123,13 +119,23 @@ static void kgr_work_fn(struct work_struct *work)
 	mutex_unlock(&kgr_in_progress_lock);
 }
 
-static void kgr_mark_processes(void)
+static void kgr_handle_processes(void)
 {
 	struct task_struct *p;
 
 	read_lock(&tasklist_lock);
-	for_each_process(p)
+	for_each_process(p) {
 		kgr_mark_task_in_progress(p);
+
+		/* wake up kthreads, they will clean the progress flag */
+		if (!p->mm) {
+			/*
+			 * this is incorrect for kthreads waiting still for
+			 * their first wake_up.
+			 */
+			wake_up_process(p);
+		}
+	}
 	read_unlock(&tasklist_lock);
 }
 
@@ -274,8 +280,7 @@ static int kgr_patch_code(const struct kgr_patch_fun *patch_fun, bool final)
  * kgr_start_patching -- the entry for a kgraft patch
  * @patch: patch to be applied
  *
- * Start patching of code that is neither running in IRQ context nor
- * kernel thread.
+ * Start patching of code that is not running in IRQ context.
  */
 int kgr_start_patching(const struct kgr_patch *patch)
 {
@@ -314,7 +319,7 @@ int kgr_start_patching(const struct kgr_patch *patch)
 	kgr_patch = patch;
 	mutex_unlock(&kgr_in_progress_lock);
 
-	kgr_mark_processes();
+	kgr_handle_processes();
 
 	/*
 	 * give everyone time to exit kernel, and check after a while
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 11/21] kgr: handle irqs
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (8 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 10/21] kgr: kthreads support Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 12/21] kgr: add MAINTAINERS entry Jiri Slaby
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Frederic Weisbecker, Thomas Gleixner

Introduce a per-cpu flag to check whether we should use the old or new
function in the slow stub. The new function starts being used on a
processor only after a scheduled function sets the flag via
schedule_on_each_cpu. Presumably this happens in the process context,
no irq is running. And protect the flag setting by disabling
interrupts so that we 1) have a barrier and 2) no interrupt triggers
while setting the flag (but the set should be atomic anyway as it is
bool).

js: fix fail paths

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/kgraft.h          |  6 +++--
 kernel/kgraft.c                 | 59 ++++++++++++++++++++++++++++++++---------
 samples/kgraft/kgraft_patcher.c |  2 +-
 3 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/include/linux/kgraft.h b/include/linux/kgraft.h
index e87623fe74ad..93bb1c50e079 100644
--- a/include/linux/kgraft.h
+++ b/include/linux/kgraft.h
@@ -18,6 +18,7 @@
 #define LINUX_KGR_H
 
 #include <linux/bitops.h>
+#include <linux/compiler.h>
 #include <linux/ftrace.h>
 #include <linux/sched.h>
 
@@ -28,7 +29,7 @@
 #define KGR_TIMEOUT 30
 
 struct kgr_patch {
-	char reserved;
+	bool __percpu *irq_use_new;
 	const struct kgr_patch_fun {
 		const char *name;
 		const char *new_name;
@@ -47,6 +48,7 @@ struct kgr_patch {
 struct kgr_loc_caches {
 	unsigned long old;
 	unsigned long new;
+	bool __percpu *irq_use_new;
 };
 
 #define KGR_PATCHED_FUNCTION(_name, _new_function)				\
@@ -67,7 +69,7 @@ struct kgr_loc_caches {
 #define KGR_PATCH(name)		&__kgr_patch_ ## name
 #define KGR_PATCH_END		NULL
 
-extern int kgr_start_patching(const struct kgr_patch *);
+extern int kgr_start_patching(struct kgr_patch *);
 
 static inline void kgr_mark_task_in_progress(struct task_struct *p)
 {
diff --git a/kernel/kgraft.c b/kernel/kgraft.c
index d21eaad8d48d..fd0ded7ce725 100644
--- a/kernel/kgraft.c
+++ b/kernel/kgraft.c
@@ -15,9 +15,11 @@
  */
 
 #include <linux/ftrace.h>
+#include <linux/hardirq.h> /* for in_interrupt() */
 #include <linux/kallsyms.h>
 #include <linux/kgraft.h>
 #include <linux/module.h>
+#include <linux/percpu.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/sort.h>
@@ -25,7 +27,8 @@
 #include <linux/types.h>
 #include <linux/workqueue.h>
 
-static int kgr_patch_code(const struct kgr_patch_fun *patch_fun, bool final);
+static int kgr_patch_code(const struct kgr_patch *patch,
+		const struct kgr_patch_fun *patch_fun, bool final);
 static void kgr_work_fn(struct work_struct *work);
 
 static struct workqueue_struct *kgr_wq;
@@ -52,8 +55,10 @@ static void kgr_stub_slow(unsigned long ip, unsigned long parent_ip,
 		struct ftrace_ops *ops, struct pt_regs *regs)
 {
 	struct kgr_loc_caches *c = ops->private;
+	bool irq = !!in_interrupt();
 
-	if (kgr_task_in_progress(current)) {
+	if ((!irq && kgr_task_in_progress(current)) ||
+			(irq && !*this_cpu_ptr(c->irq_use_new))) {
 		pr_info("kgr: slow stub: calling old code at %lx\n",
 				c->old);
 		kgr_set_regs_ip(regs, c->old + MCOUNT_INSN_SIZE);
@@ -86,7 +91,7 @@ static void kgr_finalize(void)
 	const struct kgr_patch_fun *const *patch_fun;
 
 	for (patch_fun = kgr_patch->patches; *patch_fun; patch_fun++) {
-		int ret = kgr_patch_code(*patch_fun, true);
+		int ret = kgr_patch_code(kgr_patch, *patch_fun, true);
 		/*
 		 * In case any of the symbol resolutions in the set
 		 * has failed, patch all the previously replaced fentry
@@ -96,6 +101,7 @@ static void kgr_finalize(void)
 			pr_err("kgr: finalize for %s failed, trying to continue\n",
 					(*patch_fun)->name);
 	}
+	free_percpu(kgr_patch->irq_use_new);
 }
 
 static void kgr_work_fn(struct work_struct *work)
@@ -167,6 +173,20 @@ static unsigned long kgr_get_fentry_loc(const char *f_name)
 	return fentry_loc;
 }
 
+static void kgr_handle_irq_cpu(struct work_struct *work)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	*this_cpu_ptr(kgr_patch->irq_use_new) = true;
+	local_irq_restore(flags);
+}
+
+static void kgr_handle_irqs(void)
+{
+	schedule_on_each_cpu(kgr_handle_irq_cpu);
+}
+
 static int kgr_init_ftrace_ops(const struct kgr_patch_fun *patch_fun)
 {
 	struct kgr_loc_caches *caches;
@@ -220,7 +240,8 @@ free_caches:
 	return ret;
 }
 
-static int kgr_patch_code(const struct kgr_patch_fun *patch_fun, bool final)
+static int kgr_patch_code(const struct kgr_patch *patch,
+		const struct kgr_patch_fun *patch_fun, bool final)
 {
 	struct ftrace_ops *new_ops;
 	struct kgr_loc_caches *caches;
@@ -241,6 +262,7 @@ static int kgr_patch_code(const struct kgr_patch_fun *patch_fun, bool final)
 
 	/* Flip the switch */
 	caches = new_ops->private;
+	caches->irq_use_new = patch->irq_use_new;
 	fentry_loc = caches->old;
 	err = ftrace_set_filter_ip(new_ops, fentry_loc, 0, 0);
 	if (err) {
@@ -280,28 +302,33 @@ static int kgr_patch_code(const struct kgr_patch_fun *patch_fun, bool final)
  * kgr_start_patching -- the entry for a kgraft patch
  * @patch: patch to be applied
  *
- * Start patching of code that is not running in IRQ context.
+ * Start patching of code.
  */
-int kgr_start_patching(const struct kgr_patch *patch)
+int kgr_start_patching(struct kgr_patch *patch)
 {
 	const struct kgr_patch_fun *const *patch_fun;
+	int ret;
 
 	if (!kgr_initialized) {
 		pr_err("kgr: can't patch, not initialized\n");
 		return -EINVAL;
 	}
 
+	patch->irq_use_new = alloc_percpu(bool);
+	if (!patch->irq_use_new) {
+		pr_err("kgr: can't patch, cannot allocate percpu data\n");
+		return -ENOMEM;
+	}
+
 	mutex_lock(&kgr_in_progress_lock);
 	if (kgr_in_progress) {
 		pr_err("kgr: can't patch, another patching not yet finalized\n");
-		mutex_unlock(&kgr_in_progress_lock);
-		return -EAGAIN;
+		ret = -EAGAIN;
+		goto unlock_free;
 	}
 
 	for (patch_fun = patch->patches; *patch_fun; patch_fun++) {
-		int ret;
-
-		ret = kgr_patch_code(*patch_fun, false);
+		ret = kgr_patch_code(patch, *patch_fun, false);
 		/*
 		 * In case any of the symbol resolutions in the set
 		 * has failed, patch all the previously replaced fentry
@@ -311,14 +338,14 @@ int kgr_start_patching(const struct kgr_patch *patch)
 			for (patch_fun--; patch_fun >= patch->patches;
 					patch_fun--)
 				unregister_ftrace_function((*patch_fun)->ftrace_ops_slow);
-			mutex_unlock(&kgr_in_progress_lock);
-			return ret;
+			goto unlock_free;
 		}
 	}
 	kgr_in_progress = true;
 	kgr_patch = patch;
 	mutex_unlock(&kgr_in_progress_lock);
 
+	kgr_handle_irqs();
 	kgr_handle_processes();
 
 	/*
@@ -327,6 +354,12 @@ int kgr_start_patching(const struct kgr_patch *patch)
 	queue_delayed_work(kgr_wq, &kgr_work, 5 * HZ);
 
 	return 0;
+unlock_free:
+	mutex_unlock(&kgr_in_progress_lock);
+
+	free_percpu(patch->irq_use_new);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(kgr_start_patching);
 
diff --git a/samples/kgraft/kgraft_patcher.c b/samples/kgraft/kgraft_patcher.c
index abb0c05bf739..7cb94f728128 100644
--- a/samples/kgraft/kgraft_patcher.c
+++ b/samples/kgraft/kgraft_patcher.c
@@ -63,7 +63,7 @@ static bool new_capable(int cap)
 }
 KGR_PATCHED_FUNCTION(capable, new_capable);
 
-static const struct kgr_patch patch = {
+static struct kgr_patch patch = {
 	.patches = {
 		KGR_PATCH(SyS_iopl),
 		KGR_PATCH(capable),
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 12/21] kgr: add MAINTAINERS entry
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (9 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 11/21] kgr: handle irqs Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 13/21] kgr: x86: refuse to build without fentry support Jiri Slaby
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Vojtech Pavlik <vojtech@suse.cz>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3f2e171047b9..73733eb50bb3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5239,6 +5239,15 @@ F:	include/linux/kdb.h
 F:	include/linux/kgdb.h
 F:	kernel/debug/
 
+KGRAFT
+M:	Jiri Kosina <jkosina@suse.cz>
+M:	Jiri Slaby <jslaby@suse.cz>
+M:	Vojtech Pavlik <vojtech@suse.cz>
+F:	arch/x86/include/asm/kgraft.h
+F:	include/linux/kgraft.h
+F:	kernel/kgraft.c
+F:	samples/kgraft/
+
 KMEMCHECK
 M:	Vegard Nossum <vegardno@ifi.uio.no>
 M:	Pekka Enberg <penberg@kernel.org>
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 13/21] kgr: x86: refuse to build without fentry support
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (10 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 12/21] kgr: add MAINTAINERS entry Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 14/21] kgr: add procfs interface for per-process 'kgr_in_progress' Jiri Slaby
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Frederic Weisbecker

From: Jiri Kosina <jkosina@suse.cz>

The only reliable way for function redirection through ftrace_ops (when
modifying pt_regs->rip in the handler) is fentry.

The alternative -- mcount -- is problematic in several ways. Namely the
caller's function prologue (that has already been executed by the time
mcount callsite has been reached) is not known to the callee, and can be
completely incompatible to the calee, resulting in a havoc on return from
the function.

fentry doesn't suffer from this, as it's located at the very beginning of
the function, even before prologue has been executed, and therefore callee
is the owner of both function prologue and epilogue.

Fixing mcount to properly fix everything up would be non-trivial, and
Steven is not in favor of doing that.

Both kGraft and upstream kernel (patch to be submitted) should error out
when this unsupported and non-working configuration is detected.

According to Michael Matz, the -mfentry gcc option is x86 specific. Other
architectures insert the respective profile calls before the prologue by
default.

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Michael Matz <matz@suse.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
---
 arch/x86/include/asm/kgraft.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/include/asm/kgraft.h b/arch/x86/include/asm/kgraft.h
index 5e40ba1a0753..6fc57a85d12c 100644
--- a/arch/x86/include/asm/kgraft.h
+++ b/arch/x86/include/asm/kgraft.h
@@ -17,6 +17,10 @@
 #ifndef ASM_KGR_H
 #define ASM_KGR_H
 
+#ifndef CC_USING_FENTRY
+#error Your compiler has to support -mfentry for kGraft to work on x86
+#endif
+
 #include <asm/ptrace.h>
 
 static inline void kgr_set_regs_ip(struct pt_regs *regs, unsigned long ip)
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 14/21] kgr: add procfs interface for per-process 'kgr_in_progress'
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (11 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 13/21] kgr: x86: refuse to build without fentry support Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 15/21] kgr: make a per-process 'in progress' flag a single bit Jiri Slaby
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Frederic Weisbecker

From: Jiri Kosina <jkosina@suse.cz>

Instead of flooding dmesg with data about tasks which haven't yet been
migrated to the "new universe", create a 'kgr_in_progress' in
/proc/<pid>/ so that it's possible to easily script the checks/actions
in userspace.

js: use the kgr helper

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Jiri Slaby <jslaby@suse.cz> [simplification]
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
---
 fs/proc/base.c  | 11 +++++++++++
 kernel/kgraft.c |  3 +--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 2d696b0c93bf..60f7b1ce5d1c 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -87,6 +87,7 @@
 #include <linux/slab.h>
 #include <linux/flex_array.h>
 #include <linux/posix-timers.h>
+#include <linux/kgraft.h>
 #ifdef CONFIG_HARDWALL
 #include <asm/hardwall.h>
 #endif
@@ -2106,6 +2107,13 @@ static const struct file_operations proc_timers_operations = {
 };
 #endif /* CONFIG_CHECKPOINT_RESTORE */
 
+#if IS_ENABLED(CONFIG_KGRAFT)
+static int proc_pid_kgr_in_progress(struct task_struct *task, char *buffer)
+{
+	return sprintf(buffer, "%d\n", kgr_task_in_progress(task));
+}
+#endif /* IS_ENABLED(CONFIG_KGRAFT) */
+
 static int proc_pident_instantiate(struct inode *dir,
 	struct dentry *dentry, struct task_struct *task, const void *ptr)
 {
@@ -2638,6 +2646,9 @@ static const struct pid_entry tgid_base_stuff[] = {
 #ifdef CONFIG_CHECKPOINT_RESTORE
 	REG("timers",	  S_IRUGO, proc_timers_operations),
 #endif
+#if IS_ENABLED(CONFIG_KGRAFT)
+	INF("kgr_in_progress",	S_IRUSR, proc_pid_kgr_in_progress),
+#endif
 };
 
 static int proc_tgid_base_readdir(struct file *file, struct dir_context *ctx)
diff --git a/kernel/kgraft.c b/kernel/kgraft.c
index fd0ded7ce725..d99171c6ea1d 100644
--- a/kernel/kgraft.c
+++ b/kernel/kgraft.c
@@ -77,9 +77,8 @@ static bool kgr_still_patching(void)
 	read_lock(&tasklist_lock);
 	for_each_process(p) {
 		if (kgr_task_in_progress(p)) {
-			pr_info("pid %d (%s) still in kernel after timeout\n",
-					p->pid, p->comm);
 			failed = true;
+			break;
 		}
 	}
 	read_unlock(&tasklist_lock);
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 15/21] kgr: make a per-process 'in progress' flag a single bit
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (12 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 14/21] kgr: add procfs interface for per-process 'kgr_in_progress' Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 16/21] kgr: add support for missing functions Jiri Slaby
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby,
	Frederic Weisbecker

From: Jiri Kosina <jkosina@suse.cz>

Having the per-task 'kgr_in_progress' flag stored as long is a waste
of space. And manipulating it is likely slower than just performing
single bit operations. Convert the flag to a thread info flag.

Additionally, making the KGR TI_flag part of _TIF_ALLWORK_MASK and
_TIF_WORK_SYSCALL_ENTRY allows for offloading the flag manipulation to
slow code paths.

js: use *_tsk_thread_flag helpers

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
---
 arch/x86/include/asm/thread_info.h |  7 ++++---
 arch/x86/kernel/asm-offsets.c      |  1 -
 arch/x86/kernel/entry_64.S         | 12 +++++++++---
 include/linux/kgraft.h             |  5 ++---
 include/linux/sched.h              |  2 +-
 5 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index e44c8fda9c43..53df17c72359 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -35,7 +35,6 @@ struct thread_info {
 	void __user		*sysenter_return;
 	unsigned int		sig_on_uaccess_error:1;
 	unsigned int		uaccess_err:1;	/* uaccess failed */
-	unsigned long		kgr_in_progress;
 };
 
 #define INIT_THREAD_INFO(tsk)			\
@@ -88,6 +87,7 @@ struct thread_info {
 #define TIF_IO_BITMAP		22	/* uses I/O bitmap */
 #define TIF_FORCED_TF		24	/* true if TF in eflags artificially */
 #define TIF_BLOCKSTEP		25	/* set when we want DEBUGCTLMSR_BTF */
+#define TIF_KGR_IN_PROGRESS	26	/* kGraft patching running */
 #define TIF_LAZY_MMU_UPDATES	27	/* task is updating the mmu lazily */
 #define TIF_SYSCALL_TRACEPOINT	28	/* syscall tracepoint instrumentation */
 #define TIF_ADDR32		29	/* 32-bit address space on 64 bits */
@@ -112,6 +112,7 @@ struct thread_info {
 #define _TIF_IO_BITMAP		(1 << TIF_IO_BITMAP)
 #define _TIF_FORCED_TF		(1 << TIF_FORCED_TF)
 #define _TIF_BLOCKSTEP		(1 << TIF_BLOCKSTEP)
+#define _TIF_KGR_IN_PROGRESS	(1 << TIF_KGR_IN_PROGRESS)
 #define _TIF_LAZY_MMU_UPDATES	(1 << TIF_LAZY_MMU_UPDATES)
 #define _TIF_SYSCALL_TRACEPOINT	(1 << TIF_SYSCALL_TRACEPOINT)
 #define _TIF_ADDR32		(1 << TIF_ADDR32)
@@ -121,7 +122,7 @@ struct thread_info {
 #define _TIF_WORK_SYSCALL_ENTRY	\
 	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_EMU | _TIF_SYSCALL_AUDIT |	\
 	 _TIF_SECCOMP | _TIF_SINGLESTEP | _TIF_SYSCALL_TRACEPOINT |	\
-	 _TIF_NOHZ)
+	 _TIF_NOHZ | _TIF_KGR_IN_PROGRESS)
 
 /* work to do in syscall_trace_leave() */
 #define _TIF_WORK_SYSCALL_EXIT	\
@@ -137,7 +138,7 @@ struct thread_info {
 /* work to do on any return to user space */
 #define _TIF_ALLWORK_MASK						\
 	((0x0000FFFF & ~_TIF_SECCOMP) | _TIF_SYSCALL_TRACEPOINT |	\
-	_TIF_NOHZ)
+	_TIF_NOHZ | _TIF_KGR_IN_PROGRESS)
 
 /* Only used for 64 bit */
 #define _TIF_DO_NOTIFY_MASK						\
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index 0db0437967a2..9f6b9341950f 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -32,7 +32,6 @@ void common(void) {
 	OFFSET(TI_flags, thread_info, flags);
 	OFFSET(TI_status, thread_info, status);
 	OFFSET(TI_addr_limit, thread_info, addr_limit);
-	OFFSET(TI_kgr_in_progress, thread_info, kgr_in_progress);
 
 	BLANK();
 	OFFSET(crypto_tfm_ctx_offset, crypto_tfm, __crt_ctx);
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index a7c570abc918..edaa5abd58f9 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -409,7 +409,6 @@ GLOBAL(system_call_after_swapgs)
 	movq  %rax,ORIG_RAX-ARGOFFSET(%rsp)
 	movq  %rcx,RIP-ARGOFFSET(%rsp)
 	CFI_REL_OFFSET rip,RIP-ARGOFFSET
-	movq $0, TI_kgr_in_progress+THREAD_INFO(%rsp,RIP-ARGOFFSET)
 	testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
 	jnz tracesys
 system_call_fastpath:
@@ -434,7 +433,6 @@ sysret_check:
 	LOCKDEP_SYS_EXIT
 	DISABLE_INTERRUPTS(CLBR_NONE)
 	TRACE_IRQS_OFF
-	movq $0, TI_kgr_in_progress+THREAD_INFO(%rsp,RIP-ARGOFFSET)
 	movl TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET),%edx
 	andl %edi,%edx
 	jnz  sysret_careful
@@ -454,6 +452,9 @@ sysret_check:
 	/* Handle reschedules */
 	/* edx:	work, edi: workmask */
 sysret_careful:
+#if IS_ENABLED(CONFIG_KGRAFT)
+	andl $~_TIF_KGR_IN_PROGRESS,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+#endif
 	bt $TIF_NEED_RESCHED,%edx
 	jnc sysret_signal
 	TRACE_IRQS_ON
@@ -517,6 +518,9 @@ sysret_audit:
 
 	/* Do syscall tracing */
 tracesys:
+#if IS_ENABLED(CONFIG_KGRAFT)
+	andl $~_TIF_KGR_IN_PROGRESS,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
+#endif
 #ifdef CONFIG_AUDITSYSCALL
 	testl $(_TIF_WORK_SYSCALL_ENTRY & ~_TIF_SYSCALL_AUDIT),TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
 	jz auditsys
@@ -557,7 +561,6 @@ GLOBAL(int_ret_from_sys_call)
 GLOBAL(int_with_check)
 	LOCKDEP_SYS_EXIT_IRQ
 	GET_THREAD_INFO(%rcx)
-	movq $0, TI_kgr_in_progress(%rcx)
 	movl TI_flags(%rcx),%edx
 	andl %edi,%edx
 	jnz   int_careful
@@ -568,6 +571,9 @@ GLOBAL(int_with_check)
 	/* First do a reschedule test. */
 	/* edx:	work, edi: workmask */
 int_careful:
+#if IS_ENABLED(CONFIG_KGRAFT)
+	andl $~_TIF_KGR_IN_PROGRESS,TI_flags(%rcx)
+#endif
 	bt $TIF_NEED_RESCHED,%edx
 	jnc  int_very_careful
 	TRACE_IRQS_ON
diff --git a/include/linux/kgraft.h b/include/linux/kgraft.h
index 93bb1c50e079..92b642408b6f 100644
--- a/include/linux/kgraft.h
+++ b/include/linux/kgraft.h
@@ -73,13 +73,12 @@ extern int kgr_start_patching(struct kgr_patch *);
 
 static inline void kgr_mark_task_in_progress(struct task_struct *p)
 {
-	/* This is replaced by thread_flag later. */
-	set_bit(0, &task_thread_info(p)->kgr_in_progress);
+	set_tsk_thread_flag(p, TIF_KGR_IN_PROGRESS);
 }
 
 static inline bool kgr_task_in_progress(struct task_struct *p)
 {
-	return test_bit(0, &task_thread_info(p)->kgr_in_progress);
+	return test_tsk_thread_flag(p, TIF_KGR_IN_PROGRESS);
 }
 
 #endif /* IS_ENABLED(CONFIG_KGRAFT) */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6bc2d63a59c4..0c037f6bba55 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2977,7 +2977,7 @@ static inline void mm_init_owner(struct mm_struct *mm, struct task_struct *p)
 #if IS_ENABLED(CONFIG_KGRAFT)
 static inline void kgr_task_safe(struct task_struct *p)
 {
-	clear_bit(0, &task_thread_info(p)->kgr_in_progress);
+	clear_tsk_thread_flag(p, TIF_KGR_IN_PROGRESS);
 }
 #else
 static inline void kgr_task_safe(struct task_struct *p) { }
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 16/21] kgr: add support for missing functions
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (13 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 15/21] kgr: make a per-process 'in progress' flag a single bit Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 17/21] kgr: exercise non-present function Jiri Slaby
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby

Sometimes we want to patch a function which is in a module that is not
currently loaded. In that case, patching would fail completely. So let
the user decide whether it is fatal when the function to be patched is
not found. If it is not, it is just skipped.  Other functions in the
patch (if any) are still patched in that case.

Note that this approach expects newly loaded modules to be fixed
already. No "deferred" patching happens on the module load.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 include/linux/kgraft.h          | 10 +++++++---
 kernel/kgraft.c                 | 26 ++++++++++++++++++--------
 samples/kgraft/kgraft_patcher.c |  4 ++--
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/include/linux/kgraft.h b/include/linux/kgraft.h
index 92b642408b6f..4d8665f60cbc 100644
--- a/include/linux/kgraft.h
+++ b/include/linux/kgraft.h
@@ -30,10 +30,13 @@
 
 struct kgr_patch {
 	bool __percpu *irq_use_new;
-	const struct kgr_patch_fun {
+	struct kgr_patch_fun {
 		const char *name;
 		const char *new_name;
 
+		bool abort_if_missing;
+		bool applied;
+
 		void *new_function;
 
 		struct ftrace_ops *ftrace_ops_slow;
@@ -51,16 +54,17 @@ struct kgr_loc_caches {
 	bool __percpu *irq_use_new;
 };
 
-#define KGR_PATCHED_FUNCTION(_name, _new_function)				\
+#define KGR_PATCHED_FUNCTION(_name, _new_function, abort)			\
 	static struct ftrace_ops __kgr_patch_ftrace_ops_slow_ ## _name = {	\
 		.flags = FTRACE_OPS_FL_SAVE_REGS,				\
 	};									\
 	static struct ftrace_ops __kgr_patch_ftrace_ops_fast_ ## _name = {	\
 		.flags = FTRACE_OPS_FL_SAVE_REGS,				\
 	};									\
-	static const struct kgr_patch_fun __kgr_patch_ ## _name = {		\
+	static struct kgr_patch_fun __kgr_patch_ ## _name = {			\
 		.name = #_name,							\
 		.new_name = #_new_function,					\
+		.abort_if_missing = abort,					\
 		.new_function = _new_function,					\
 		.ftrace_ops_slow = &__kgr_patch_ftrace_ops_slow_ ## _name,	\
 		.ftrace_ops_fast = &__kgr_patch_ftrace_ops_fast_ ## _name,	\
diff --git a/kernel/kgraft.c b/kernel/kgraft.c
index d99171c6ea1d..121faefcbb28 100644
--- a/kernel/kgraft.c
+++ b/kernel/kgraft.c
@@ -28,7 +28,7 @@
 #include <linux/workqueue.h>
 
 static int kgr_patch_code(const struct kgr_patch *patch,
-		const struct kgr_patch_fun *patch_fun, bool final);
+		struct kgr_patch_fun *patch_fun, bool final);
 static void kgr_work_fn(struct work_struct *work);
 
 static struct workqueue_struct *kgr_wq;
@@ -87,7 +87,7 @@ static bool kgr_still_patching(void)
 
 static void kgr_finalize(void)
 {
-	const struct kgr_patch_fun *const *patch_fun;
+	struct kgr_patch_fun *const *patch_fun;
 
 	for (patch_fun = kgr_patch->patches; *patch_fun; patch_fun++) {
 		int ret = kgr_patch_code(kgr_patch, *patch_fun, true);
@@ -240,7 +240,7 @@ free_caches:
 }
 
 static int kgr_patch_code(const struct kgr_patch *patch,
-		const struct kgr_patch_fun *patch_fun, bool final)
+		struct kgr_patch_fun *patch_fun, bool final)
 {
 	struct ftrace_ops *new_ops;
 	struct kgr_loc_caches *caches;
@@ -250,11 +250,16 @@ static int kgr_patch_code(const struct kgr_patch *patch,
 	/* Choose between slow and fast stub */
 	if (!final) {
 		err = kgr_init_ftrace_ops(patch_fun);
-		if (err)
+		if (err) {
+			if (err == -ENOENT && !patch_fun->abort_if_missing)
+				return 0;
 			return err;
+		}
 		pr_debug("kgr: patching %s to slow stub\n", patch_fun->name);
 		new_ops = patch_fun->ftrace_ops_slow;
 	} else {
+		if (!patch_fun->applied)
+			return 0;
 		pr_debug("kgr: patching %s to fast stub\n", patch_fun->name);
 		new_ops = patch_fun->ftrace_ops_fast;
 	}
@@ -290,7 +295,9 @@ static int kgr_patch_code(const struct kgr_patch *patch,
 			/* don't fail: we are only slower */
 			return 0;
 		}
-	}
+	} else
+		patch_fun->applied = true;
+
 	pr_debug("kgr: redirection for %lx (%s) done\n", fentry_loc,
 			patch_fun->name);
 
@@ -305,7 +312,7 @@ static int kgr_patch_code(const struct kgr_patch *patch,
  */
 int kgr_start_patching(struct kgr_patch *patch)
 {
-	const struct kgr_patch_fun *const *patch_fun;
+	struct kgr_patch_fun *const *patch_fun;
 	int ret;
 
 	if (!kgr_initialized) {
@@ -335,8 +342,11 @@ int kgr_start_patching(struct kgr_patch *patch)
 		 */
 		if (ret < 0) {
 			for (patch_fun--; patch_fun >= patch->patches;
-					patch_fun--)
-				unregister_ftrace_function((*patch_fun)->ftrace_ops_slow);
+					patch_fun--) {
+				if ((*patch_fun)->applied)
+					unregister_ftrace_function(
+						(*patch_fun)->ftrace_ops_slow);
+			}
 			goto unlock_free;
 		}
 	}
diff --git a/samples/kgraft/kgraft_patcher.c b/samples/kgraft/kgraft_patcher.c
index 7cb94f728128..5d02a908bc26 100644
--- a/samples/kgraft/kgraft_patcher.c
+++ b/samples/kgraft/kgraft_patcher.c
@@ -53,7 +53,7 @@ asmlinkage long kgr_new_sys_iopl(unsigned int level)
 
 	return 0;
 }
-KGR_PATCHED_FUNCTION(SyS_iopl, kgr_new_sys_iopl);
+KGR_PATCHED_FUNCTION(SyS_iopl, kgr_new_sys_iopl, true);
 
 static bool new_capable(int cap)
 {
@@ -61,7 +61,7 @@ static bool new_capable(int cap)
 
 	return ns_capable(&init_user_ns, cap);
 }
-KGR_PATCHED_FUNCTION(capable, new_capable);
+KGR_PATCHED_FUNCTION(capable, new_capable, true);
 
 static struct kgr_patch patch = {
 	.patches = {
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 17/21] kgr: exercise non-present function
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (14 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 16/21] kgr: add support for missing functions Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 18/21] kgr: fix race of stub and patching Jiri Slaby
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby

This is to test the newly added functionality: non-fatal patching of
yet unknown functions.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 samples/kgraft/kgraft_patcher.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/samples/kgraft/kgraft_patcher.c b/samples/kgraft/kgraft_patcher.c
index 5d02a908bc26..e96eef840397 100644
--- a/samples/kgraft/kgraft_patcher.c
+++ b/samples/kgraft/kgraft_patcher.c
@@ -63,10 +63,17 @@ static bool new_capable(int cap)
 }
 KGR_PATCHED_FUNCTION(capable, new_capable, true);
 
+static void new_function(unsigned long data)
+{
+	pr_info("kgr-patcher: %s\n", __func__);
+}
+KGR_PATCHED_FUNCTION(unknown_function, new_function, false);
+
 static struct kgr_patch patch = {
 	.patches = {
 		KGR_PATCH(SyS_iopl),
 		KGR_PATCH(capable),
+		KGR_PATCH(unknown_function),
 		KGR_PATCH_END
 	}
 };
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 18/21] kgr: fix race of stub and patching
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (15 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 17/21] kgr: exercise non-present function Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 19/21] kgr: expose global 'in_progress' state through procfs Jiri Slaby
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby

While we are patching, we set up a stub which refers to
kgr_in_progress of a process. The stub can be called immediately when
set up, but we set the flag even after done with patching in
kgr_handle_processes. This is obviously too late, so set the flag
before we start patching, but after we check that no other patching is
in progress -- we would interfere otherwise.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Reported-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
---
 kernel/kgraft.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/kernel/kgraft.c b/kernel/kgraft.c
index 121faefcbb28..5ec0c1abe0d6 100644
--- a/kernel/kgraft.c
+++ b/kernel/kgraft.c
@@ -124,14 +124,22 @@ static void kgr_work_fn(struct work_struct *work)
 	mutex_unlock(&kgr_in_progress_lock);
 }
 
-static void kgr_handle_processes(void)
+static void kgr_mark_processes(void)
 {
 	struct task_struct *p;
 
 	read_lock(&tasklist_lock);
-	for_each_process(p) {
+	for_each_process(p)
 		kgr_mark_task_in_progress(p);
+	read_unlock(&tasklist_lock);
+}
 
+static void kgr_handle_processes(void)
+{
+	struct task_struct *p;
+
+	read_lock(&tasklist_lock);
+	for_each_process(p) {
 		/* wake up kthreads, they will clean the progress flag */
 		if (!p->mm) {
 			/*
@@ -333,6 +341,8 @@ int kgr_start_patching(struct kgr_patch *patch)
 		goto unlock_free;
 	}
 
+	kgr_mark_processes();
+
 	for (patch_fun = patch->patches; *patch_fun; patch_fun++) {
 		ret = kgr_patch_code(patch, *patch_fun, false);
 		/*
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 19/21] kgr: expose global 'in_progress' state through procfs
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (16 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 18/21] kgr: fix race of stub and patching Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 20/21] kgr: rephrase the "kGraft failed" message Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 21/21] kgr: x86: optimize handling of CPU-bound tasks Jiri Slaby
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby

From: Jiri Kosina <jkosina@suse.cz>

In addition to having a per-process flag that shows which processess have
already been "migrated", it's useful to have a global-wide flag that will
show whether the patching operation is currently undergoing without having
to traverse all /proc entries.

js: handle error

Reported-by: Libor Pechacek <lpechacek@suse.cz>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 kernel/kgraft.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/kernel/kgraft.c b/kernel/kgraft.c
index 5ec0c1abe0d6..1247f1c60b09 100644
--- a/kernel/kgraft.c
+++ b/kernel/kgraft.c
@@ -26,6 +26,8 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
+#include <linux/seq_file.h>
+#include <linux/proc_fs.h>
 
 static int kgr_patch_code(const struct kgr_patch *patch,
 		struct kgr_patch_fun *patch_fun, bool final);
@@ -382,6 +384,25 @@ unlock_free:
 }
 EXPORT_SYMBOL_GPL(kgr_start_patching);
 
+static int kgr_show(struct seq_file *m, void *v)
+{
+	seq_printf(m, "%d\n", kgr_in_progress);
+	return 0;
+}
+
+static int kgr_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, kgr_show, NULL);
+}
+
+static const struct file_operations kgr_fops = {
+	.owner      = THIS_MODULE,
+	.open       = kgr_open,
+	.read       = seq_read,
+	.llseek     = seq_lseek,
+	.release    = single_release,
+};
+
 static int __init kgr_init(void)
 {
 	if (ftrace_is_dead()) {
@@ -398,6 +419,9 @@ static int __init kgr_init(void)
 	kgr_initialized = true;
 	pr_info("kgr: successfully initialized\n");
 
+	if (!proc_create("kgr_in_progress", 0, NULL, &kgr_fops))
+		pr_warn("kgr: cannot create kgr_in_progress in procfs\n");
+
 	return 0;
 }
 module_init(kgr_init);
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 20/21] kgr: rephrase the "kGraft failed" message
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (17 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 19/21] kgr: expose global 'in_progress' state through procfs Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  2014-06-25 11:07 ` [PATCH -repost 21/21] kgr: x86: optimize handling of CPU-bound tasks Jiri Slaby
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Libor Pechacek,
	Jiri Slaby

From: Libor Pechacek <lpechacek@suse.cz>

kGraft not succeeding on the first attempt can hardly be called a
failure.  kGraft is merely waiting for sleeping processes to wake up
and get out of the way.

Signed-off-by: Libor Pechacek <lpechacek@suse.cz>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 kernel/kgraft.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/kgraft.c b/kernel/kgraft.c
index 1247f1c60b09..90ef7fba6d0a 100644
--- a/kernel/kgraft.c
+++ b/kernel/kgraft.c
@@ -108,7 +108,7 @@ static void kgr_finalize(void)
 static void kgr_work_fn(struct work_struct *work)
 {
 	if (kgr_still_patching()) {
-		pr_info("kgr failed after timeout (%d), still in degraded mode\n",
+		pr_info("kgr still in progress after timeout (%d)\n",
 			KGR_TIMEOUT);
 		/* recheck again later */
 		queue_delayed_work(kgr_wq, &kgr_work, KGR_TIMEOUT * HZ);
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH -repost 21/21] kgr: x86: optimize handling of CPU-bound tasks
  2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
                   ` (18 preceding siblings ...)
  2014-06-25 11:07 ` [PATCH -repost 20/21] kgr: rephrase the "kGraft failed" message Jiri Slaby
@ 2014-06-25 11:07 ` Jiri Slaby
  19 siblings, 0 replies; 29+ messages in thread
From: Jiri Slaby @ 2014-06-25 11:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: tj, rostedt, mingo, akpm, andi, paulmck, pavel, jirislaby,
	Vojtech Pavlik, Michael Matz, Jiri Kosina, Jiri Slaby

From: Jiri Kosina <jkosina@suse.cz>

Processes which are running in userspace at the time of patching can
be immediately marked as "migrated" to the new universe, as they are
provably outside the kernel and would have their 'in_progress' flag
cleared upon (eventual) kernel entry anyway.

This eliminates the need to send a SIGSTOP/SIGCONT signal (or perform
any kind of alternative handling that would force the tasks to go
through the kernel) to such tasks. This allows the tasks to run
completely undisturbed by the patching.

We do this by looking at the task's stack trace. This is suboptimal
and perhaps ugly solution but we have not find any other easy way
without interrupting the task's computation. I.e. we are aware of IPIs
and looking at stored regs for example. If anyone can come up with an
idea how to dig out the process' state (whether running in user space
or not) from task_struct or such, please draw faster and shoot this
one dead.

js: remove unneeded headers
js: cleanup

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 arch/x86/include/asm/kgraft.h | 30 ++++++++++++++++++++++++++++++
 kernel/kgraft.c               |  3 +++
 2 files changed, 33 insertions(+)

diff --git a/arch/x86/include/asm/kgraft.h b/arch/x86/include/asm/kgraft.h
index 6fc57a85d12c..3b13738f3665 100644
--- a/arch/x86/include/asm/kgraft.h
+++ b/arch/x86/include/asm/kgraft.h
@@ -22,10 +22,40 @@
 #endif
 
 #include <asm/ptrace.h>
+#include <linux/stacktrace.h>
 
 static inline void kgr_set_regs_ip(struct pt_regs *regs, unsigned long ip)
 {
 	regs->ip = ip;
 }
 
+#ifdef CONFIG_STACKTRACE
+/*
+ * Tasks which are running in userspace after the patching has been started
+ * can immediately be marked as migrated to the new universe.
+ *
+ * If this function returns non-zero (i.e. also when error happens), the task
+ * needs to be migrated using kgraft lazy mechanism.
+ */
+static inline bool kgr_needs_lazy_migration(struct task_struct *p)
+{
+	unsigned long s[3];
+	struct stack_trace t = {
+		.nr_entries = 0,
+		.skip = 0,
+		.max_entries = 3,
+		.entries = s,
+	};
+
+	save_stack_trace_tsk(p, &t);
+
+	return t.nr_entries > 2;
+}
+#else
+static inline bool kgr_needs_lazy_migration(struct task_struct *p)
+{
+	return true;
+}
+#endif
+
 #endif
diff --git a/kernel/kgraft.c b/kernel/kgraft.c
index 90ef7fba6d0a..151e00648ffc 100644
--- a/kernel/kgraft.c
+++ b/kernel/kgraft.c
@@ -150,6 +150,9 @@ static void kgr_handle_processes(void)
 			 */
 			wake_up_process(p);
 		}
+		/* mark tasks wandering in userspace as already migrated */
+		if (!kgr_needs_lazy_migration(p))
+			kgr_task_safe(p);
 	}
 	read_unlock(&tasklist_lock);
 }
-- 
2.0.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH -repost 05/21] kgr: update Kconfig documentation
  2014-06-25 11:06 ` [PATCH -repost 05/21] kgr: update Kconfig documentation Jiri Slaby
@ 2014-06-25 12:42   ` One Thousand Gnomes
  2014-06-26  8:25     ` Jiri Slaby
  0 siblings, 1 reply; 29+ messages in thread
From: One Thousand Gnomes @ 2014-06-25 12:42 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: linux-kernel, tj, rostedt, mingo, akpm, andi, paulmck, pavel,
	jirislaby, Vojtech Pavlik, Michael Matz, Jiri Kosina, Udo Seidel

On Wed, 25 Jun 2014 13:06:59 +0200
Jiri Slaby <jslaby@suse.cz> wrote:

> This is based on Udo's text which was augmented in this patch.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Udo Seidel <udoseidel@gmx.de>
> Cc: Vojtech Pavlik <vojtech@suse.cz>
> ---
>  kernel/Kconfig.kgraft | 3 +++
>  samples/Kconfig       | 4 ++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/kernel/Kconfig.kgraft b/kernel/Kconfig.kgraft
> index f38d82c06580..bead93646071 100644
> --- a/kernel/Kconfig.kgraft
> +++ b/kernel/Kconfig.kgraft
> @@ -5,3 +5,6 @@ config KGRAFT
>  	bool "kGraft infrastructure"
>  	depends on DYNAMIC_FTRACE_WITH_REGS
>  	depends on HAVE_KGRAFT
> +	help
> +	  Select this to enable kGraft online kernel patching. The
> +	  runtime price is zero, so it is safe to say Y here.
> diff --git a/samples/Kconfig b/samples/Kconfi

The runtime impact is that you've just introduced a virus and trojan
writers delight into your kernel. There's a balance between convenience
and security but given most users will never use kgraft this advice seems
incorrect.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH -repost 05/21] kgr: update Kconfig documentation
  2014-06-25 12:42   ` One Thousand Gnomes
@ 2014-06-26  8:25     ` Jiri Slaby
  2014-06-26  8:34       ` Jiri Kosina
  0 siblings, 1 reply; 29+ messages in thread
From: Jiri Slaby @ 2014-06-26  8:25 UTC (permalink / raw)
  To: One Thousand Gnomes
  Cc: linux-kernel, tj, rostedt, mingo, akpm, andi, paulmck, pavel,
	jirislaby, Vojtech Pavlik, Michael Matz, Jiri Kosina, Udo Seidel

On 06/25/2014 02:42 PM, One Thousand Gnomes wrote:
> On Wed, 25 Jun 2014 13:06:59 +0200
> Jiri Slaby <jslaby@suse.cz> wrote:
> 
>> This is based on Udo's text which was augmented in this patch.
>>
>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>> Cc: Udo Seidel <udoseidel@gmx.de>
>> Cc: Vojtech Pavlik <vojtech@suse.cz>
>> ---
>>  kernel/Kconfig.kgraft | 3 +++
>>  samples/Kconfig       | 4 ++++
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/kernel/Kconfig.kgraft b/kernel/Kconfig.kgraft
>> index f38d82c06580..bead93646071 100644
>> --- a/kernel/Kconfig.kgraft
>> +++ b/kernel/Kconfig.kgraft
>> @@ -5,3 +5,6 @@ config KGRAFT
>>  	bool "kGraft infrastructure"
>>  	depends on DYNAMIC_FTRACE_WITH_REGS
>>  	depends on HAVE_KGRAFT
>> +	help
>> +	  Select this to enable kGraft online kernel patching. The
>> +	  runtime price is zero, so it is safe to say Y here.
>> diff --git a/samples/Kconfig b/samples/Kconfi
> 
> The runtime impact is that you've just introduced a virus and trojan
> writers delight into your kernel. There's a balance between convenience
> and security but given most users will never use kgraft this advice seems
> incorrect.

This now writes:
+       help
+         Select this to enable kGraft online kernel patching. The
+         runtime price is nearly zero, so it is safe to say Y here
+         provided you are aware of all the consequences (e.g. in
+         security).

Is it OK with you?

thanks,
-- 
js
suse labs

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH -repost 05/21] kgr: update Kconfig documentation
  2014-06-26  8:25     ` Jiri Slaby
@ 2014-06-26  8:34       ` Jiri Kosina
  2014-06-27 19:18         ` Pavel Machek
  0 siblings, 1 reply; 29+ messages in thread
From: Jiri Kosina @ 2014-06-26  8:34 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: One Thousand Gnomes, linux-kernel, tj, rostedt, mingo, akpm,
	andi, paulmck, pavel, jirislaby, Vojtech Pavlik, Michael Matz,
	Udo Seidel

On Thu, 26 Jun 2014, Jiri Slaby wrote:

> >> ---
> >>  kernel/Kconfig.kgraft | 3 +++
> >>  samples/Kconfig       | 4 ++++
> >>  2 files changed, 7 insertions(+)
> >>
> >> diff --git a/kernel/Kconfig.kgraft b/kernel/Kconfig.kgraft
> >> index f38d82c06580..bead93646071 100644
> >> --- a/kernel/Kconfig.kgraft
> >> +++ b/kernel/Kconfig.kgraft
> >> @@ -5,3 +5,6 @@ config KGRAFT
> >>  	bool "kGraft infrastructure"
> >>  	depends on DYNAMIC_FTRACE_WITH_REGS
> >>  	depends on HAVE_KGRAFT
> >> +	help
> >> +	  Select this to enable kGraft online kernel patching. The
> >> +	  runtime price is zero, so it is safe to say Y here.
> >> diff --git a/samples/Kconfig b/samples/Kconfi
> > 
> > The runtime impact is that you've just introduced a virus and trojan
> > writers delight into your kernel. There's a balance between convenience
> > and security but given most users will never use kgraft this advice seems
> > incorrect.
> 
> This now writes:
> +       help
> +         Select this to enable kGraft online kernel patching. The
> +         runtime price is nearly zero, so it is safe to say Y here
> +         provided you are aware of all the consequences (e.g. in
> +         security).
> 
> Is it OK with you?

This might cause a false impression that we are actually opening a 
security hole into a system, which is not true at all.

Yes, backdoor writeres might (or might not) make use of kGraft API, but 
they have gazillion of other comparable options (*probes, ftrace, 
text_poke(), ...).

I'd perhaps propose something like

"Select this to enable kGraft live kernel patching. The runtime penalty is 
nearly zero, so it is safe to say Y here if you want the kernel to expose 
API for live patching to modules".

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH -repost 05/21] kgr: update Kconfig documentation
  2014-06-26  8:34       ` Jiri Kosina
@ 2014-06-27 19:18         ` Pavel Machek
  2014-07-04  9:14           ` Jiri Slaby
  0 siblings, 1 reply; 29+ messages in thread
From: Pavel Machek @ 2014-06-27 19:18 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Jiri Slaby, One Thousand Gnomes, linux-kernel, tj, rostedt,
	mingo, akpm, andi, paulmck, jirislaby, Vojtech Pavlik,
	Michael Matz, Udo Seidel

Hi!

> > This now writes:
> > +       help
> > +         Select this to enable kGraft online kernel patching. The
> > +         runtime price is nearly zero, so it is safe to say Y here
> > +         provided you are aware of all the consequences (e.g. in
> > +         security).
> > 
> > Is it OK with you?
> 
> This might cause a false impression that we are actually opening a 
> security hole into a system, which is not true at all.
> 
> Yes, backdoor writeres might (or might not) make use of kGraft API, but 
> they have gazillion of other comparable options (*probes, ftrace, 
> text_poke(), ...).
> 
> I'd perhaps propose something like
> 
> "Select this to enable kGraft live kernel patching. The runtime penalty is 
> nearly zero, so it is safe to say Y here if you want the kernel to expose 
> API for live patching to modules".

Well. People that are not distro vendors will not prepare patches for
themselves, right? And patches prepared for suse will not work on
self-configured kernels.

So probably everyone should say "N" here...


                                           				Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH -repost 05/21] kgr: update Kconfig documentation
  2014-06-27 19:18         ` Pavel Machek
@ 2014-07-04  9:14           ` Jiri Slaby
  2014-07-04 10:35             ` Pavel Machek
  0 siblings, 1 reply; 29+ messages in thread
From: Jiri Slaby @ 2014-07-04  9:14 UTC (permalink / raw)
  To: Pavel Machek, Jiri Kosina
  Cc: One Thousand Gnomes, linux-kernel, tj, rostedt, mingo, akpm,
	andi, paulmck, jirislaby, Vojtech Pavlik, Michael Matz,
	Udo Seidel

On 06/27/2014 09:18 PM, Pavel Machek wrote:
>>> This now writes:
>>> +       help
>>> +         Select this to enable kGraft online kernel patching. The
>>> +         runtime price is nearly zero, so it is safe to say Y here
>>> +         provided you are aware of all the consequences (e.g. in
>>> +         security).
>>>
>>> Is it OK with you?
>>
>> This might cause a false impression that we are actually opening a 
>> security hole into a system, which is not true at all.
>>
>> Yes, backdoor writeres might (or might not) make use of kGraft API, but 
>> they have gazillion of other comparable options (*probes, ftrace, 
>> text_poke(), ...).
>>
>> I'd perhaps propose something like
>>
>> "Select this to enable kGraft live kernel patching. The runtime penalty is 
>> nearly zero, so it is safe to say Y here if you want the kernel to expose 
>> API for live patching to modules".
> 
> Well. People that are not distro vendors will not prepare patches for
> themselves, right?

Hi, why do you believe so? But it is not so important, see below.

> And patches prepared for suse will not work on
> self-configured kernels.
> 
> So probably everyone should say "N" here...

The text is formulated correctly and satisfies your concerns, I think.
Say Y, if you want the API...

thanks,
-- 
js
suse labs

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH -repost 05/21] kgr: update Kconfig documentation
  2014-07-04  9:14           ` Jiri Slaby
@ 2014-07-04 10:35             ` Pavel Machek
  2014-07-05 19:47               ` Jiri Kosina
  0 siblings, 1 reply; 29+ messages in thread
From: Pavel Machek @ 2014-07-04 10:35 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Jiri Kosina, One Thousand Gnomes, linux-kernel, tj, rostedt,
	mingo, akpm, andi, paulmck, jirislaby, Vojtech Pavlik,
	Michael Matz, Udo Seidel

On Fri 2014-07-04 11:14:54, Jiri Slaby wrote:
> On 06/27/2014 09:18 PM, Pavel Machek wrote:
> >>> This now writes:
> >>> +       help
> >>> +         Select this to enable kGraft online kernel patching. The
> >>> +         runtime price is nearly zero, so it is safe to say Y here
> >>> +         provided you are aware of all the consequences (e.g. in
> >>> +         security).
> >>>
> >>> Is it OK with you?
> >>
> >> This might cause a false impression that we are actually opening a 
> >> security hole into a system, which is not true at all.
> >>
> >> Yes, backdoor writeres might (or might not) make use of kGraft API, but 
> >> they have gazillion of other comparable options (*probes, ftrace, 
> >> text_poke(), ...).
> >>
> >> I'd perhaps propose something like
> >>
> >> "Select this to enable kGraft live kernel patching. The runtime penalty is 
> >> nearly zero, so it is safe to say Y here if you want the kernel to expose 
> >> API for live patching to modules".

It is "safe" but completely stupid to say Y here, unless you are a
distro vendor.

> > Well. People that are not distro vendors will not prepare patches for
> > themselves, right?
> 
> Hi, why do you believe so? But it is not so important, see below.

Because it is quite hard to prepare the patch, and there's not really
enough documentation..? And given choice between "spend half an hour
preparing patch" and "just reboot", people compiling their own kernels
know what to do...

> > And patches prepared for suse will not work on
> > self-configured kernels.
> > 
> > So probably everyone should say "N" here...
> 
> The text is formulated correctly and satisfies your concerns, I think.
> Say Y, if you want the API...

Lawyer may read it correctly. For the rest of human beings, it should
say "say N here". Three people in the world that can prepare the patch
know they need the option  enabled, even without help text.

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH -repost 05/21] kgr: update Kconfig documentation
  2014-07-04 10:35             ` Pavel Machek
@ 2014-07-05 19:47               ` Jiri Kosina
  2014-07-06 12:35                 ` Pavel Machek
  0 siblings, 1 reply; 29+ messages in thread
From: Jiri Kosina @ 2014-07-05 19:47 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Jiri Slaby, One Thousand Gnomes, linux-kernel, tj, rostedt,
	mingo, akpm, andi, paulmck, jirislaby, Vojtech Pavlik,
	Michael Matz, Udo Seidel

On Fri, 4 Jul 2014, Pavel Machek wrote:

> > Hi, why do you believe so? But it is not so important, see below.
> 
> Because it is quite hard to prepare the patch, and there's not really
> enough documentation..? And given choice between "spend half an hour
> preparing patch" and "just reboot", people compiling their own kernels
> know what to do...

I fail to see how "having the skills to compile my own kernel" and "being 
responsible for a datacenter that can't afford immediate reboot of all 
nodes" are connected in any way.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH -repost 05/21] kgr: update Kconfig documentation
  2014-07-05 19:47               ` Jiri Kosina
@ 2014-07-06 12:35                 ` Pavel Machek
  0 siblings, 0 replies; 29+ messages in thread
From: Pavel Machek @ 2014-07-06 12:35 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Jiri Slaby, One Thousand Gnomes, linux-kernel, tj, rostedt,
	mingo, akpm, andi, paulmck, jirislaby, Vojtech Pavlik,
	Michael Matz, Udo Seidel

On Sat 2014-07-05 21:47:45, Jiri Kosina wrote:
> On Fri, 4 Jul 2014, Pavel Machek wrote:
> 
> > > Hi, why do you believe so? But it is not so important, see below.
> > 
> > Because it is quite hard to prepare the patch, and there's not really
> > enough documentation..? And given choice between "spend half an hour
> > preparing patch" and "just reboot", people compiling their own kernels
> > know what to do...
> 
> I fail to see how "having the skills to compile my own kernel" and "being 
> responsible for a datacenter that can't afford immediate reboot of all 
> nodes" are connected in any way.

Parse error, or too much irony.

Anyway, I'd suggest the Kconfig test be something like "say N unless
you know how to prepare patch for running kernel, and plan to do
that".

And, since you seem to be targeting sysadmins with this, it would be
cool to have sysadmin-targeted documentation explaining what patches
are safe to apply with kgraft and what are not.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2014-07-06 12:35 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-25 11:06 [PATCH -repost 01/21] ftrace: Add function to find fentry of function Jiri Slaby
2014-06-25 11:06 ` [PATCH -repost 02/21] ftrace: Make ftrace_is_dead available globally Jiri Slaby
2014-06-25 11:06 ` [PATCH -repost 03/21] kgr: initial code Jiri Slaby
2014-06-25 11:06 ` [PATCH -repost 04/21] kgr: add testing kgraft patch Jiri Slaby
2014-06-25 11:06 ` [PATCH -repost 05/21] kgr: update Kconfig documentation Jiri Slaby
2014-06-25 12:42   ` One Thousand Gnomes
2014-06-26  8:25     ` Jiri Slaby
2014-06-26  8:34       ` Jiri Kosina
2014-06-27 19:18         ` Pavel Machek
2014-07-04  9:14           ` Jiri Slaby
2014-07-04 10:35             ` Pavel Machek
2014-07-05 19:47               ` Jiri Kosina
2014-07-06 12:35                 ` Pavel Machek
2014-06-25 11:07 ` [PATCH -repost 06/21] kgr: add Documentation Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 07/21] kgr: trigger the first check earlier Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 08/21] kgr: sched.h, introduce kgr_task_safe helper Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 09/21] kgr: mark task_safe in some kthreads Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 10/21] kgr: kthreads support Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 11/21] kgr: handle irqs Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 12/21] kgr: add MAINTAINERS entry Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 13/21] kgr: x86: refuse to build without fentry support Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 14/21] kgr: add procfs interface for per-process 'kgr_in_progress' Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 15/21] kgr: make a per-process 'in progress' flag a single bit Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 16/21] kgr: add support for missing functions Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 17/21] kgr: exercise non-present function Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 18/21] kgr: fix race of stub and patching Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 19/21] kgr: expose global 'in_progress' state through procfs Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 20/21] kgr: rephrase the "kGraft failed" message Jiri Slaby
2014-06-25 11:07 ` [PATCH -repost 21/21] kgr: x86: optimize handling of CPU-bound tasks Jiri Slaby

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.