[PATCH HACK 1/6] livepatch-test: Add more cases

[PATCH HACK 1/6] livepatch-test: Add more cases

[Michael Ellerman]

Not for merging.

---
 arch/powerpc/kernel/setup_64.c       | 26 +++++++++++
 samples/livepatch/livepatch-sample.c | 90 ++++++++++++++++++++++++++++++++++--
 2 files changed, 113 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index f98be8383a39..3807fb05b6de 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -823,3 +823,29 @@ static int __init disable_hardlockup_detector(void)
 }
 early_initcall(disable_hardlockup_detector);
 #endif
+
+int func_with_lots_of_args(int a, int b, int c, int d, int e, int f, int g,
+			   int h, int i, int j, int k, int l)
+{
+	printk("%s: %d %d %d %d %d %d %d %d %d %d %d %d\n",
+	       __func__, a, b, c, d, e, f, g, h, i, j, k, l);
+
+	return a + b + c + d + e + f + g + h + i + j + k + l;
+}
+EXPORT_SYMBOL(func_with_lots_of_args);
+
+int func_with_nested_func(int a, int b, int c)
+{
+	volatile int z;
+
+	int noinline nested_sum (int x, int y) {
+		return z + x + y;
+	}
+
+	z = a + b;
+
+	printk("%s: %d %d\n", __func__, nested_sum(a, b), nested_sum(a, c));
+
+	return nested_sum(nested_sum(a, b), nested_sum(b, c));
+}
+EXPORT_SYMBOL(func_with_nested_func);
diff --git a/samples/livepatch/livepatch-sample.c b/samples/livepatch/livepatch-sample.c
index fb8c8614e728..b34ffa4f82ae 100644
--- a/samples/livepatch/livepatch-sample.c
+++ b/samples/livepatch/livepatch-sample.c
@@ -40,24 +40,108 @@
  */
 
 #include <linux/seq_file.h>
+
+static struct seq_file *cmdline_seq_file = NULL;
+
+int func_with_lots_of_args(int a, int b, int c, int d, int e, int f, int g,
+			   int h, int i, int j, int k, int l);
+
+int func_with_nested_func(int a, int b, int c);
+
 static int livepatch_cmdline_proc_show(struct seq_file *m, void *v)
 {
-	seq_printf(m, "%s\n", "this has been live patched");
+	int i, j;
+
+	cmdline_seq_file = m;
+
+	i = func_with_lots_of_args(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12);
+	j = func_with_nested_func(8, 9, 10);
+
+	seq_printf(m, "%s %p i = %d j = %d\n", "this has been live patched", m, i, j);
+
 	return 0;
 }
 
+static void livepatch_seq_printf(struct seq_file *m, const char *f, ...)
+{
+	va_list args;
+
+	va_start(args, f);
+	seq_vprintf(m, f, args);
+	va_end(args);
+
+	if (m == cmdline_seq_file) {
+		printk("livepatch: patched seq_printf() called\n");
+		dump_stack();
+		m = NULL;
+	}
+}
+
+static int livepatch_func_with_lots_of_args(int a, int b, int c, int d, int e,
+					    int f, int g, int h, int i, int j,
+					    int k, int l)
+{
+	printk("%s: %d %d %d %d %d %d %d %d %d %d %d %d\n",
+	       __func__, a, b, c, d, e, f, g, h, i, j, k, l);
+
+	return 1 + a + b + c + d + e + f + g + h + i + j + k + l;
+}
+
+struct scsi_lun {
+	__u8 scsi_lun[8];
+};
+
+static void livepatch_int_to_scsilun(u64 lun, struct scsi_lun *scsilun)
+{
+	int i;
+
+	memset(scsilun->scsi_lun, 0, sizeof(scsilun->scsi_lun));
+
+	for (i = 0; i < sizeof(lun); i += 2) {
+		scsilun->scsi_lun[i] = (lun >> 8) & 0xFF;
+		scsilun->scsi_lun[i+1] = lun & 0xFF;
+		lun = lun >> 16;
+	}
+
+	printk("livepatch: patched int_to_scsilun()\n");
+}
+
 static struct klp_func funcs[] = {
 	{
 		.old_name = "cmdline_proc_show",
 		.new_func = livepatch_cmdline_proc_show,
-	}, { }
+	},
+	{
+		.old_name = "seq_printf",
+		.new_func = livepatch_seq_printf,
+	},
+	{
+		.old_name = "func_with_lots_of_args",
+		.new_func = livepatch_func_with_lots_of_args,
+	},
+	{ }
+};
+
+static struct klp_func scsi_funcs[] = {
+	{
+		.old_name = "int_to_scsilun",
+		.new_func = livepatch_int_to_scsilun,
+	},
+	{ }
 };
 
 static struct klp_object objs[] = {
 	{
 		/* name being NULL means vmlinux */
 		.funcs = funcs,
-	}, { }
+	},
+	{
+#if IS_MODULE(CONFIG_SCSI)
+		.name = "scsi_mod",
+#endif
+		.funcs = scsi_funcs,
+	},
+	{ }
 };
 
 static struct klp_patch patch = {
-- 
2.5.0

[PATCH 2/6] ftrace: Make ftrace_location_range() global

[Michael Ellerman]

In order to support live patching on powerpc we would like to call
ftrace_location_range(), so make it global.

Signed-off-by: Torsten Duwe <duwe@suse.de>
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 include/linux/ftrace.h |  1 +
 kernel/trace/ftrace.c  | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 6d9df3f7e334..93230e97e897 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -455,6 +455,7 @@ int ftrace_update_record(struct dyn_ftrace *rec, int enable);
 int ftrace_test_record(struct dyn_ftrace *rec, int enable);
 void ftrace_run_stop_machine(int command);
 unsigned long ftrace_location(unsigned long ip);
+unsigned long ftrace_location_range(unsigned long start, unsigned long end);
 unsigned long ftrace_get_addr_new(struct dyn_ftrace *rec);
 unsigned long ftrace_get_addr_curr(struct dyn_ftrace *rec);
 
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 57a6eea84694..f4e6aae6ebe7 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1533,7 +1533,19 @@ static int ftrace_cmp_recs(const void *a, const void *b)
 	return 0;
 }
 
-static unsigned long ftrace_location_range(unsigned long start, unsigned long end)
+/**
+ * ftrace_location_range - return the first address of a traced location
+ *	if it touches the given ip range
+ * @start: start of range to search.
+ * @end: end of range to search (inclusive). @end points to the last byte
+ *	to check.
+ *
+ * Returns rec->ip if the related ftrace location is a least partly within
+ * the given address range. That is, the first address of the instruction
+ * that is either a NOP or call to the function tracer. It checks the ftrace
+ * internal tables to determine if the address belongs or not.
+ */
+unsigned long ftrace_location_range(unsigned long start, unsigned long end)
 {
 	struct ftrace_page *pg;
 	struct dyn_ftrace *rec;
-- 
2.5.0

Re: [PATCH 2/6] ftrace: Make ftrace_location_range() global

[Steven Rostedt]

On Thu, 24 Mar 2016 22:04:01 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

> In order to support live patching on powerpc we would like to call
> ftrace_location_range(), so make it global.

Do you want me to try to get this into this merge window?

-- Steve

Re: [PATCH 2/6] ftrace: Make ftrace_location_range() global

[Jiri Kosina]

On Thu, 24 Mar 2016, Steven Rostedt wrote:

> > In order to support live patching on powerpc we would like to call
> > ftrace_location_range(), so make it global.
> 
> Do you want me to try to get this into this merge window?

I don't think that's necessary. The dependency (rest of livepatching bits 
for ppc64le) will not go in before 4.7 merge window anyway.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 2/6] ftrace: Make ftrace_location_range() global

