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

Groups > linux.kernel > #1537127 > unrolled thread

[PATCH 0/7] arm: Add livepatch support

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] arm: Add livepatch support Abel Vesa <abelvesa@linux.com> - 2016-12-06 18:10 +0100
    [PATCH 1/7] arm: Add livepatch arch specific code Abel Vesa <abelvesa@linux.com> - 2016-12-06 18:10 +0100
    [PATCH 3/7] arm: module: Add apply_relocate_add Abel Vesa <abelvesa@linux.com> - 2016-12-06 18:10 +0100
      Re: [PATCH 3/7] arm: module: Add apply_relocate_add kbuild test robot <lkp@intel.com> - 2016-12-07 03:10 +0100
    [PATCH 7/7] arm: Add livepatch necessary arch selects into Kconfig Abel Vesa <abelvesa@linux.com> - 2016-12-06 18:10 +0100
      Re: [PATCH 7/7] arm: Add livepatch necessary arch selects into  Kconfig Steven Rostedt <rostedt@goodmis.org> - 2016-12-07 03:50 +0100
    Re: [PATCH 0/7] arm: Add livepatch support zhouchengming <zhouchengming1@huawei.com> - 2016-12-07 02:40 +0100
      Re: [PATCH 0/7] arm: Add livepatch support Abel Vesa <abelvesa@gmail.com> - 2016-12-07 12:50 +0100
    Re: [PATCH 0/7] arm: Add livepatch support Petr Mladek <pmladek@suse.com> - 2016-12-07 16:30 +0100

#1537127 — [PATCH 0/7] arm: Add livepatch support

This is just an idea I've been trying out for a while now. 

Just in case somebody wants to play with it, this applies to linux-arm/for-next.

Also please note that this was only tested in qemu, but I will do some testing 
on some real hardware in the following days.

FWICT, on this arch the compiler always generates a function prologue somewhere
between these lines:

e1a0c00d        mov     ip, sp
e92ddff0        push    {r4-r9, sl, fp, ip, lr, pc}
e24cb004        sub     fp, ip, #4
e24dd064        sub     sp, sp, #100    ; 0x64 <--- local variables
e52de004        push    {lr}            ; (str lr, [sp, #-4]!)
ebf9c2c9        bl      80110364 <__gnu_mcount_nc>
....

Every function that follows this pattern (the number of registers pushed and the
sp subtraction for the local variables being the only acceptable exception) can
be patched with this mechanism. IIRC, only the inline functions and notrace 
functions do not follow this pattern.

Considering that the function is livepatchable, when the time comes to call
ftrace_call, the ftrace_regs_caller is called instead.

Because this arch didn't have a ftrace with regs implementation, the
ftrace_regs_caller was added.

This new function adds the regs saving/restoring part, plus the part necessary
for the livepatch mechanism to work. After the regs are saved and the r3 is set
to contain the sp's value, we're keeping the old pc into r10 in order to be
checked later against the new pc.

Next, the r1 and r0 are set for the ftrace_func, then, the ftrace_stub is called
and the klp_ftrace_handler overwrites the old pc with the new one.

Here comes the tricky part. We're checking if the pc is still the old one, if it
is we jump the whole livepatching and go ahead with restoring the saved regs.

If the pc is modified, it means we're livepatching current function and we need
to pop all regs from r1 through r12, jump over the next two regs saved on stack
(we're not interested in those since we're trying to get the same regs context
as it was at the point the function-to-be-patched was called) and put the new pc
into r11.

Since r12 contains the sp from when the function just got branched to, we need
to set the sp back to that.

Then we need to put the new pc on stack so that when we're popping r11 through 
pc, we will actually jump to the first instruction from the new function.

We don't need to worry about the returning phase since the epilogue of the new
function will take care of that and from there on everything goes back to 
normal.

The whole advantage of this over adding compiler support is that we're not
introducing nops at the beginning of the function. As a matter of fact, we're
not changing anything between an image with livepatch and an image without it
(except the ftrace_regs_call addition and the livepatch necessary code).

As for the implementation of the ftrace_regs_caller, I still think there might
be some unsafe stack handling since I'm getting some build warnings. Those are
due to pushing/popping of a list of regs in which the sp resides. I'll try to 
get around those in a next iteration (if necessary), but first I would like to
hear some opinions about this work and if it's worth going forward.

Everything else should be pretty straightforward, so I'll skip explaining that.

Abel Vesa (7):
  arm: Add livepatch arch specific code
  arm: ftrace: Add call modify mechanism
  arm: module: Add apply_relocate_add
  arm: Add ftrace with regs support
  arm: ftrace: Add ARCH_SUPPORTS_FTRACE_OPS for ftrace with regs
  arm: Add livepatch to build if CONFIG_LIVEPATCH
  arm: Add livepatch necessary arch selects into Kconfig

 MAINTAINERS                      |  3 +++
 arch/arm/Kconfig                 |  4 ++++
 arch/arm/include/asm/ftrace.h    |  4 ++++
 arch/arm/include/asm/livepatch.h | 46 +++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/Makefile         |  1 +
 arch/arm/kernel/entry-ftrace.S   | 49 ++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/ftrace.c         | 21 +++++++++++++++++
 arch/arm/kernel/livepatch.c      | 43 +++++++++++++++++++++++++++++++++++
 arch/arm/kernel/module.c         |  9 ++++++++
 9 files changed, 180 insertions(+)
 create mode 100644 arch/arm/include/asm/livepatch.h
 create mode 100644 arch/arm/kernel/livepatch.c

-- 
2.7.4

[toc] | [next] | [standalone]

#1537128 — [PATCH 1/7] arm: Add livepatch arch specific code

klp_get_ftrace_location is used by ftrace to get the entry for a
specific function from the mcount list. klp_arch_set_pc is used
to set the pc from the regs passed as an argument to the
ftrace_ops_no_ops function to the starting address of the patched
function. klp_write_module_reloc is not doing anything at this
moment.

Signed-off-by: Abel Vesa <abelvesa@linux.com>
---
 MAINTAINERS                      |  3 +++
 arch/arm/include/asm/livepatch.h | 46 ++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/livepatch.c      | 43 +++++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+)
 create mode 100644 arch/arm/include/asm/livepatch.h
 create mode 100644 arch/arm/kernel/livepatch.c

diff --git a/MAINTAINERS b/MAINTAINERS
index bd182a1..d43b790 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7466,12 +7466,15 @@ M:	Josh Poimboeuf <jpoimboe@redhat.com>
 M:	Jessica Yu <jeyu@redhat.com>
 M:	Jiri Kosina <jikos@kernel.org>
 M:	Miroslav Benes <mbenes@suse.cz>
+M:	Abel Vesa <abelvesa@linux.com>
 R:	Petr Mladek <pmladek@suse.com>
 S:	Maintained
 F:	kernel/livepatch/
 F:	include/linux/livepatch.h
 F:	arch/x86/include/asm/livepatch.h
 F:	arch/x86/kernel/livepatch.c
+F:	arch/arm/include/asm/livepatch.h
+F:	arch/arm/kernel/livepatch.c
 F:	Documentation/livepatch/
 F:	Documentation/ABI/testing/sysfs-kernel-livepatch
 F:	samples/livepatch/
diff --git a/arch/arm/include/asm/livepatch.h b/arch/arm/include/asm/livepatch.h
new file mode 100644
index 0000000..d4e3ff0
--- /dev/null
+++ b/arch/arm/include/asm/livepatch.h
@@ -0,0 +1,46 @@
+/*
+ * livepatch.h - arm specific Kernel Live Patching Core
+ *
+ * Copyright (C) 2016 Abel Vesa <abelvesa@linux.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _ASM_ARM_LIVEPATCH_H
+#define _ASM_ARM_LIVEPATCH_H
+
+#include <asm/setup.h>
+#include <linux/module.h>
+#include <linux/ftrace.h>
+
+static inline int klp_check_compiler_support(void)
+{
+	return 0;
+}
+
+int klp_write_module_reloc(struct module *mod, unsigned long type,
+			   unsigned long loc, unsigned long value);
+
+static inline void klp_arch_set_pc(struct pt_regs *regs, unsigned long ip)
+{
+	regs->uregs[15] = ip;
+}
+
+#define klp_get_ftrace_location klp_get_ftrace_location
+static inline unsigned long klp_get_ftrace_location(unsigned long faddr)
+{
+	return ftrace_location_range(faddr, faddr + 24);
+}
+
+#endif /* _ASM_ARM_LIVEPATCH_H */
diff --git a/arch/arm/kernel/livepatch.c b/arch/arm/kernel/livepatch.c
new file mode 100644
index 0000000..0656cd6
--- /dev/null
+++ b/arch/arm/kernel/livepatch.c
@@ -0,0 +1,43 @@
+/*
+ * livepatch.c - arm specific Kernel Live Patching Core
+ *
+ * Copyright (C) 2016 Abel Vesa <abelvesa@linux.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/module.h>
+#include <linux/uaccess.h>
+#include <linux/ftrace.h>
+#include <asm/elf.h>
+#include <asm/livepatch.h>
+#include <asm/insn.h>
+#include <asm/ftrace.h>
+
+/**
+ * klp_write_module_reloc() - write a relocation in a module
+ * @mod:	module in which the section to be modified is found
+ * @type:	ELF relocation type (see asm/elf.h)
+ * @loc:	address that the relocation should be written to
+ * @value:	relocation value (sym address + addend)
+ *
+ * This function writes a relocation to the specified location for
+ * a particular module.
+ */
+int klp_write_module_reloc(struct module *mod, unsigned long type,
+			   unsigned long loc, unsigned long value)
+{
+	/* Not implemented yet */
+	return 0;
+}
-- 
2.7.4

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

#1537129 — [PATCH 3/7] arm: module: Add apply_relocate_add

It was only added to fix compiler error. It is not implemented
yet.

Signed-off-by: Abel Vesa <abelvesa@linux.com>
---
 arch/arm/kernel/module.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index 4f14b5c..bf94922 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -52,6 +52,15 @@ void *module_alloc(unsigned long size)
 #endif
 
 int
+apply_relocate_add(Elf32_Shdr *sechdrs, const char *strtab,
+		   unsigned int symindex, unsigned int relindex,
+		   struct module *module)
+{
+	/* Not implemented yet */
+	return 0;
+}
+
+int
 apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex,
 	       unsigned int relindex, struct module *module)
 {
-- 
2.7.4

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

#1537417 — Re: [PATCH 3/7] arm: module: Add apply_relocate_add

[Multipart message — attachments visible in raw view] — view raw

Hi Abel,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.9-rc8 next-20161206]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Abel-Vesa/arm-Add-livepatch-support/20161207-074210
config: arm-simpad_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