[Michael Ellerman]

On Thu, 2016-03-24 at 11:20 -0400, Steven Rostedt wrote:

> On Thu, 24 Mar 2016 22:04:01 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> > In order to support live patching on powerpc we would like to call
> > ftrace_location_range(), so make it global.
>
> Do you want me to try to get this into this merge window?

Nah, we can just take it next window with the whole series. If you're happy to
ack it.

cheers

[PATCH 3/6] livepatch: Allow architectures to specify an alternate ftrace location

[Michael Ellerman]

When livepatch tries to patch a function it takes the function address
and asks ftrace to install the livepatch handler at that location.
ftrace will look for an mcount call site at that exact address.

On powerpc the mcount location is not the first instruction of the
function, and in fact it's not at a constant offset from the start of
the function. To accommodate this add a hook which arch code can
override to customise the behaviour.

Signed-off-by: Torsten Duwe <duwe@suse.de>
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 kernel/livepatch/core.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index d68fbf63b083..b0476bb30f92 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -298,6 +298,19 @@ unlock:
 	rcu_read_unlock();
 }
 
+/*
+ * Convert a function address into the appropriate ftrace location.
+ *
+ * Usually this is just the address of the function, but on some architectures
+ * it's more complicated so allow them to provide a custom behaviour.
+ */
+#ifndef klp_get_ftrace_location
+static unsigned long klp_get_ftrace_location(unsigned long faddr)
+{
+	return faddr;
+}
+#endif
+
 static void klp_disable_func(struct klp_func *func)
 {
 	struct klp_ops *ops;
@@ -312,8 +325,14 @@ static void klp_disable_func(struct klp_func *func)
 		return;
 
 	if (list_is_singular(&ops->func_stack)) {
+		unsigned long ftrace_loc;
+
+		ftrace_loc = klp_get_ftrace_location(func->old_addr);
+		if (WARN_ON(!ftrace_loc))
+			return;
+
 		WARN_ON(unregister_ftrace_function(&ops->fops));
-		WARN_ON(ftrace_set_filter_ip(&ops->fops, func->old_addr, 1, 0));
+		WARN_ON(ftrace_set_filter_ip(&ops->fops, ftrace_loc, 1, 0));
 
 		list_del_rcu(&func->stack_node);
 		list_del(&ops->node);
@@ -338,6 +357,15 @@ static int klp_enable_func(struct klp_func *func)
 
 	ops = klp_find_ops(func->old_addr);
 	if (!ops) {
+		unsigned long ftrace_loc;
+
+		ftrace_loc = klp_get_ftrace_location(func->old_addr);
+		if (!ftrace_loc) {
+			pr_err("failed to find location for function '%s'\n",
+				func->old_name);
+			return -EINVAL;
+		}
+
 		ops = kzalloc(sizeof(*ops), GFP_KERNEL);
 		if (!ops)
 			return -ENOMEM;
@@ -352,7 +380,7 @@ static int klp_enable_func(struct klp_func *func)
 		INIT_LIST_HEAD(&ops->func_stack);
 		list_add_rcu(&func->stack_node, &ops->func_stack);
 
-		ret = ftrace_set_filter_ip(&ops->fops, func->old_addr, 0, 0);
+		ret = ftrace_set_filter_ip(&ops->fops, ftrace_loc, 0, 0);
 		if (ret) {
 			pr_err("failed to set ftrace filter for function '%s' (%d)\n",
 			       func->old_name, ret);
@@ -363,7 +391,7 @@ static int klp_enable_func(struct klp_func *func)
 		if (ret) {
 			pr_err("failed to register ftrace handler for function '%s' (%d)\n",
 			       func->old_name, ret);
-			ftrace_set_filter_ip(&ops->fops, func->old_addr, 1, 0);
+			ftrace_set_filter_ip(&ops->fops, ftrace_loc, 1, 0);
 			goto err;
 		}
 
-- 
2.5.0

[PATCH 4/6] powerpc/livepatch: Add livepatch header

[Michael Ellerman]

Add the powerpc specific livepatch definitions. In particular we provide
a non-default implementation of klp_get_ftrace_location().

This is required because the location of the mcount call is not constant
when using -mprofile-kernel (which we always do for live patching).

Signed-off-by: Torsten Duwe <duwe@suse.de>
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/livepatch.h | 54 ++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 arch/powerpc/include/asm/livepatch.h

diff --git a/arch/powerpc/include/asm/livepatch.h b/arch/powerpc/include/asm/livepatch.h
new file mode 100644
index 000000000000..ad36e8e34fa1
--- /dev/null
+++ b/arch/powerpc/include/asm/livepatch.h
@@ -0,0 +1,54 @@
+/*
+ * livepatch.h - powerpc-specific Kernel Live Patching Core
+ *
+ * Copyright (C) 2015-2016, SUSE, IBM Corp.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _ASM_POWERPC_LIVEPATCH_H
+#define _ASM_POWERPC_LIVEPATCH_H
+
+#include <linux/module.h>
+#include <linux/ftrace.h>
+
+#ifdef CONFIG_LIVEPATCH
+static inline int klp_check_compiler_support(void)
+{
+	return 0;
+}
+
+static inline int klp_write_module_reloc(struct module *mod, unsigned long
+		type, unsigned long loc, unsigned long value)
+{
+	/* This requires infrastructure changes; we need the loadinfos. */
+	return -ENOSYS;
+}
+
+static inline void klp_arch_set_pc(struct pt_regs *regs, unsigned long ip)
+{
+	regs->nip = ip;
+}
+
+#define klp_get_ftrace_location klp_get_ftrace_location
+static inline unsigned long klp_get_ftrace_location(unsigned long faddr)
+{
+	/*
+	 * Live patch works only with -mprofile-kernel on PPC. In this case,
+	 * the ftrace location is always within the first 16 bytes.
+	 */
+	return ftrace_location_range(faddr, faddr + 16);
+}
+#endif /* CONFIG_LIVEPATCH */
+
+#endif /* _ASM_POWERPC_LIVEPATCH_H */
-- 
2.5.0

[PATCH 5/6] powerpc/livepatch: Add livepatch stack to struct thread_info

[Michael Ellerman]

In order to support live patching we need to maintain an alternate
stack of TOC & LR values. We use the base of the stack for this, and
store the "live patch stack pointer" in struct thread_info.

Unlike the other fields of thread_info, we can not statically initialise
that value, so it must be done at run time.

This patch just adds the code to support that, it is not enabled until
the next patch which actually adds live patch support.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/livepatch.h   |  8 ++++++++
 arch/powerpc/include/asm/thread_info.h |  4 +++-
 arch/powerpc/kernel/irq.c              |  3 +++
 arch/powerpc/kernel/process.c          |  6 +++++-
 arch/powerpc/kernel/setup_64.c         | 17 ++++++++++-------
 5 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/include/asm/livepatch.h b/arch/powerpc/include/asm/livepatch.h
index ad36e8e34fa1..a402f7f94896 100644
--- a/arch/powerpc/include/asm/livepatch.h
+++ b/arch/powerpc/include/asm/livepatch.h
@@ -49,6 +49,14 @@ static inline unsigned long klp_get_ftrace_location(unsigned long faddr)
 	 */
 	return ftrace_location_range(faddr, faddr + 16);
 }
+
+static inline void klp_init_thread_info(struct thread_info *ti)
+{
+	/* + 1 to account for STACK_END_MAGIC */
+	ti->livepatch_sp = (unsigned long *)(ti + 1) + 1;
+}
+#else
+static void klp_init_thread_info(struct thread_info *ti) { }
 #endif /* CONFIG_LIVEPATCH */
 
 #endif /* _ASM_POWERPC_LIVEPATCH_H */
diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index 7efee4a3240b..8febc3f66d53 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -43,7 +43,9 @@ struct thread_info {
 	int		preempt_count;		/* 0 => preemptable,
 						   <0 => BUG */
 	unsigned long	local_flags;		/* private flags for thread */
-
+#ifdef CONFIG_LIVEPATCH
+	unsigned long *livepatch_sp;
+#endif
 	/* low level flags - has atomic operations done on it */
 	unsigned long	flags ____cacheline_aligned_in_smp;
 };
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 290559df1e8b..3cb46a3b1de7 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -66,6 +66,7 @@
 #include <asm/udbg.h>
 #include <asm/smp.h>
 #include <asm/debug.h>
+#include <asm/livepatch.h>
 
 #ifdef CONFIG_PPC64
 #include <asm/paca.h>
@@ -607,10 +608,12 @@ void irq_ctx_init(void)
 		memset((void *)softirq_ctx[i], 0, THREAD_SIZE);
 		tp = softirq_ctx[i];
 		tp->cpu = i;
+		klp_init_thread_info(tp);
 
 		memset((void *)hardirq_ctx[i], 0, THREAD_SIZE);
 		tp = hardirq_ctx[i];
 		tp->cpu = i;
+		klp_init_thread_info(tp);
 	}
 }
 
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 612df305886b..c27215e6ef48 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -55,6 +55,8 @@
 #include <asm/firmware.h>
 #endif
 #include <asm/code-patching.h>
+#include <asm/livepatch.h>
+
 #include <linux/kprobes.h>
 #include <linux/kdebug.h>
 
@@ -1400,13 +1402,15 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
 	extern void ret_from_kernel_thread(void);
 	void (*f)(void);
 	unsigned long sp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
+	struct thread_info *ti = task_thread_info(p);
+
+	klp_init_thread_info(ti);
 
 	/* Copy registers */
 	sp -= sizeof(struct pt_regs);
 	childregs = (struct pt_regs *) sp;
 	if (unlikely(p->flags & PF_KTHREAD)) {
 		/* kernel thread */
-		struct thread_info *ti = (void *)task_stack_page(p);
 		memset(childregs, 0, sizeof(struct pt_regs));
 		childregs->gpr[1] = sp + sizeof(struct pt_regs);
 		/* function */
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 3807fb05b6de..1b6cabb8e715 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -69,6 +69,7 @@
 #include <asm/kvm_ppc.h>
 #include <asm/hugetlb.h>
 #include <asm/epapr_hcalls.h>
+#include <asm/livepatch.h>
 
 #ifdef DEBUG
 #define DBG(fmt...) udbg_printf(fmt)
@@ -667,16 +668,16 @@ static void __init emergency_stack_init(void)
 	limit = min(safe_stack_limit(), ppc64_rma_size);
 
 	for_each_possible_cpu(i) {
-		unsigned long sp;
-		sp  = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
-		sp += THREAD_SIZE;
-		paca[i].emergency_sp = __va(sp);
+		struct thread_info *ti;
+		ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit));
+		klp_init_thread_info(ti);
+		paca[i].emergency_sp = (void *)ti + THREAD_SIZE;
 
 #ifdef CONFIG_PPC_BOOK3S_64
 		/* emergency stack for machine check exception handling. */
-		sp  = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
-		sp += THREAD_SIZE;
-		paca[i].mc_emergency_sp = __va(sp);
+		ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit));
+		klp_init_thread_info(ti);
+		paca[i].mc_emergency_sp = (void *)ti + THREAD_SIZE;
 #endif
 	}
 }
@@ -700,6 +701,8 @@ void __init setup_arch(char **cmdline_p)
 	if (ppc_md.panic)
 		setup_panic();
 
+	klp_init_thread_info(&init_thread_info);
+
 	init_mm.start_code = (unsigned long)_stext;
 	init_mm.end_code = (unsigned long) _etext;
 	init_mm.end_data = (unsigned long) _edata;
-- 
2.5.0

Re: [PATCH 5/6] powerpc/livepatch: Add livepatch stack to struct thread_info

[Balbir Singh]