Note: the linux-review/Abel-Vesa/arm-Add-livepatch-support/20161207-074210 HEAD 49113edc744f38a682a4afa9e904384bb00f2988 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

>> arch/arm/kernel/module.c:55:1: error: redefinition of 'apply_relocate_add'
    apply_relocate_add(Elf32_Shdr *sechdrs, const char *strtab,
    ^~~~~~~~~~~~~~~~~~
   In file included from arch/arm/kernel/module.c:14:0:
   include/linux/moduleloader.h:65:19: note: previous definition of 'apply_relocate_add' was here
    static inline int apply_relocate_add(Elf_Shdr *sechdrs,
                      ^~~~~~~~~~~~~~~~~~

vim +/apply_relocate_add +55 arch/arm/kernel/module.c

    49					GFP_KERNEL, PAGE_KERNEL_EXEC, 0, NUMA_NO_NODE,
    50					__builtin_return_address(0));
    51	}
    52	#endif
    53	
    54	int
  > 55	apply_relocate_add(Elf32_Shdr *sechdrs, const char *strtab,
    56			   unsigned int symindex, unsigned int relindex,
    57			   struct module *module)
    58	{

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

#1537133 — [PATCH 7/7] arm: Add livepatch necessary arch selects into Kconfig

This adds HAVE_LIVEPATCH, MODULES_USE_ELF_RELA and HAVE_LIVEPATCH
to arm Kconfig.

Signed-off-by: Abel Vesa <abelvesa@linux.com>
---
 arch/arm/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 186c4c2..f4e9ace 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -50,6 +50,7 @@ config ARM
 	select HAVE_DMA_API_DEBUG
 	select HAVE_DMA_CONTIGUOUS if MMU
 	select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
+	select HAVE_DYNAMIC_FTRACE_WITH_REGS
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
 	select HAVE_EXIT_THREAD
 	select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
@@ -67,6 +68,7 @@ config ARM
 	select HAVE_KERNEL_XZ
 	select HAVE_KPROBES if !XIP_KERNEL && !CPU_ENDIAN_BE32 && !CPU_V7M
 	select HAVE_KRETPROBES if (HAVE_KPROBES)
+	select HAVE_LIVEPATCH
 	select HAVE_MEMBLOCK
 	select HAVE_MOD_ARCH_SPECIFIC
 	select HAVE_NMI
@@ -82,6 +84,7 @@ config ARM
 	select HAVE_VIRT_CPU_ACCOUNTING_GEN
 	select IRQ_FORCED_THREADING
 	select MODULES_USE_ELF_REL
+	select MODULES_USE_ELF_RELA
 	select NO_BOOTMEM
 	select OF_EARLY_FLATTREE if OF
 	select OF_RESERVED_MEM if OF
@@ -1841,6 +1844,7 @@ config XEN
 	help
 	  Say Y if you want to run Linux in a Virtual Machine on Xen on ARM.
 
+source "kernel/livepatch/Kconfig"
 endmenu
 
 menu "Boot options"
-- 
2.7.4

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

#1537432 — Re: [PATCH 7/7] arm: Add livepatch necessary arch selects into Kconfig

On Tue,  6 Dec 2016 17:06:07 +0000
Abel Vesa <abelvesa@linux.com> wrote:

> This adds HAVE_LIVEPATCH, MODULES_USE_ELF_RELA and HAVE_LIVEPATCH
> to arm Kconfig.
> 
> Signed-off-by: Abel Vesa <abelvesa@linux.com>

Patch 5, 6 and 7 really ought to be one patch.

-- Steve

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

#1537407

On 2016/12/7 1:06, Abel Vesa wrote:
> This is just an idea I've been trying out for a while now.
>
> Just in case somebody wants to play with it, this applies to linux-arm/for-next.
>
> Also please note that this was only tested in qemu, but I will do some testing
> on some real hardware in the following days.
>
> FWICT, on this arch the compiler always generates a function prologue somewhere
> between these lines:
>
> e1a0c00d        mov     ip, sp
> e92ddff0        push    {r4-r9, sl, fp, ip, lr, pc}
> e24cb004        sub     fp, ip, #4
> e24dd064        sub     sp, sp, #100    ; 0x64<--- local variables
> e52de004        push    {lr}            ; (str lr, [sp, #-4]!)
> ebf9c2c9        bl      80110364<__gnu_mcount_nc>
> ....
>
> Every function that follows this pattern (the number of registers pushed and the
> sp subtraction for the local variables being the only acceptable exception) can
> be patched with this mechanism. IIRC, only the inline functions and notrace
> functions do not follow this pattern.
>
> Considering that the function is livepatchable, when the time comes to call
> ftrace_call, the ftrace_regs_caller is called instead.
>
> Because this arch didn't have a ftrace with regs implementation, the
> ftrace_regs_caller was added.
>
> This new function adds the regs saving/restoring part, plus the part necessary
> for the livepatch mechanism to work. After the regs are saved and the r3 is set
> to contain the sp's value, we're keeping the old pc into r10 in order to be
> checked later against the new pc.
>
> Next, the r1 and r0 are set for the ftrace_func, then, the ftrace_stub is called
> and the klp_ftrace_handler overwrites the old pc with the new one.
>
> Here comes the tricky part. We're checking if the pc is still the old one, if it
> is we jump the whole livepatching and go ahead with restoring the saved regs.
>
> If the pc is modified, it means we're livepatching current function and we need
> to pop all regs from r1 through r12, jump over the next two regs saved on stack
> (we're not interested in those since we're trying to get the same regs context
> as it was at the point the function-to-be-patched was called) and put the new pc
> into r11.
>
> Since r12 contains the sp from when the function just got branched to, we need
> to set the sp back to that.
>
> Then we need to put the new pc on stack so that when we're popping r11 through
> pc, we will actually jump to the first instruction from the new function.
>
> We don't need to worry about the returning phase since the epilogue of the new
> function will take care of that and from there on everything goes back to
> normal.
>
> The whole advantage of this over adding compiler support is that we're not
> introducing nops at the beginning of the function. As a matter of fact, we're
> not changing anything between an image with livepatch and an image without it
> (except the ftrace_regs_call addition and the livepatch necessary code).
>
> As for the implementation of the ftrace_regs_caller, I still think there might
> be some unsafe stack handling since I'm getting some build warnings. Those are
> due to pushing/popping of a list of regs in which the sp resides. I'll try to
> get around those in a next iteration (if necessary), but first I would like to
> hear some opinions about this work and if it's worth going forward.
>

Hi, so your idea is that when the pc is modified, we undo the work of the prologue
of the old function, and then jump to the first instruction of the new function.
But I doubt if we can really undo the work of the prologue correctly ? I don't know
about arm, but gcc on arm64 may do some tricky things in prologue. So is there any
chance we may restore a wrong context for the new function ?

Thanks.

> Everything else should be pretty straightforward, so I'll skip explaining that.
>
> Abel Vesa (7):
>    arm: Add livepatch arch specific code
>    arm: ftrace: Add call modify mechanism
>    arm: module: Add apply_relocate_add
>    arm: Add ftrace with regs support
>    arm: ftrace: Add ARCH_SUPPORTS_FTRACE_OPS for ftrace with regs
>    arm: Add livepatch to build if CONFIG_LIVEPATCH
>    arm: Add livepatch necessary arch selects into Kconfig
>
>   MAINTAINERS                      |  3 +++
>   arch/arm/Kconfig                 |  4 ++++
>   arch/arm/include/asm/ftrace.h    |  4 ++++
>   arch/arm/include/asm/livepatch.h | 46 +++++++++++++++++++++++++++++++++++++
>   arch/arm/kernel/Makefile         |  1 +
>   arch/arm/kernel/entry-ftrace.S   | 49 ++++++++++++++++++++++++++++++++++++++++
>   arch/arm/kernel/ftrace.c         | 21 +++++++++++++++++
>   arch/arm/kernel/livepatch.c      | 43 +++++++++++++++++++++++++++++++++++
>   arch/arm/kernel/module.c         |  9 ++++++++
>   9 files changed, 180 insertions(+)
>   create mode 100644 arch/arm/include/asm/livepatch.h
>   create mode 100644 arch/arm/kernel/livepatch.c
>

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

#1537695

On Wed, Dec 07, 2016 at 09:38:07AM +0800, zhouchengming wrote:
> On 2016/12/7 1:06, Abel Vesa wrote:
> >This is just an idea I've been trying out for a while now.
> >
> >Just in case somebody wants to play with it, this applies to linux-arm/for-next.
> >
> >Also please note that this was only tested in qemu, but I will do some testing
> >on some real hardware in the following days.
> >
> >FWICT, on this arch the compiler always generates a function prologue somewhere
> >between these lines:
> >
> >e1a0c00d        mov     ip, sp
> >e92ddff0        push    {r4-r9, sl, fp, ip, lr, pc}
> >e24cb004        sub     fp, ip, #4
> >e24dd064        sub     sp, sp, #100    ; 0x64<--- local variables
> >e52de004        push    {lr}            ; (str lr, [sp, #-4]!)
> >ebf9c2c9        bl      80110364<__gnu_mcount_nc>
> >....
> >
> >Every function that follows this pattern (the number of registers pushed and the
> >sp subtraction for the local variables being the only acceptable exception) can
> >be patched with this mechanism. IIRC, only the inline functions and notrace
> >functions do not follow this pattern.
> >
> >Considering that the function is livepatchable, when the time comes to call
> >ftrace_call, the ftrace_regs_caller is called instead.
> >
> >Because this arch didn't have a ftrace with regs implementation, the
> >ftrace_regs_caller was added.
> >
> >This new function adds the regs saving/restoring part, plus the part necessary
> >for the livepatch mechanism to work. After the regs are saved and the r3 is set
> >to contain the sp's value, we're keeping the old pc into r10 in order to be
> >checked later against the new pc.
> >
> >Next, the r1 and r0 are set for the ftrace_func, then, the ftrace_stub is called
> >and the klp_ftrace_handler overwrites the old pc with the new one.
> >
> >Here comes the tricky part. We're checking if the pc is still the old one, if it
> >is we jump the whole livepatching and go ahead with restoring the saved regs.
> >
> >If the pc is modified, it means we're livepatching current function and we need
> >to pop all regs from r1 through r12, jump over the next two regs saved on stack
> >(we're not interested in those since we're trying to get the same regs context
> >as it was at the point the function-to-be-patched was called) and put the new pc
> >into r11.
> >
> >Since r12 contains the sp from when the function just got branched to, we need
> >to set the sp back to that.
> >
> >Then we need to put the new pc on stack so that when we're popping r11 through
> >pc, we will actually jump to the first instruction from the new function.
> >
> >We don't need to worry about the returning phase since the epilogue of the new
> >function will take care of that and from there on everything goes back to
> >normal.
> >
> >The whole advantage of this over adding compiler support is that we're not
> >introducing nops at the beginning of the function. As a matter of fact, we're
> >not changing anything between an image with livepatch and an image without it
> >(except the ftrace_regs_call addition and the livepatch necessary code).
> >
> >As for the implementation of the ftrace_regs_caller, I still think there might
> >be some unsafe stack handling since I'm getting some build warnings. Those are
> >due to pushing/popping of a list of regs in which the sp resides. I'll try to
> >get around those in a next iteration (if necessary), but first I would like to
> >hear some opinions about this work and if it's worth going forward.
> >
> 
> Hi, so your idea is that when the pc is modified, we undo the work of the prologue
> of the old function, and then jump to the first instruction of the new function.
> But I doubt if we can really undo the work of the prologue correctly ? I don't know
> about arm, but gcc on arm64 may do some tricky things in prologue. So is there any
> chance we may restore a wrong context for the new function ?
> 
> Thanks.
>
I forgot to mention that this is actually taking advantage of how this arch deals 
with function calling. This mechanism might not be appliable to any other arch 
AFAIK. On arm 32bit, as long as the mcount prologue looks like in the shown 
example, the function is livepatchable. I will come back with a new version of
this patch today (latest tomorrow) with comments as Russel and Steven mentioned.
I hope that will clarify why this is working on arm 32bit.
> >Everything else should be pretty straightforward, so I'll skip explaining that.
> >
> >Abel Vesa (7):
> >   arm: Add livepatch arch specific code
> >   arm: ftrace: Add call modify mechanism
> >   arm: module: Add apply_relocate_add
> >   arm: Add ftrace with regs support
> >   arm: ftrace: Add ARCH_SUPPORTS_FTRACE_OPS for ftrace with regs
> >   arm: Add livepatch to build if CONFIG_LIVEPATCH
> >   arm: Add livepatch necessary arch selects into Kconfig
> >
> >  MAINTAINERS                      |  3 +++
> >  arch/arm/Kconfig                 |  4 ++++
> >  arch/arm/include/asm/ftrace.h    |  4 ++++
> >  arch/arm/include/asm/livepatch.h | 46 +++++++++++++++++++++++++++++++++++++
> >  arch/arm/kernel/Makefile         |  1 +
> >  arch/arm/kernel/entry-ftrace.S   | 49 ++++++++++++++++++++++++++++++++++++++++
> >  arch/arm/kernel/ftrace.c         | 21 +++++++++++++++++
> >  arch/arm/kernel/livepatch.c      | 43 +++++++++++++++++++++++++++++++++++
> >  arch/arm/kernel/module.c         |  9 ++++++++
> >  9 files changed, 180 insertions(+)
> >  create mode 100644 arch/arm/include/asm/livepatch.h
> >  create mode 100644 arch/arm/kernel/livepatch.c
> >
> 
> 

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

#1537840

On Tue 2016-12-06 17:06:00, Abel Vesa wrote:
> This is just an idea I've been trying out for a while now. 
> 
> Just in case somebody wants to play with it, this applies to linux-arm/for-next.
> 
> Also please note that this was only tested in qemu, but I will do some testing 
> on some real hardware in the following days.
> 
> FWICT, on this arch the compiler always generates a function prologue somewhere
> between these lines:
> 
> e1a0c00d        mov     ip, sp
> e92ddff0        push    {r4-r9, sl, fp, ip, lr, pc}
> e24cb004        sub     fp, ip, #4
> e24dd064        sub     sp, sp, #100    ; 0x64 <--- local variables
> e52de004        push    {lr}            ; (str lr, [sp, #-4]!)
> ebf9c2c9        bl      80110364 <__gnu_mcount_nc>
> ....
> 
> Every function that follows this pattern (the number of registers pushed and the
> sp subtraction for the local variables being the only acceptable exception) can
> be patched with this mechanism. IIRC, only the inline functions and notrace 
> functions do not follow this pattern.

Please, where do you check that the given function follows this
pattern? I do not have experience with arm at all. But compiler
is able to do crazy optimizations these days.

I think that this was already mentioned somewhere. But please, put
this detailed explanation also to related patch/code so that it
can later be found in the git commits. It will also help to
better understand/review the particular patches.

Best Regards,
Petr

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web