On 24/03/16 22:04, Michael Ellerman wrote:
> In order to support live patching we need to maintain an alternate
> stack of TOC & LR values. We use the base of the stack for this, and
> store the "live patch stack pointer" in struct thread_info.
>
> Unlike the other fields of thread_info, we can not statically initialise
> that value, so it must be done at run time.
>
> This patch just adds the code to support that, it is not enabled until
> the next patch which actually adds live patch support.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/include/asm/livepatch.h   |  8 ++++++++
>  arch/powerpc/include/asm/thread_info.h |  4 +++-
>  arch/powerpc/kernel/irq.c              |  3 +++
>  arch/powerpc/kernel/process.c          |  6 +++++-
>  arch/powerpc/kernel/setup_64.c         | 17 ++++++++++-------
>  5 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/livepatch.h b/arch/powerpc/include/asm/livepatch.h
> index ad36e8e34fa1..a402f7f94896 100644
> --- a/arch/powerpc/include/asm/livepatch.h
> +++ b/arch/powerpc/include/asm/livepatch.h
> @@ -49,6 +49,14 @@ static inline unsigned long klp_get_ftrace_location(unsigned long faddr)
>  	 */
>  	return ftrace_location_range(faddr, faddr + 16);
>  }
> +
> +static inline void klp_init_thread_info(struct thread_info *ti)
> +{
> +	/* + 1 to account for STACK_END_MAGIC */
> +	ti->livepatch_sp = (unsigned long *)(ti + 1) + 1;
> +}
> +#else
> +static void klp_init_thread_info(struct thread_info *ti) { }
>  #endif /* CONFIG_LIVEPATCH */
>  
>  #endif /* _ASM_POWERPC_LIVEPATCH_H */
> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
> index 7efee4a3240b..8febc3f66d53 100644
> --- a/arch/powerpc/include/asm/thread_info.h
> +++ b/arch/powerpc/include/asm/thread_info.h
> @@ -43,7 +43,9 @@ struct thread_info {
>  	int		preempt_count;		/* 0 => preemptable,
>  						   <0 => BUG */
>  	unsigned long	local_flags;		/* private flags for thread */
> -
> +#ifdef CONFIG_LIVEPATCH
> +	unsigned long *livepatch_sp;
> +#endif
>  	/* low level flags - has atomic operations done on it */
>  	unsigned long	flags ____cacheline_aligned_in_smp;
>  };
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index 290559df1e8b..3cb46a3b1de7 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -66,6 +66,7 @@
>  #include <asm/udbg.h>
>  #include <asm/smp.h>
>  #include <asm/debug.h>
> +#include <asm/livepatch.h>
>  
>  #ifdef CONFIG_PPC64
>  #include <asm/paca.h>
> @@ -607,10 +608,12 @@ void irq_ctx_init(void)
>  		memset((void *)softirq_ctx[i], 0, THREAD_SIZE);
>  		tp = softirq_ctx[i];
>  		tp->cpu = i;
> +		klp_init_thread_info(tp);
At this point ti->livepatch_sp points to the next CPUs thread_info for softirq_ctx?
>  
>  		memset((void *)hardirq_ctx[i], 0, THREAD_SIZE);
>  		tp = hardirq_ctx[i];
>  		tp->cpu = i;
> +		klp_init_thread_info(tp);
Same question here
>  	}
>  }
>  
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 612df305886b..c27215e6ef48 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -55,6 +55,8 @@
>  #include <asm/firmware.h>
>  #endif
>  #include <asm/code-patching.h>
> +#include <asm/livepatch.h>
> +
>  #include <linux/kprobes.h>
>  #include <linux/kdebug.h>
>  
> @@ -1400,13 +1402,15 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
>  	extern void ret_from_kernel_thread(void);
>  	void (*f)(void);
>  	unsigned long sp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
> +	struct thread_info *ti = task_thread_info(p);
> +
> +	klp_init_thread_info(ti);
>  
>  	/* Copy registers */
>  	sp -= sizeof(struct pt_regs);
>  	childregs = (struct pt_regs *) sp;
>  	if (unlikely(p->flags & PF_KTHREAD)) {
>  		/* kernel thread */
> -		struct thread_info *ti = (void *)task_stack_page(p);
>  		memset(childregs, 0, sizeof(struct pt_regs));
>  		childregs->gpr[1] = sp + sizeof(struct pt_regs);
>  		/* function */
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 3807fb05b6de..1b6cabb8e715 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -69,6 +69,7 @@
>  #include <asm/kvm_ppc.h>
>  #include <asm/hugetlb.h>
>  #include <asm/epapr_hcalls.h>
> +#include <asm/livepatch.h>
>  
>  #ifdef DEBUG
>  #define DBG(fmt...) udbg_printf(fmt)
> @@ -667,16 +668,16 @@ static void __init emergency_stack_init(void)
>  	limit = min(safe_stack_limit(), ppc64_rma_size);
>  
>  	for_each_possible_cpu(i) {
> -		unsigned long sp;
> -		sp  = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
> -		sp += THREAD_SIZE;
> -		paca[i].emergency_sp = __va(sp);
> +		struct thread_info *ti;
> +		ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit));
> +		klp_init_thread_info(ti);
> +		paca[i].emergency_sp = (void *)ti + THREAD_SIZE;
>  
Does emergency_sp still end up 128 byte aligned after this?
>  #ifdef CONFIG_PPC_BOOK3S_64
>  		/* emergency stack for machine check exception handling. */
> -		sp  = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
> -		sp += THREAD_SIZE;
> -		paca[i].mc_emergency_sp = __va(sp);
> +		ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit));
> +		klp_init_thread_info(ti);
Do we care about live-patching in this context? Are we mixing per-thread and per-cpu contexts?
> +		paca[i].mc_emergency_sp = (void *)ti + THREAD_SIZE;
>  #endif
>  	}
>  }
> @@ -700,6 +701,8 @@ void __init setup_arch(char **cmdline_p)
>  	if (ppc_md.panic)
>  		setup_panic();
>  
> +	klp_init_thread_info(&init_thread_info);
> +
>  	init_mm.start_code = (unsigned long)_stext;
>  	init_mm.end_code = (unsigned long) _etext;
>  	init_mm.end_data = (unsigned long) _edata;

Re: [PATCH 5/6] powerpc/livepatch: Add livepatch stack to struct thread_info

[Michael Ellerman]

On Tue, 2016-03-29 at 10:58 +1100, Balbir Singh wrote:
> On 24/03/16 22:04, Michael Ellerman wrote:
> > diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> > index 290559df1e8b..3cb46a3b1de7 100644
> > --- a/arch/powerpc/kernel/irq.c
> > +++ b/arch/powerpc/kernel/irq.c
> > @@ -66,6 +66,7 @@
> >  #include <asm/udbg.h>
> >  #include <asm/smp.h>
> >  #include <asm/debug.h>
> > +#include <asm/livepatch.h>
> >  
> >  #ifdef CONFIG_PPC64
> >  #include <asm/paca.h>
> > @@ -607,10 +608,12 @@ void irq_ctx_init(void)
> >  		memset((void *)softirq_ctx[i], 0, THREAD_SIZE);
> >  		tp = softirq_ctx[i];
> >  		tp->cpu = i;
> > +		klp_init_thread_info(tp);

> At this point ti->livepatch_sp points to the next CPUs thread_info for softirq_ctx?

Sorry I'm not sure what you mean.

None of this relates to the current CPUs thread info.

> > diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> > index 3807fb05b6de..1b6cabb8e715 100644
> > --- a/arch/powerpc/kernel/setup_64.c
> > +++ b/arch/powerpc/kernel/setup_64.c
> > @@ -69,6 +69,7 @@
> >  #include <asm/kvm_ppc.h>
> >  #include <asm/hugetlb.h>
> >  #include <asm/epapr_hcalls.h>
> > +#include <asm/livepatch.h>
> >  
> >  #ifdef DEBUG
> >  #define DBG(fmt...) udbg_printf(fmt)
> > @@ -667,16 +668,16 @@ static void __init emergency_stack_init(void)
> >  	limit = min(safe_stack_limit(), ppc64_rma_size);
> >  
> >  	for_each_possible_cpu(i) {
> > -		unsigned long sp;
> > -		sp  = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
> > -		sp += THREAD_SIZE;
> > -		paca[i].emergency_sp = __va(sp);
> > +		struct thread_info *ti;
> > +		ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit));
> > +		klp_init_thread_info(ti);
> > +		paca[i].emergency_sp = (void *)ti + THREAD_SIZE;
>  
> Does emergency_sp still end up 128 byte aligned after this?

It should end up THREAD_SIZE aligned as before, due to the memblock_alloc_base().

> >  #ifdef CONFIG_PPC_BOOK3S_64
> >  		/* emergency stack for machine check exception handling. */
> > -		sp  = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
> > -		sp += THREAD_SIZE;
> > -		paca[i].mc_emergency_sp = __va(sp);
> > +		ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit));
> > +		klp_init_thread_info(ti);

> Do we care about live-patching in this context? Are we mixing per-thread and per-cpu contexts?

Well we probably don't want to be doing live patching when we're on the
emergency stacks. But we have no control over whether that happens so we have
to support it.

cheers

Re: [PATCH 5/6] powerpc/livepatch: Add livepatch stack to struct thread_info

[Balbir Singh]

>> At this point ti->livepatch_sp points to the next CPUs thread_info for softirq_ctx?
> Sorry I'm not sure what you mean.
>
> None of this relates to the current CPUs thread info.
Oh! I meant that klp_init_thread_info points to the end of (struct thread_info {} + 1) in the stack of the thread/task, but with the irq_contexts they are a separate array and not on stack
>>> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
>>> index 3807fb05b6de..1b6cabb8e715 100644
>>> --- a/arch/powerpc/kernel/setup_64.c
>>> +++ b/arch/powerpc/kernel/setup_64.c
>>> @@ -69,6 +69,7 @@
>>>  #include <asm/kvm_ppc.h>
>>>  #include <asm/hugetlb.h>
>>>  #include <asm/epapr_hcalls.h>
>>> +#include <asm/livepatch.h>
>>>  
>>>  #ifdef DEBUG
>>>  #define DBG(fmt...) udbg_printf(fmt)
>>> @@ -667,16 +668,16 @@ static void __init emergency_stack_init(void)
>>>  	limit = min(safe_stack_limit(), ppc64_rma_size);
>>>  
>>>  	for_each_possible_cpu(i) {
>>> -		unsigned long sp;
>>> -		sp  = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
>>> -		sp += THREAD_SIZE;
>>> -		paca[i].emergency_sp = __va(sp);
>>> +		struct thread_info *ti;
>>> +		ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit));
>>> +		klp_init_thread_info(ti);
>>> +		paca[i].emergency_sp = (void *)ti + THREAD_SIZE;
>>  
>> Does emergency_sp still end up 128 byte aligned after this?
> It should end up THREAD_SIZE aligned as before, due to the memblock_alloc_base().
Yep.. I missed it.. so where is the space for ti? The stack is going to go grow downwards from ti+THREAD_SIZE
>>>  #ifdef CONFIG_PPC_BOOK3S_64
>>>  		/* emergency stack for machine check exception handling. */
>>> -		sp  = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
>>> -		sp += THREAD_SIZE;
>>> -		paca[i].mc_emergency_sp = __va(sp);
>>> +		ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit));
>>> +		klp_init_thread_info(ti);
>> Do we care about live-patching in this context? Are we mixing per-thread and per-cpu contexts?
> Well we probably don't want to be doing live patching when we're on the
> emergency stacks. But we have no control over whether that happens so we have
> to support it.
>

OK.. I was wondering if the code will even work.. I wonder if the ftrace data structures will work in real mode, including the hash/etc.

Balbir

Re: [PATCH 5/6] powerpc/livepatch: Add livepatch stack to struct thread_info

[Balbir Singh]

On 24/03/16 22:04, Michael Ellerman wrote:
> In order to support live patching we need to maintain an alternate
> stack of TOC & LR values. We use the base of the stack for this, and
> store the "live patch stack pointer" in struct thread_info.
>
> Unlike the other fields of thread_info, we can not statically initialise
> that value, so it must be done at run time.
>
> This patch just adds the code to support that, it is not enabled until
> the next patch which actually adds live patch support.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/include/asm/livepatch.h   |  8 ++++++++
>  arch/powerpc/include/asm/thread_info.h |  4 +++-
>  arch/powerpc/kernel/irq.c              |  3 +++
>  arch/powerpc/kernel/process.c          |  6 +++++-
>  arch/powerpc/kernel/setup_64.c         | 17 ++++++++++-------
>  5 files changed, 29 insertions(+), 9 deletions(-)
>
>

Acked-by: Balbir Singh <bsingharora@gmail.com>

[PATCH 6/6] powerpc/livepatch: Add live patching support on ppc64le

[Michael Ellerman]

Add the kconfig logic & assembly support for handling live patched
functions. This depends on DYNAMIC_FTRACE_WITH_REGS, which in turn
depends on the new -mprofile-kernel ftrace ABI, which is only supported
currently on ppc64le.

Live patching is handled by a special ftrace handler. This means it runs
from ftrace_caller(). The live patch handler modifies the NIP so as to
redirect the return from ftrace_caller() to the new patched function.

However there is one particularly tricky case we need to handle.

If a function A calls another function B, and it is known at link time
that they share the same TOC, then A will not save or restore its TOC,
and will call the local entry point of B.

When we live patch B, we replace it with a new function C, which may
not have the same TOC as A. At live patch time it's too late to modify A
to do the TOC save/restore, so the live patching code must interpose
itself between A and C, and do the TOC save/restore that A omitted.

An additionaly complication is that the livepatch code can not create a
stack frame in order to save the TOC. That is because if C takes > 8
arguments, or is varargs, A will have written the arguments for C in
A's stack frame.

To solve this, we introduce a "livepatch stack" which grows upward from
the base of the regular stack, and is used to store the TOC & LR when
calling a live patched function.

When the patched function returns, we retrieve the real LR & TOC from
the livepatch stack, restore them, and pop the livepatch "stack frame".

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/Kconfig              |  3 ++
 arch/powerpc/kernel/asm-offsets.c |  4 ++
 arch/powerpc/kernel/entry_64.S    | 97 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 104 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 7cd32c038286..ed0603102442 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -160,6 +160,7 @@ config PPC
 	select HAVE_ARCH_SECCOMP_FILTER
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
 	select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
+	select HAVE_LIVEPATCH if HAVE_DYNAMIC_FTRACE_WITH_REGS
 
 config GENERIC_CSUM
 	def_bool CPU_LITTLE_ENDIAN
@@ -1107,3 +1108,5 @@ config PPC_LIB_RHEAP
 	bool
 
 source "arch/powerpc/kvm/Kconfig"
+
+source "kernel/livepatch/Kconfig"
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 0d0183d3180a..c9370d4e36bd 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -86,6 +86,10 @@ int main(void)
 	DEFINE(KSP_LIMIT, offsetof(struct thread_struct, ksp_limit));
 #endif /* CONFIG_PPC64 */
 
+#ifdef CONFIG_LIVEPATCH
+	DEFINE(TI_livepatch_sp, offsetof(struct thread_info, livepatch_sp));
+#endif
+
 	DEFINE(KSP, offsetof(struct thread_struct, ksp));
 	DEFINE(PT_REGS, offsetof(struct thread_struct, regs));
 #ifdef CONFIG_BOOKE
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 9916d150b28c..39a79c89a4b6 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -20,6 +20,7 @@
 
 #include <linux/errno.h>
 #include <linux/err.h>
+#include <linux/magic.h>
 #include <asm/unistd.h>
 #include <asm/processor.h>
 #include <asm/page.h>
@@ -1248,6 +1249,9 @@ _GLOBAL(ftrace_caller)
 	addi	r3,r3,function_trace_op@toc@l
 	ld	r5,0(r3)
 
+#ifdef CONFIG_LIVEPATCH
+	mr	r14,r7		/* remember old NIP */
+#endif
 	/* Calculate ip from nip-4 into r3 for call below */
 	subi    r3, r7, MCOUNT_INSN_SIZE
 
@@ -1272,6 +1276,9 @@ ftrace_call:
 	/* Load ctr with the possibly modified NIP */
 	ld	r3, _NIP(r1)
 	mtctr	r3
+#ifdef CONFIG_LIVEPATCH
+	cmpd	r14,r3		/* has NIP been altered? */
+#endif
 
 	/* Restore gprs */
 	REST_8GPRS(0,r1)
@@ -1289,6 +1296,11 @@ ftrace_call:
 	ld	r0, LRSAVE(r1)
 	mtlr	r0
 
+#ifdef CONFIG_LIVEPATCH
+        /* Based on the cmpd above, if the NIP was altered handle livepatch */
+	bne-	livepatch_handler
+#endif
+
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	stdu	r1, -112(r1)
 .globl ftrace_graph_call
@@ -1305,6 +1317,91 @@ _GLOBAL(ftrace_graph_stub)
 
 _GLOBAL(ftrace_stub)
 	blr
+
+#ifdef CONFIG_LIVEPATCH
+	/*
+	 * This function runs in the mcount context, between two functions. As
+	 * such it can only clobber registers which are volatile and used in
+	 * function linkage.
+	 *
+	 * We get here when a function A, calls another function B, but B has
+	 * been live patched with a new function C.
+	 *
+	 * On entry:
+	 *  - we have no stack frame and can not allocate one
+	 *  - LR points back to the original caller (in A)
+	 *  - CTR holds the new NIP in C
+	 *  - r0 & r12 are free
+	 *
+	 * r0 can't be used as the base register for a DS-form load or store, so
+	 * we temporarily shuffle r1 (stack pointer) into r0 and then put it back.
+	 */
+livepatch_handler:
+	CURRENT_THREAD_INFO(r12, r1)
+
+	/* Save stack pointer into r0 */
+	mr	r0, r1
+
+	/* Allocate 3 x 8 bytes */
+	ld	r1, TI_livepatch_sp(r12)
+	addi	r1, r1, 24
+	std	r1, TI_livepatch_sp(r12)
+
+	/* Save toc & real LR on livepatch stack */
+	std	r2,  -24(r1)
+	mflr	r12
+	std	r12, -16(r1)
+
+	/* Store stack end marker */
+	lis     r12, STACK_END_MAGIC@h
+	ori     r12, r12, STACK_END_MAGIC@l
+	std	r12, -8(r1)
+
+	/* Restore real stack pointer */
+	mr	r1, r0
+
+	/* Put ctr in r12 for global entry and branch there */
+	mfctr	r12
+	bctrl
+
+	/*
+	 * Now we are returning from the patched function to the original
+	 * caller A. We are free to use r0 and r12, and we can use r2 until we
+	 * restore it.
+	 */
+
+	CURRENT_THREAD_INFO(r12, r1)
+
+	/* Save stack pointer into r0 */
+	mr	r0, r1
+
+	ld	r1, TI_livepatch_sp(r12)
+
+	/* Check stack marker hasn't been trashed */
+	lis     r2,  STACK_END_MAGIC@h
+	ori     r2,  r2, STACK_END_MAGIC@l
+	ld	r12, -8(r1)
+1:	tdne	r12, r2
+	EMIT_BUG_ENTRY 1b, __FILE__, __LINE__ - 1, 0
+
+	/* Restore LR & toc from livepatch stack */
+	ld	r12, -16(r1)
+	mtlr	r12
+	ld	r2,  -24(r1)
+
+	/* Pop livepatch stack frame */
+	CURRENT_THREAD_INFO(r12, r0)
+	subi	r1, r1, 24
+	std	r1, TI_livepatch_sp(r12)
+
+	/* Restore real stack pointer */
+	mr	r1, r0
+
+	/* Return to original caller of live patched function */
+	blr
+#endif
+
+
 #else
 _GLOBAL_TOC(_mcount)
 	/* Taken from output of objdump from lib64/glibc */
-- 
2.5.0

Re: [PATCH 6/6] powerpc/livepatch: Add live patching support on ppc64le

[Torsten Duwe]

On Thu, Mar 24, 2016 at 10:04:05PM +1100, Michael Ellerman wrote:
> +livepatch_handler:
> +	CURRENT_THREAD_INFO(r12, r1)
[...]
> +	/* Put ctr in r12 for global entry and branch there */
> +	mfctr	r12
> +	bctrl
            ^
I like this piece. No need to fiddle out the return helper address.

> +	/*
> +	 * Now we are returning from the patched function to the original
> +	 * caller A. We are free to use r0 and r12, and we can use r2 until we
> +	 * restore it.
> +	 */
> +
> +	CURRENT_THREAD_INFO(r12, r1)
> +
> +	/* Save stack pointer into r0 */
> +	mr	r0, r1
> +
> +	ld	r1, TI_livepatch_sp(r12)
> +
> +	/* Check stack marker hasn't been trashed */
> +	lis     r2,  STACK_END_MAGIC@h
> +	ori     r2,  r2, STACK_END_MAGIC@l
> +	ld	r12, -8(r1)
> +1:	tdne	r12, r2
> +	EMIT_BUG_ENTRY 1b, __FILE__, __LINE__ - 1, 0

This however worries me a bit. Sure, in the end, a stack overflow is
a stack overflow, and if all the information does not fit there,
there's little you can do.

But wouldn't it be better to kmalloc that area and realloc in
klp_arch_set_pc when it's full? Maybe along with a warning message?
That way a live patched kernel will not run into stack size problems
any earlier than an unpatched kernel would.
Just a thought.

Anyway, patch 5+6

Reviewed-by: Torsten Duwe <duwe@suse.de>

	Torsten

Re: [PATCH 6/6] powerpc/livepatch: Add live patching support on ppc64le

[Michael Ellerman]

On Thu, 2016-03-24 at 14:42 +0100, Torsten Duwe wrote:

> On Thu, Mar 24, 2016 at 10:04:05PM +1100, Michael Ellerman wrote:

> > +livepatch_handler:
> > +	CURRENT_THREAD_INFO(r12, r1)
> [...]

> > +	/* Put ctr in r12 for global entry and branch there */
> > +	mfctr	r12
> > +	bctrl
>             ^
> I like this piece. No need to fiddle out the return helper address.

Good.

> > +	/*
> > +	 * Now we are returning from the patched function to the original
> > +	 * caller A. We are free to use r0 and r12, and we can use r2 until we
> > +	 * restore it.
> > +	 */
> > +
> > +	CURRENT_THREAD_INFO(r12, r1)
> > +
> > +	/* Save stack pointer into r0 */
> > +	mr	r0, r1
> > +
> > +	ld	r1, TI_livepatch_sp(r12)
> > +
> > +	/* Check stack marker hasn't been trashed */
> > +	lis     r2,  STACK_END_MAGIC@h
> > +	ori     r2,  r2, STACK_END_MAGIC@l
> > +	ld	r12, -8(r1)
> > +1:	tdne	r12, r2
> > +	EMIT_BUG_ENTRY 1b, __FILE__, __LINE__ - 1, 0
> 
> This however worries me a bit. Sure, in the end, a stack overflow is
> a stack overflow, and if all the information does not fit there,
> there's little you can do.

Yeah stack overflow in the kernel is very very fatal.

> But wouldn't it be better to kmalloc that area and realloc in
> klp_arch_set_pc when it's full? Maybe along with a warning message?

You can't realloc in klp_arch_set_pc(), you might be patching sl*b and holding
one of its locks. You might also recurse.

We could allocate a larger buffer as a "klp stack" for each task when the first
live patch is installed, and for every task created afterward. But that
potentially significantly increases memory usage on live patched kernels :)

> That way a live patched kernel will not run into stack size problems
> any earlier than an unpatched kernel would.

Yeah that's true. I'm not sure what the best trade off is.

cheers

Re: [PATCH 6/6] powerpc/livepatch: Add live patching support on ppc64le

[Balbir Singh]

On 24/03/16 22:04, Michael Ellerman wrote:
> Add the kconfig logic & assembly support for handling live patched
> functions. This depends on DYNAMIC_FTRACE_WITH_REGS, which in turn
> depends on the new -mprofile-kernel ftrace ABI, which is only supported
> currently on ppc64le.
>
> Live patching is handled by a special ftrace handler. This means it runs
> from ftrace_caller(). The live patch handler modifies the NIP so as to
> redirect the return from ftrace_caller() to the new patched function.
>
> However there is one particularly tricky case we need to handle.
>
> If a function A calls another function B, and it is known at link time
> that they share the same TOC, then A will not save or restore its TOC,
> and will call the local entry point of B.
>
> When we live patch B, we replace it with a new function C, which may
> not have the same TOC as A. At live patch time it's too late to modify A
> to do the TOC save/restore, so the live patching code must interpose
> itself between A and C, and do the TOC save/restore that A omitted.
>
> An additionaly complication is that the livepatch code can not create a
> stack frame in order to save the TOC. That is because if C takes > 8
> arguments, or is varargs, A will have written the arguments for C in
> A's stack frame.
>
> To solve this, we introduce a "livepatch stack" which grows upward from
> the base of the regular stack, and is used to store the TOC & LR when
> calling a live patched function.
>
> When the patched function returns, we retrieve the real LR & TOC from
> the livepatch stack, restore them, and pop the livepatch "stack frame".
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>

Reviewed-by: Balbir Singh <bsingharora@gmail.com>

Re: [PATCH HACK 1/6] livepatch-test: Add more cases

[Kamalesh Babulal]

* Michael Ellerman <mpe@ellerman.id.au> [2016-03-24 22:04:00]:

> Not for merging.
> 

Hi Michael,

Loading the livepatch sample module, trigger following warning

Mar 24 21:44:59 ubuntu kernel: [   40.467580] sysfs: cannot create duplicate filename '/kernel/livepatch/livepatch_sample/vmlinux'
Mar 24 21:44:59 ubuntu kernel: [   40.467601] ------------[ cut here ]------------
Mar 24 21:44:59 ubuntu kernel: [   40.467603] WARNING: at fs/sysfs/dir.c:31
Mar 24 21:44:59 ubuntu kernel: [   40.467604] Modules linked in: livepatch_sample(+) rtc_generic autofs4 ibmvscsi
Mar 24 21:44:59 ubuntu kernel: [   40.467610] CPU: 5 PID: 1479 Comm: modprobe Not tainted 4.5.0+ #2
Mar 24 21:44:59 ubuntu kernel: [   40.467612] task: c000000039a1b800 ti: c00000003767c000 task.ti: c00000003767c000
Mar 24 21:44:59 ubuntu kernel: [   40.467614] NIP: c00000000038bddc LR: c00000000038bdd8 CTR: c000000000557b20
Mar 24 21:44:59 ubuntu kernel: [   40.467615] REGS: c00000003767f4b0 TRAP: 0700   Not tainted  (4.5.0+)
Mar 24 21:44:59 ubuntu kernel: [   40.467616] MSR: 8000000102029033 <SF,VEC,EE,ME,IR,DR,RI,LE,TM[E]>  CR: 28222442  XER: 20000000
Mar 24 21:44:59 ubuntu kernel: [   40.467624] CFAR: c000000000a65fac SOFTE: 1 
Mar 24 21:44:59 ubuntu kernel: [   40.467624] GPR00: c00000000038bdd8 c00000003767f730 c0000000014c3d00 0000000000000054 
Mar 24 21:44:59 ubuntu kernel: [   40.467624] GPR04: c00000003fe49c58 c00000003fe5b4d8 0000000000000064 000000000000010f 
Mar 24 21:44:59 ubuntu kernel: [   40.467624] GPR08: 0000000000000007 c000000000ecb274 000000003ef80000 7461706576696c2f 
Mar 24 21:44:59 ubuntu kernel: [   40.467624] GPR12: 0000000000002200 c00000000fe80f00 d00000000155c498 c00000003767fdb0 
Mar 24 21:44:59 ubuntu kernel: [   40.467624] GPR16: 000000000000003d c0000000013cc318 0000000000000000 c000000000cf1510 
Mar 24 21:44:59 ubuntu kernel: [   40.467624] GPR20: c0000000013cc328 c000000001478f28 d000000001570cb8 d000000001570c98 
Mar 24 21:44:59 ubuntu kernel: [   40.467624] GPR24: c000000000cc0070 c000000001478f50 0000000000000000 d000000001570cb8 
Mar 24 21:44:59 ubuntu kernel: [   40.467624] GPR28: d0000000015708d0 c00000003c750870 c000000000cbfe68 c00000003a730000 
Mar 24 21:44:59 ubuntu kernel: [   40.467642] NIP [c00000000038bddc] sysfs_warn_dup+0x8c/0xc0
Mar 24 21:44:59 ubuntu kernel: [   40.467645] LR [c00000000038bdd8] sysfs_warn_dup+0x88/0xc0
Mar 24 21:44:59 ubuntu kernel: [   40.467645] Call Trace:
Mar 24 21:44:59 ubuntu kernel: [   40.467648] [c00000003767f730] [c00000000038bdd8] sysfs_warn_dup+0x88/0xc0 (unreliable)
Mar 24 21:44:59 ubuntu kernel: [   40.467650] [c00000003767f7b0] [c00000000038bfac] sysfs_create_dir_ns+0xdc/0x100
Mar 24 21:44:59 ubuntu kernel: [   40.467652] [c00000003767f7f0] [c00000000054b354] kobject_add_internal+0xe4/0x420
Mar 24 21:44:59 ubuntu kernel: [   40.467654] [c00000003767f880] [c00000000054ba34] kobject_init_and_add+0x64/0xa0
Mar 24 21:44:59 ubuntu kernel: [   40.467657] [c00000003767f900] [c000000000135e8c] klp_register_patch+0x1ac/0x3b0
Mar 24 21:44:59 ubuntu kernel: [   40.467659] [c00000003767f9e0] [d000000001570354] livepatch_init+0x34/0xc0 [livepatch_sample]
Mar 24 21:44:59 ubuntu kernel: [   40.467661] [c00000003767fa10] [c00000000000b5ac] do_one_initcall+0x12c/0x2a0
Mar 24 21:44:59 ubuntu kernel: [   40.467664] [c00000003767fae0] [c000000000a6647c] do_init_module+0x98/0x254
Mar 24 21:44:59 ubuntu kernel: [   40.467666] [c00000003767fb70] [c000000000163178] load_module+0x22c8/0x2a80
Mar 24 21:44:59 ubuntu kernel: [   40.467668] [c00000003767fd40] [c000000000163c90] SyS_finit_module+0x110/0x170
Mar 24 21:44:59 ubuntu kernel: [   40.467670] [c00000003767fe30] [c000000000009204] system_call+0x38/0xb4
Mar 24 21:44:59 ubuntu kernel: [   40.467671] Instruction dump:
Mar 24 21:44:59 ubuntu kernel: [   40.467672] 4182001c 7fe4fb78 38a01000 7fa3eb78 4bffa3bd 60000000 7c641b78 3c62ff82 
Mar 24 21:44:59 ubuntu kernel: [   40.467676] 7fc5f378 386392c0 486da175 60000000 <0fe00000> 7fe3fb78 4bf16c95 60000000 
Mar 24 21:44:59 ubuntu kernel: [   40.467680] ---[ end trace 77c74eaaf32878c5 ]---
Mar 24 21:44:59 ubuntu kernel: [   40.467681] kobject_add_internal failed for vmlinux with -EEXIST, don't try to register things with the same name in the same directory.
Mar 24 21:44:59 ubuntu kernel: [   40.467694] ------------[ cut here ]------------
Mar 24 21:44:59 ubuntu kernel: [   40.467696] WARNING: at lib/kobject.c:240
Mar 24 21:44:59 ubuntu kernel: [   40.467697] Modules linked in: livepatch_sample(+) rtc_generic autofs4 ibmvscsi
Mar 24 21:44:59 ubuntu kernel: [   40.467700] CPU: 5 PID: 1479 Comm: modprobe Tainted: G        W       4.5.0+ #2
Mar 24 21:44:59 ubuntu kernel: [   40.467702] task: c000000039a1b800 ti: c00000003767c000 task.ti: c00000003767c000
Mar 24 21:44:59 ubuntu kernel: [   40.467703] NIP: c00000000054b5ec LR: c00000000054b5e8 CTR: c000000000557b20
Mar 24 21:44:59 ubuntu kernel: [   40.467704] REGS: c00000003767f570 TRAP: 0700   Tainted: G        W        (4.5.0+)
Mar 24 21:44:59 ubuntu kernel: [   40.467706] MSR: 8000000102029033 <SF,VEC,EE,ME,IR,DR,RI,LE,TM[E]>  CR: 28222448  XER: 20000000
Mar 24 21:44:59 ubuntu kernel: [   40.467711] CFAR: c000000000a65fac SOFTE: 1 
Mar 24 21:44:59 ubuntu kernel: [   40.467711] GPR00: c00000000054b5e8 c00000003767f7f0 c0000000014c3d00 000000000000007c 
Mar 24 21:44:59 ubuntu kernel: [   40.467711] GPR04: 0000000000000001 0000000000000000 000000000000008c 000000000000012b 
Mar 24 21:44:59 ubuntu kernel: [   40.467711] GPR08: 0000000000000007 0000000000000007 0000000000000001 6f64202c54534958 
Mar 24 21:44:59 ubuntu kernel: [   40.467711] GPR12: 0000000000002200 c00000000fe80f00 d00000000155c498 c00000003767fdb0 
Mar 24 21:44:59 ubuntu kernel: [   40.467711] GPR16: 000000000000003d c0000000013cc318 0000000000000000 c000000000cf1510 
Mar 24 21:44:59 ubuntu kernel: [   40.467711] GPR20: c0000000013cc328 c000000001478f28 d000000001570cb8 d000000001570c98 
Mar 24 21:44:59 ubuntu kernel: [   40.467711] GPR24: c000000000cc0070 c000000001478f50 0000000000000000 d000000001570cb8 
Mar 24 21:44:59 ubuntu kernel: [   40.467711] GPR28: d0000000015708d0 d000000001570cb8 ffffffffffffffef d0000000015708d0 
Mar 24 21:44:59 ubuntu kernel: [   40.467727] NIP [c00000000054b5ec] kobject_add_internal+0x37c/0x420
Mar 24 21:44:59 ubuntu kernel: [   40.467729] LR [c00000000054b5e8] kobject_add_internal+0x378/0x420
Mar 24 21:44:59 ubuntu kernel: [   40.467730] Call Trace:
Mar 24 21:44:59 ubuntu kernel: [   40.467732] [c00000003767f7f0] [c00000000054b5e8] kobject_add_internal+0x378/0x420 (unreliable)
Mar 24 21:44:59 ubuntu kernel: [   40.467734] [c00000003767f880] [c00000000054ba34] kobject_init_and_add+0x64/0xa0
Mar 24 21:44:59 ubuntu kernel: [   40.467736] [c00000003767f900] [c000000000135e8c] klp_register_patch+0x1ac/0x3b0
Mar 24 21:44:59 ubuntu kernel: [   40.467739] [c00000003767f9e0] [d000000001570354] livepatch_init+0x34/0xc0 [livepatch_sample]
Mar 24 21:44:59 ubuntu kernel: [   40.467741] [c00000003767fa10] [c00000000000b5ac] do_one_initcall+0x12c/0x2a0
Mar 24 21:44:59 ubuntu kernel: [   40.467743] [c00000003767fae0] [c000000000a6647c] do_init_module+0x98/0x254
Mar 24 21:44:59 ubuntu kernel: [   40.467745] [c00000003767fb70] [c000000000163178] load_module+0x22c8/0x2a80
Mar 24 21:44:59 ubuntu kernel: [   40.467747] [c00000003767fd40] [c000000000163c90] SyS_finit_module+0x110/0x170
Mar 24 21:44:59 ubuntu kernel: [   40.467749] [c00000003767fe30] [c000000000009204] system_call+0x38/0xb4
Mar 24 21:44:59 ubuntu kernel: [   40.467750] Instruction dump:
Mar 24 21:44:59 ubuntu kernel: [   40.467751] 60420000 3ce2ff83 38e75bf0 4bfffdcc 60420000 e8bf0000 3c82ff5f 3c62ff83 
Mar 24 21:44:59 ubuntu kernel: [   40.467755] 3884cd48 38635c88 4851a965 60000000 <0fe00000> 4bfffdc4 3ce2fff8 89070be3 
Mar 24 21:44:59 ubuntu kernel: [   40.467758] ---[ end trace 77c74eaaf32878c6 ]---

I am away for the long weekend, will help in testing next week.

Thanks,
Kamalesh.

Re: [PATCH HACK 1/6] livepatch-test: Add more cases

[Balbir Singh]

On Fri, Mar 25, 2016 at 3:37 AM, Kamalesh Babulal
<kamalesh@linux.vnet.ibm.com> wrote:
> * Michael Ellerman <mpe@ellerman.id.au> [2016-03-24 22:04:00]:
>
>> Not for merging.
>>
>
> Hi Michael,
>
> Loading the livepatch sample module, trigger following warning
>

The #if IS_MODULE(CONFIG_SCSI) code is buggy, you probably have
CONFIG_SCSI=y, make it M or you can fix the code yourself. I had the
same issue while testing

BTW, the tests worked fine with the changes proposed by Michael.

Balbir Singh

Re: [PATCH HACK 1/6] livepatch-test: Add more cases

[Kamalesh Babulal]

* Balbir Singh <bsingharora@gmail.com> [2016-03-26 18:11:22]:

> On Fri, Mar 25, 2016 at 3:37 AM, Kamalesh Babulal
> <kamalesh@linux.vnet.ibm.com> wrote:
> > * Michael Ellerman <mpe@ellerman.id.au> [2016-03-24 22:04:00]:
> >
> >> Not for merging.
> >>
> >
> > Hi Michael,
> >
> > Loading the livepatch sample module, trigger following warning
> >
> 
> The #if IS_MODULE(CONFIG_SCSI) code is buggy, you probably have
> CONFIG_SCSI=y, make it M or you can fix the code yourself. I had the
> same issue while testing
> 
> BTW, the tests worked fine with the changes proposed by Michael.
> 

Thanks, It helped. I was able to load the sample livepatch module
with proposed changes.

Thanks,
Kamalesh.