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

Groups > linux.kernel > #1331367 > unrolled thread

[PATCH v8 8/8] livepatch: Detect offset for the ftrace location during build

Started byTorsten Duwe <duwe@lst.de>
First post2016-02-10 18:50 +0100
Last post2016-02-16 11:40 +0100
Articles 10 — 5 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v8 8/8] livepatch: Detect offset for the ftrace location  during build Torsten Duwe <duwe@lst.de> - 2016-02-10 18:50 +0100
    Re: [PATCH v8 8/8] livepatch: Detect offset for the ftrace location  during build Balbir Singh <bsingharora@gmail.com> - 2016-02-12 17:20 +0100
      Re: [PATCH v8 8/8] livepatch: Detect offset for the ftrace location  during build Petr Mladek <pmladek@suse.com> - 2016-02-12 17:50 +0100
        Re: [PATCH v8 8/8] livepatch: Detect offset for the ftrace location  during build Balbir Singh <bsingharora@gmail.com> - 2016-02-13 02:40 +0100
        Re: [PATCH v8 8/8] livepatch: Detect offset for the ftrace location  during build Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> - 2016-02-16 06:50 +0100
          Re: [PATCH v8 8/8] livepatch: Detect offset for the ftrace  location during build Torsten Duwe <duwe@lst.de> - 2016-02-16 09:30 +0100
            Re: [PATCH v8 8/8] livepatch: Detect offset for the ftrace  location during build Torsten Duwe <duwe@lst.de> - 2016-02-16 11:40 +0100
              Re: [PATCH v8 8/8] livepatch: Detect offset for the ftrace location  during build Petr Mladek <pmladek@suse.com> - 2016-02-16 15:00 +0100
                Re: [PATCH v8 8/8] livepatch: Detect offset for the ftrace location  during build Michael Ellerman <mpe@ellerman.id.au> - 2016-02-17 04:10 +0100
            Re: [PATCH v8 8/8] livepatch: Detect offset for the ftrace location  during build Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> - 2016-02-16 11:40 +0100

#1331367 — [PATCH v8 8/8] livepatch: Detect offset for the ftrace location during build

FromTorsten Duwe <duwe@lst.de>
Date2016-02-10 18:50 +0100
Subject[PATCH v8 8/8] livepatch: Detect offset for the ftrace location during build
Message-ID<r0H3P-5r5-1@gated-at.bofh.it>
From: Petr Mladek <pmladek@suse.com>

Livepatch works on x86_64 and s390 only when the ftrace call
is at the very beginning of the function. But PPC is different.
We need to handle TOC and save LR there before calling the
global ftrace handler.

Now, the problem is that the extra operations have different
length on PPC depending on the used gcc version. It is
4 instructions (16 bytes) before gcc-6 and only 3 instructions
(12 bytes) with gcc-6.

This patch tries to detect the offset a generic way during
build. It assumes that the offset of the ftrace location
is the same for all functions. It modifies the existing
recordmcount tool that is able to find read mcount locations
directly from the object files. It adds an option -p
to print the first found offset.

The recordmcount tool is then used in the kernel/livepatch
subdirectory to generate a header file. It defines
a constant that is used to compute the ftrace location
from the function address.

Finally, we have to enable the C implementation of the
recordmcount tool to be used on PPC and S390. It seems
to work fine there. It should be more reliable because
it reads the standardized elf structures. The old perl
implementation uses rather complex regular expressions
to parse objdump output and is therefore much more tricky.

Signed-off-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Torsten Duwe <duwe@suse.de>
---
 arch/powerpc/Kconfig           |  1 +
 arch/s390/Kconfig              |  1 +
 kernel/livepatch/Makefile      | 13 +++++++++++++
 kernel/livepatch/core.c        | 12 +++++++++---
 kernel/livepatch/ftrace-test.c |  6 ++++++
 scripts/recordmcount.c         |  6 +++++-
 scripts/recordmcount.h         | 17 +++++++++++++++--
 7 files changed, 50 insertions(+), 6 deletions(-)
 create mode 100644 kernel/livepatch/ftrace-test.c

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 8c7a327..a546829 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -93,6 +93,7 @@ config PPC
 	select OF_EARLY_FLATTREE
 	select OF_RESERVED_MEM
 	select HAVE_FTRACE_MCOUNT_RECORD
+	select HAVE_C_RECORDMCOUNT
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS if PPC64 && CPU_LITTLE_ENDIAN
 	select HAVE_FUNCTION_TRACER
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 3be9c83..c574bc4 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -121,6 +121,7 @@ config S390
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_ARCH_TRANSPARENT_HUGEPAGE
 	select HAVE_BPF_JIT if PACK_STACK && HAVE_MARCH_Z196_FEATURES
+	select HAVE_C_RECORDMCOUNT
 	select HAVE_CMPXCHG_DOUBLE
 	select HAVE_CMPXCHG_LOCAL
 	select HAVE_DEBUG_KMEMLEAK
diff --git a/kernel/livepatch/Makefile b/kernel/livepatch/Makefile
index e8780c0..65a44b6 100644
--- a/kernel/livepatch/Makefile
+++ b/kernel/livepatch/Makefile
@@ -1,3 +1,16 @@
 obj-$(CONFIG_LIVEPATCH) += livepatch.o
 
 livepatch-objs := core.o
+
+always		:= $(hostprogs-y) ftrace-test.o
+
+# dependencies on generated files need to be listed explicitly
+$(obj)/core.o: $(obj)/livepatch-ftrace.h
+
+quiet_cmd_livepatch-rmcount = RMCOUNT $@
+      cmd_livepatch-rmcount = $(objtree)/scripts/recordmcount -p $< > $@
+
+$(obj)/livepatch-ftrace.h: $(obj)/ftrace-test.o $(objtree)/scripts/recordmcount
+	$(call if_changed,livepatch-rmcount)
+
+targets += livepatch-ftrace.h
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index bc2c85c..864d589 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -30,6 +30,8 @@
 #include <linux/livepatch.h>
 #include <asm/cacheflush.h>
 
+#include "livepatch-ftrace.h"
+
 /**
  * struct klp_ops - structure for tracking registered ftrace ops structs
  *
@@ -312,8 +314,10 @@ static void klp_disable_func(struct klp_func *func)
 		return;
 
 	if (list_is_singular(&ops->func_stack)) {
+		unsigned long ftrace_loc = func->old_addr + KLP_FTRACE_LOCATION;
+
 		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 +342,8 @@ static int klp_enable_func(struct klp_func *func)
 
 	ops = klp_find_ops(func->old_addr);
 	if (!ops) {
+		unsigned long ftrace_loc = func->old_addr + KLP_FTRACE_LOCATION;
+
 		ops = kzalloc(sizeof(*ops), GFP_KERNEL);
 		if (!ops)
 			return -ENOMEM;
@@ -352,7 +358,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 +369,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;
 		}
 
diff --git a/kernel/livepatch/ftrace-test.c b/kernel/livepatch/ftrace-test.c
new file mode 100644
index 0000000..22f0c54
--- /dev/null
+++ b/kernel/livepatch/ftrace-test.c
@@ -0,0 +1,6 @@
+/* Sample code to figure out mcount location offset */
+
+int test(int a)
+{
+	return ++a;
+}
diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index e167592..e351b2f 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -53,6 +53,7 @@ static struct stat sb;	/* Remember .st_size, etc. */
 static jmp_buf jmpenv;	/* setjmp/longjmp per-file error escape */
 static const char *altmcount;	/* alternate mcount symbol name */
 static int warn_on_notrace_sect; /* warn when section has mcount not being recorded */
+static int print_mcount_offset; /* print offset of the first mcount location */
 static void *file_map;	/* pointer of the mapped file */
 static void *file_end;	/* pointer to the end of the mapped file */
 static int file_updated; /* flag to state file was changed */
@@ -539,11 +540,14 @@ main(int argc, char *argv[])
 	int c;
 	int i;
 
-	while ((c = getopt(argc, argv, "w")) >= 0) {
+	while ((c = getopt(argc, argv, "wp")) >= 0) {
 		switch (c) {
 		case 'w':
 			warn_on_notrace_sect = 1;
 			break;
+		case 'p':
+			print_mcount_offset = 1;
+			break;
 		default:
 			fprintf(stderr, "usage: recordmcount [-w] file.o...\n");
 			return 0;
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index b9897e2..a677a5a 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -47,6 +47,7 @@
 #undef fn_ELF_R_SYM
 #undef fn_ELF_R_INFO
 #undef uint_t
+#undef uint_t_format
 #undef _w
 #undef _align
 #undef _size
@@ -81,6 +82,7 @@
 # define fn_ELF_R_SYM		fn_ELF64_R_SYM
 # define fn_ELF_R_INFO		fn_ELF64_R_INFO
 # define uint_t			uint64_t
+# define uint_t_format		"%lu"
 # define _w			w8
 # define _align			7u
 # define _size			8
@@ -114,6 +116,7 @@
 # define fn_ELF_R_SYM		fn_ELF32_R_SYM
 # define fn_ELF_R_INFO		fn_ELF32_R_INFO
 # define uint_t			uint32_t
+# define uint_t_format		"%u"
 # define _w			w
 # define _align			3u
 # define _size			4
@@ -338,7 +341,14 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
 			} else
 				*mlocp++ = addend;
 
+			if (print_mcount_offset) {
+				printf("#define KLP_FTRACE_LOCATION " uint_t_format "\n",
+				       addend);
+				succeed_file();
+			}
+
 			mrelp = (Elf_Rel *)(rel_entsize + (void *)mrelp);
+
 		}
 		relp = (Elf_Rel const *)(rel_entsize + (void *)relp);
 	}
@@ -458,7 +468,8 @@ __has_rel_mcount(Elf_Shdr const *const relhdr,  /* is SHT_REL or SHT_RELA */
 	Elf_Shdr const *const txthdr = &shdr0[w(relhdr->sh_info)];
 	char const *const txtname = &shstrtab[w(txthdr->sh_name)];
 
-	if (strcmp("__mcount_loc", txtname) == 0) {
+	/* Allow to print the mcount offset for an already modified file. */
+	if (strcmp("__mcount_loc", txtname) == 0 && !print_mcount_offset) {
 		fprintf(stderr, "warning: __mcount_loc already exists: %s\n",
 			fname);
 		succeed_file();
@@ -546,7 +557,9 @@ do_func(Elf_Ehdr *const ehdr, char const *const fname, unsigned const reltype)
 			nop_mcount(relhdr, ehdr, txtname);
 		}
 	}
-	if (mloc0 != mlocp) {
+
+	/* The file is not modified when the offset is just printed. */
+	if (mloc0 != mlocp && !print_mcount_offset) {
 		append_func(ehdr, shstr, mloc0, mlocp, mrel0, mrelp,
 			    rel_entsize, symsec_sh_link);
 	}
-- 
1.8.5.6

[toc] | [next] | [standalone]

#1332852

FromBalbir Singh <bsingharora@gmail.com>
Date2016-02-12 17:20 +0100
Message-ID<r1oBQ-SL-9@gated-at.bofh.it>
In reply to#1331367
On Thu, 2016-01-28 at 16:32 +0100, Torsten Duwe wrote:
> From: Petr Mladek <pmladek@suse.com>
> 
> Livepatch works on x86_64 and s390 only when the ftrace call
> is at the very beginning of the function. But PPC is different.
> We need to handle TOC and save LR there before calling the
> global ftrace handler.
> 
> Now, the problem is that the extra operations have different
> length on PPC depending on the used gcc version. It is
> 4 instructions (16 bytes) before gcc-6 and only 3 instructions
> (12 bytes) with gcc-6.
> 
> This patch tries to detect the offset a generic way during
> build. It assumes that the offset of the ftrace location
> is the same for all functions. It modifies the existing
> recordmcount tool that is able to find read mcount locations
> directly from the object files. It adds an option -p
> to print the first found offset.
> 
> The recordmcount tool is then used in the kernel/livepatch
> subdirectory to generate a header file. It defines
> a constant that is used to compute the ftrace location
> from the function address.
> 
> Finally, we have to enable the C implementation of the
> recordmcount tool to be used on PPC and S390. It seems
> to work fine there. It should be more reliable because
> it reads the standardized elf structures. The old perl
> implementation uses rather complex regular expressions
> to parse objdump output and is therefore much more tricky.

I'm still missing something, I'm getting offset as 8

When I run, I get

scripts/recordmcount -p kernel/livepatch/core.o 
#define KLP_FTRACE_LOCATION 8

scripts/recordmcount -p kernel/livepatch/ftrace-test.o 
#define KLP_FTRACE_LOCATION 8

My sample fails as well, since the expected offset is 16.
I guess the script is being run against a not so good
test.

A quick hack (no signoff below, its just an experiment),
seems to do the trick for the provided sample-livepatch.
It is hacky because it uses the sample object and due to
lack of a better description of srctree, it uses 
srctree/../..

I suspect the usage of recordmcount needs to be revisited

diff --git a/kernel/livepatch/Makefile b/kernel/livepatch/Makefile
index 65a44b68..10b5f38 100644
--- a/kernel/livepatch/Makefile
+++ b/kernel/livepatch/Makefile
@@ -2,7 +2,7 @@ obj-$(CONFIG_LIVEPATCH) += livepatch.o
 
 livepatch-objs := core.o
 
-always		:= $(hostprogs-y) ftrace-test.o
+always		:= $(hostprogs-y) $(srctree)/../../samples/livepatch/livepatch-sample.o
 
 # dependencies on generated files need to be listed explicitly
 $(obj)/core.o: $(obj)/livepatch-ftrace.h
@@ -10,7 +10,7 @@ $(obj)/core.o: $(obj)/livepatch-ftrace.h
 quiet_cmd_livepatch-rmcount = RMCOUNT $@
       cmd_livepatch-rmcount = $(objtree)/scripts/recordmcount -p $< > $@
 
-$(obj)/livepatch-ftrace.h: $(obj)/ftrace-test.o $(objtree)/scripts/recordmcount
+$(obj)/livepatch-ftrace.h: $(obj)/../../samples/livepatch/livepatch-sample.o $(objtree)/scripts/recordmcount
 	$(call if_changed,livepatch-rmcount)
 
 targets += livepatch-ftrace.h

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

#1332890

FromPetr Mladek <pmladek@suse.com>
Date2016-02-12 17:50 +0100
Message-ID<r1p4R-14o-15@gated-at.bofh.it>
In reply to#1332852
On Sat 2016-02-13 03:13:29, Balbir Singh wrote:
> On Thu, 2016-01-28 at 16:32 +0100, Torsten Duwe wrote:
> > From: Petr Mladek <pmladek@suse.com>
> > 
> > Livepatch works on x86_64 and s390 only when the ftrace call
> > is at the very beginning of the function. But PPC is different.
> > We need to handle TOC and save LR there before calling the
> > global ftrace handler.
> > 
> > Now, the problem is that the extra operations have different
> > length on PPC depending on the used gcc version. It is
> > 4 instructions (16 bytes) before gcc-6 and only 3 instructions
> > (12 bytes) with gcc-6.
> > 
> > This patch tries to detect the offset a generic way during
> > build. It assumes that the offset of the ftrace location
> > is the same for all functions. It modifies the existing
> > recordmcount tool that is able to find read mcount locations
> > directly from the object files. It adds an option -p
> > to print the first found offset.
> > 
> > The recordmcount tool is then used in the kernel/livepatch
> > subdirectory to generate a header file. It defines
> > a constant that is used to compute the ftrace location
> > from the function address.
> > 
> > Finally, we have to enable the C implementation of the
> > recordmcount tool to be used on PPC and S390. It seems
> > to work fine there. It should be more reliable because
> > it reads the standardized elf structures. The old perl
> > implementation uses rather complex regular expressions
> > to parse objdump output and is therefore much more tricky.
> 
> I'm still missing something, I'm getting offset as 8
>
> When I run, I get
> 
> scripts/recordmcount -p kernel/livepatch/core.o 
> #define KLP_FTRACE_LOCATION 8
> 
> scripts/recordmcount -p kernel/livepatch/ftrace-test.o 
> #define KLP_FTRACE_LOCATION 8
> 
> My sample fails as well, since the expected offset is 16.
> I guess the script is being run against a not so good
> test.

I guess that you used a broken gcc and cheated the check
to pass the compilation. Did you, please?

The test used to detect the offset is using a minimalistic
function is is afftected by the gcc bug.

The patch below might be used to cheat the offset check as well.

Torsten, please mention this somewhere if you, just by chance,
send a new version of the patchset.

From f6a438a3f2f60cc1acc859b41d0cc9259c9a331e Mon Sep 17 00:00:00 2001
From: root <root@c79.arch.suse.de>
Date: Tue, 2 Feb 2016 15:35:06 +0100
Subject: [PATCH 2/2] livepatch: Make sure the TOC is handled when detecting
 ftrace location

There seems to be a bug in gcc on PPC. It does not handle TOC
if the function does not access global variables or functions
by default. But it should when profiling is enabled.

This patch works around this problem by adding a call
to a global function.

This patch is for testing only!
---
 kernel/livepatch/ftrace-test.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/livepatch/ftrace-test.c b/kernel/livepatch/ftrace-test.c
index 22f0c54bf7b3..a3b7aabb67e5 100644
--- a/kernel/livepatch/ftrace-test.c
+++ b/kernel/livepatch/ftrace-test.c
@@ -1,6 +1,9 @@
 /* Sample code to figure out mcount location offset */
+#include <linux/printk.h>
+
 
 int test(int a)
 {
+	printk("%d\n", a);
 	return ++a;
 }
-- 
1.8.5.6

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

#1333246

FromBalbir Singh <bsingharora@gmail.com>
Date2016-02-13 02:40 +0100
Message-ID<r1xlL-6xx-3@gated-at.bofh.it>
In reply to#1332890
On Fri, 2016-02-12 at 17:45 +0100, Petr Mladek wrote:
> On Sat 2016-02-13 03:13:29, Balbir Singh wrote:
> > On Thu, 2016-01-28 at 16:32 +0100, Torsten Duwe wrote:
> > > From: Petr Mladek <pmladek@suse.com>
> > > 
> > > Livepatch works on x86_64 and s390 only when the ftrace call
> > > is at the very beginning of the function. But PPC is different.
> > > We need to handle TOC and save LR there before calling the
> > > global ftrace handler.
> > > 
> > > Now, the problem is that the extra operations have different
> > > length on PPC depending on the used gcc version. It is
> > > 4 instructions (16 bytes) before gcc-6 and only 3 instructions
> > > (12 bytes) with gcc-6.
> > > 
> > > This patch tries to detect the offset a generic way during
> > > build. It assumes that the offset of the ftrace location
> > > is the same for all functions. It modifies the existing
> > > recordmcount tool that is able to find read mcount locations
> > > directly from the object files. It adds an option -p
> > > to print the first found offset.
> > > 
> > > The recordmcount tool is then used in the kernel/livepatch
> > > subdirectory to generate a header file. It defines
> > > a constant that is used to compute the ftrace location
> > > from the function address.
> > > 
> > > Finally, we have to enable the C implementation of the
> > > recordmcount tool to be used on PPC and S390. It seems
> > > to work fine there. It should be more reliable because
> > > it reads the standardized elf structures. The old perl
> > > implementation uses rather complex regular expressions
> > > to parse objdump output and is therefore much more tricky.
> > 
> > I'm still missing something, I'm getting offset as 8
> > 
> > When I run, I get
> > 
> > scripts/recordmcount -p kernel/livepatch/core.o 
> > #define KLP_FTRACE_LOCATION 8
> > 
> > scripts/recordmcount -p kernel/livepatch/ftrace-test.o 
> > #define KLP_FTRACE_LOCATION 8
> > 
> > My sample fails as well, since the expected offset is 16.
> > I guess the script is being run against a not so good
> > test.
> 
> I guess that you used a broken gcc and cheated the check
> to pass the compilation. Did you, please?
> 
> The test used to detect the offset is using a minimalistic
> function is is afftected by the gcc bug.
> 
> The patch below might be used to cheat the offset check as well.
> 
> Torsten, please mention this somewhere if you, just by chance,
> send a new version of the patchset.
> 
> From f6a438a3f2f60cc1acc859b41d0cc9259c9a331e Mon Sep 17 00:00:00 2001
> From: root <root@c79.arch.suse.de>
> Date: Tue, 2 Feb 2016 15:35:06 +0100
> Subject: [PATCH 2/2] livepatch: Make sure the TOC is handled when detecting
>  ftrace location
> 
> There seems to be a bug in gcc on PPC. It does not handle TOC
> if the function does not access global variables or functions
> by default. But it should when profiling is enabled.
> 

Yep.. Please see see http://marc.info/?l=linux-kernel&m=145518015816435&w=2
and my question at http://marc.info/?l=linuxppc-embedded&m=145518330317496&w=2

> This patch works around this problem by adding a call
> to a global function.
> 
> This patch is for testing only!
> ---
>  kernel/livepatch/ftrace-test.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/livepatch/ftrace-test.c b/kernel/livepatch/ftrace-test.c
> index 22f0c54bf7b3..a3b7aabb67e5 100644
> --- a/kernel/livepatch/ftrace-test.c
> +++ b/kernel/livepatch/ftrace-test.c
> @@ -1,6 +1,9 @@
>  /* Sample code to figure out mcount location offset */
> +#include 
> +
>  
>  int test(int a)
>  {
> +	printk("%d\n", a);
>  	return ++a;
>  }

This is much better, I see the offset of 16.

Balbir Singh

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

#1335013

FromKamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Date2016-02-16 06:50 +0100
Message-ID<r2GGn-3pA-25@gated-at.bofh.it>
In reply to#1332890
* Petr Mladek <pmladek@suse.com> [2016-02-12 17:45:17]:

[...]
> I guess that you used a broken gcc and cheated the check
> to pass the compilation. Did you, please?
> 
> The test used to detect the offset is using a minimalistic
> function is is afftected by the gcc bug.
> 
> The patch below might be used to cheat the offset check as well.
> 
> Torsten, please mention this somewhere if you, just by chance,
> send a new version of the patchset.
> 
> From f6a438a3f2f60cc1acc859b41d0cc9259c9a331e Mon Sep 17 00:00:00 2001
> From: root <root@c79.arch.suse.de>
> Date: Tue, 2 Feb 2016 15:35:06 +0100
> Subject: [PATCH 2/2] livepatch: Make sure the TOC is handled when detecting
>  ftrace location
> 
> There seems to be a bug in gcc on PPC. It does not handle TOC
> if the function does not access global variables or functions
> by default. But it should when profiling is enabled.
> 
> This patch works around this problem by adding a call
> to a global function.
> 
> This patch is for testing only!
> ---
>  kernel/livepatch/ftrace-test.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/livepatch/ftrace-test.c b/kernel/livepatch/ftrace-test.c
> index 22f0c54bf7b3..a3b7aabb67e5 100644
> --- a/kernel/livepatch/ftrace-test.c
> +++ b/kernel/livepatch/ftrace-test.c
> @@ -1,6 +1,9 @@
>  /* Sample code to figure out mcount location offset */
> +#include <linux/printk.h>
> +
>  
>  int test(int a)
>  {
> +	printk("%d\n", a);
>  	return ++a;
>  }

Thanks. This workaround, helped to load sample livepatch module.

Thanks,
Kamalesh.

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

#1335134 — Re: [PATCH v8 8/8] livepatch: Detect offset for the ftrace location during build

FromTorsten Duwe <duwe@lst.de>
Date2016-02-16 09:30 +0100
SubjectRe: [PATCH v8 8/8] livepatch: Detect offset for the ftrace location during build
Message-ID<r2Jbc-5ii-17@gated-at.bofh.it>
In reply to#1335013
On Tue, Feb 16, 2016 at 11:17:02AM +0530, Kamalesh Babulal wrote:
> * Petr Mladek <pmladek@suse.com> [2016-02-12 17:45:17]:
> >  int test(int a)
> >  {
> > +	printk("%d\n", a);
> >  	return ++a;
> >  }
> 
> Thanks. This workaround, helped to load sample livepatch module.

N.b.: if you try to livepatch/trace such a leaf function without
global dependencies, it will crash if that function got called with
a different TOC value. Hence this whole testing.

You may alternatively try my gcc patch ;-)

Another caveat is functions with stack arguments (>8 args, varargs).
My code needs special precautions then because of the return helper.

	Torsten

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

#1335261 — Re: [PATCH v8 8/8] livepatch: Detect offset for the ftrace location during build

FromTorsten Duwe <duwe@lst.de>
Date2016-02-16 11:40 +0100
SubjectRe: [PATCH v8 8/8] livepatch: Detect offset for the ftrace location during build
Message-ID<r2Ld0-6Dh-11@gated-at.bofh.it>
In reply to#1335134
On Tue, Feb 16, 2016 at 04:00:30PM +0530, Kamalesh Babulal wrote:
> * Torsten Duwe <duwe@lst.de> [2016-02-16 09:23:02]:
> > 
> > N.b.: if you try to livepatch/trace such a leaf function without
> > global dependencies, it will crash if that function got called with
> > a different TOC value. Hence this whole testing.
> > 
> 
> I am running out of ideas on how to generate this crash, any pointers
> will be helpful.

Petr discovered some modular SCSI helper function does it.

HTH,
	Torsten

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

#1335414

FromPetr Mladek <pmladek@suse.com>
Date2016-02-16 15:00 +0100
Message-ID<r2OkB-8F-57@gated-at.bofh.it>
In reply to#1335261
On Tue 2016-02-16 11:39:07, Torsten Duwe wrote:
> On Tue, Feb 16, 2016 at 04:00:30PM +0530, Kamalesh Babulal wrote:
> > * Torsten Duwe <duwe@lst.de> [2016-02-16 09:23:02]:
> > > 
> > > N.b.: if you try to livepatch/trace such a leaf function without
> > > global dependencies, it will crash if that function got called with
> > > a different TOC value. Hence this whole testing.
> > > 
> > 
> > I am running out of ideas on how to generate this crash, any pointers
> > will be helpful.
> 
> Petr discovered some modular SCSI helper function does it.

The kernel crashes here when I build it _with_ modules and
enable a tracer.

I see this:

$> lsmod
Module                  Size  Used by
af_packet              46767  0 
dm_mod                148307  0 
e1000                 184154  0 
rtc_generic             2617  0 
ext4                  716333  1 
crc16                   2307  1 ext4
mbcache                14237  1 ext4
jbd2                  143496  1 ext4
sd_mod                 49002  3 
sr_mod                 22532  0 
cdrom                  65024  1 sr_mod
ibmvscsi               34139  2 
scsi_transport_srp     18979  1 ibmvscsi
sg                     44568  0 
scsi_mod              289641  5 sg,scsi_transport_srp,ibmvscsi,sd_mod,sr_mod
autofs4                48256  0 

echo function >/sys/kernel/debug/tracing/current_tracer

BANG!


Some dugging has shown an Oops in the fucntion int_to_scsilun()
called from ibmvscsi_queuecommand(). So, I rebooted and
did the following test:

$> echo ibmvscsi_queuecommand >/sys/kernel/debug/tracing/set_ftrace_filter
$> echo function > /sys/kernel/debug/tracing/current_tracer 
$> echo 1 > /sys/kernel/debug/tracing/tracing_on
$> cat /sys/kernel/debug/tracing/trace
# tracer: function
#
# entries-in-buffer/entries-written: 7/7   #P:4
#
#                              _-----=> irqs-off
#                             / _----=> need-resched
#                            | / _---=> hardirq/softirq
#                            || / _--=> preempt-depth
#                            ||| /     delay
#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
#              | |       |   ||||       |         |
            bash-3488  [000] ....   100.278622: ibmvscsi_queuecommand <-scsi_dispatch_cmd
     kworker/1:2-223   [001] ....   101.048569: ibmvscsi_queuecommand <-scsi_dispatch_cmd
     kworker/1:2-223   [001] ....   103.048575: ibmvscsi_queuecommand <-scsi_dispatch_cmd
     jbd2/sda3-8-1021  [003] ....   104.008645: ibmvscsi_queuecommand <-scsi_dispatch_cmd
     jbd2/sda3-8-1021  [003] ....   104.008883: ibmvscsi_queuecommand <-scsi_dispatch_cmd
          <idle>-0     [000] ..s.   104.017672: ibmvscsi_queuecommand <-scsi_dispatch_cmd
          <idle>-0     [003] ..s.   104.017771: ibmvscsi_queuecommand <-scsi_dispatch_cmd

It means that ibmvscsi_queuecommand can be traced. Then I did

c79:/sys/kernel/debug/tracing # echo int_to_scsilun >set_ftrace_filter

BANG!

Unable to handle kernel paging request for data at address 0xd00000000108b148
Faulting instruction address: 0xd000000000bde35c
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: af_packet(E) dm_mod(E) e1000(E) rtc_generic(E) ext4(E) crc16(E) mbcache(E) jbd2(E) sr_mod(E) cdrom(E) sd_mod(E) ibmvscsi(E) scsi_transport_srp(E) sg(E) scsi_mod(E) autofs4(E)
CPU: 1 PID: 223 Comm: kworker/1:2 Tainted: G            E   4.5.0-rc2-11-default+ #90
Workqueue: events_freezable_power_ disk_events_workfn
task: c0000000f7d99aa0 ti: c0000000f7cb8000 task.ti: c0000000f7cb8000
NIP: d000000000bde35c LR: d000000000bcffec CTR: d000000000bcffe0
REGS: c0000000f7cbb3b0 TRAP: 0300   Tainted: G            E    (4.5.0-rc2-11-default+)
MSR: 8000000100009033 <SF,EE,ME,IR,DR,RI,LE,TM[E]>  CR: 24c82220  XER: 00000000
CFAR: d000000000bdd144 DAR: d00000000108b148 DSISR: 40000000 SOFTE: 0 
GPR00: d0000000010a21cc c0000000f7cbb630 d0000000010aeda0 0000000000008200 
GPR04: c0000000fa0b33fc 000000000000014a c0000000fa0b3408 0000000000000010 
GPR08: 0000000000000008 c00000000370a4e0 0000000000000000 d00000000108b128 
GPR12: d000000000bcffe0 c000000007e80300 c0000000000d8b18 c0000000fe04ba80 
GPR16: 0000000000000000 c0000000f7b6f208 0000000000000000 0000000000010000 
GPR20: c0000000f7b6f144 c0000000f7b6f140 d000000000bcbcf0 0000000000000000 
GPR24: 0000000000000200 0000000000000000 0000000000000001 c0000000f7b6f810 
GPR28: c0000000f7b6f000 c0000000f7b6f000 c0000000fa0b33a0 c0000000fe837e00 
NIP [d000000000bde35c] scsi_inq_str+0x21b0/0x41ac [scsi_mod]
LR [d000000000bcffec] int_to_scsilun+0xc/0x60 [scsi_mod]
Call Trace:
[c0000000f7cbb630] [d0000000010a21cc] ibmvscsi_queuecommand+0x10c/0x4e0 [ibmvscsi] (unreliable)
[c0000000f7cbb6e0] [d000000000bcbea8] scsi_dispatch_cmd+0xe8/0x2c0 [scsi_mod]
[c0000000f7cbb760] [d000000000bceb0c] scsi_request_fn+0x50c/0x8b0 [scsi_mod]
[c0000000f7cbb850] [c000000000407280] __blk_run_queue+0x60/0x90
[c0000000f7cbb880] [c000000000413640] blk_execute_rq_nowait+0x100/0x1a0
[c0000000f7cbb8d0] [c000000000413768] blk_execute_rq+0x88/0x170
[c0000000f7cbb9b0] [d000000000bca048] scsi_execute+0x108/0x1d0 [scsi_mod]
[c0000000f7cbba20] [d000000000bca2e8] scsi_execute_req_flags+0xc8/0x150 [scsi_mod]
[c0000000f7cbbae0] [d0000000012209e4] sr_check_events+0xb4/0x340 [sr_mod]
[c0000000f7cbbb90] [d0000000011c00b4] cdrom_check_events+0x44/0x80 [cdrom]
[c0000000f7cbbbc0] [d000000001220fa4] sr_block_check_events+0x44/0x60 [sr_mod]
[c0000000f7cbbbe0] [c0000000004222f8] disk_check_events+0x78/0x1b0
[c0000000f7cbbc50] [c0000000000d0610] process_one_work+0x1a0/0x480
[c0000000f7cbbce0] [c0000000000d0998] worker_thread+0xa8/0x5c0
[c0000000f7cbbd80] [c0000000000d8c24] kthread+0x114/0x140
[c0000000f7cbbe30] [c000000000009538] ret_from_kernel_thread+0x5c/0xa4
Instruction dump:
396bc360 f8410018 e98b0020 7d8903a6 4e800420 00000000 00000000 0044fb30 
c0000000 3d62fffe 396bc388 60000000 <e98b0020> 7d8903a6 4e800420 00000000 
---[ end trace 3b830c669dd7adb5 ]---


Note that ibmvscsi_queuecommand() handle TOC and int_to_scsilun()
does not handle TOC

$> objdump -hdr  drivers/scsi/ibmvscsi/ibmvscsi.ko
00000000000020c0 <ibmvscsi_queuecommand>:
    20c0:       00 00 4c 3c     addis   r2,r12,0
                        20c0: R_PPC64_REL16_HA  .TOC.
    20c4:       00 00 42 38     addi    r2,r2,0
                        20c4: R_PPC64_REL16_LO  .TOC.+0x4
    20c8:       a6 02 08 7c     mflr    r0
    20cc:       10 00 01 f8     std     r0,16(r1)
    20d0:       01 00 00 48     bl      20d0 <ibmvscsi_queuecommand+0x10>
                        20d0: R_PPC64_REL24     _mcount
    20d4:       a6 02 08 7c     mflr    r0


$> objdump -hdr  drivers/scsi/scsi_mod.ko
[...]
000000000000ffe0 <int_to_scsilun>:
    ffe0:       a6 02 08 7c     mflr    r0
    ffe4:       10 00 01 f8     std     r0,16(r1)
    ffe8:       01 00 00 48     bl      ffe8 <int_to_scsilun+0x8>
                        ffe8: R_PPC64_REL24     _mcount
    ffec:       a6 02 08 7c     mflr    r0
    fff0:       10 00 01 f8     std     r0,16(r1)
    fff4:       e1 ff 21 f8     stdu    r1,-32(r1)


I am able to trace int_to_scsilun() if I add a global call
into this fucntion.

I could trace any function when everything is built into the kernel.


I am not sure where the problem is but even normal tracing does not
work with this patchset and when -pg -mprofile-kernel is used.


Best Regards,
Petr

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

#1336024

FromMichael Ellerman <mpe@ellerman.id.au>
Date2016-02-17 04:10 +0100
Message-ID<r30F4-d2-1@gated-at.bofh.it>
In reply to#1335414
On Tue, 2016-02-16 at 14:57 +0100, Petr Mladek wrote:
> 
> Some dugging has shown an Oops in the fucntion int_to_scsilun()
> called from ibmvscsi_queuecommand(). So, I rebooted and
> did the following test:
> 
> $> echo ibmvscsi_queuecommand >/sys/kernel/debug/tracing/set_ftrace_filter
> $> echo function > /sys/kernel/debug/tracing/current_tracer 
> $> echo 1 > /sys/kernel/debug/tracing/tracing_on
> $> cat /sys/kernel/debug/tracing/trace
> # tracer: function
> #
> # entries-in-buffer/entries-written: 7/7   #P:4
> #
> #                              _-----=> irqs-off
> #                             / _----=> need-resched
> #                            | / _---=> hardirq/softirq
> #                            || / _--=> preempt-depth
> #                            ||| /     delay
> #           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
> #              | |       |   ||||       |         |
>             bash-3488  [000] ....   100.278622: ibmvscsi_queuecommand <-scsi_dispatch_cmd
>      kworker/1:2-223   [001] ....   101.048569: ibmvscsi_queuecommand <-scsi_dispatch_cmd
>      kworker/1:2-223   [001] ....   103.048575: ibmvscsi_queuecommand <-scsi_dispatch_cmd
>      jbd2/sda3-8-1021  [003] ....   104.008645: ibmvscsi_queuecommand <-scsi_dispatch_cmd
>      jbd2/sda3-8-1021  [003] ....   104.008883: ibmvscsi_queuecommand <-scsi_dispatch_cmd
>           <idle>-0     [000] ..s.   104.017672: ibmvscsi_queuecommand <-scsi_dispatch_cmd
>           <idle>-0     [003] ..s.   104.017771: ibmvscsi_queuecommand <-scsi_dispatch_cmd
> 
> It means that ibmvscsi_queuecommand can be traced. Then I did
> 
> c79:/sys/kernel/debug/tracing # echo int_to_scsilun >set_ftrace_filter
> 
> BANG!
> 
> Unable to handle kernel paging request for data at address 0xd00000000108b148
> Faulting instruction address: 0xd000000000bde35c
> Oops: Kernel access of bad area, sig: 11 [#1]
> SMP NR_CPUS=2048 NUMA pSeries
> Modules linked in: af_packet(E) dm_mod(E) e1000(E) rtc_generic(E) ext4(E) crc16(E) mbcache(E) jbd2(E) sr_mod(E) cdrom(E) sd_mod(E) ibmvscsi(E) scsi_transport_srp(E) sg(E) scsi_mod(E) autofs4(E)
> CPU: 1 PID: 223 Comm: kworker/1:2 Tainted: G            E   4.5.0-rc2-11-default+ #90
> Workqueue: events_freezable_power_ disk_events_workfn
> task: c0000000f7d99aa0 ti: c0000000f7cb8000 task.ti: c0000000f7cb8000
> NIP: d000000000bde35c LR: d000000000bcffec CTR: d000000000bcffe0
> REGS: c0000000f7cbb3b0 TRAP: 0300   Tainted: G            E    (4.5.0-rc2-11-default+)
> MSR: 8000000100009033 <SF,EE,ME,IR,DR,RI,LE,TM[E]>  CR: 24c82220  XER: 00000000
> CFAR: d000000000bdd144 DAR: d00000000108b148 DSISR: 40000000 SOFTE: 0 
> GPR00: d0000000010a21cc c0000000f7cbb630 d0000000010aeda0 0000000000008200 
> GPR04: c0000000fa0b33fc 000000000000014a c0000000fa0b3408 0000000000000010 
> GPR08: 0000000000000008 c00000000370a4e0 0000000000000000 d00000000108b128 
> GPR12: d000000000bcffe0 c000000007e80300 c0000000000d8b18 c0000000fe04ba80 
> GPR16: 0000000000000000 c0000000f7b6f208 0000000000000000 0000000000010000 
> GPR20: c0000000f7b6f144 c0000000f7b6f140 d000000000bcbcf0 0000000000000000 
> GPR24: 0000000000000200 0000000000000000 0000000000000001 c0000000f7b6f810 
> GPR28: c0000000f7b6f000 c0000000f7b6f000 c0000000fa0b33a0 c0000000fe837e00 
> NIP [d000000000bde35c] scsi_inq_str+0x21b0/0x41ac [scsi_mod]
> LR [d000000000bcffec] int_to_scsilun+0xc/0x60 [scsi_mod]
> Call Trace:
> [c0000000f7cbb630] [d0000000010a21cc] ibmvscsi_queuecommand+0x10c/0x4e0 [ibmvscsi] (unreliable)
> [c0000000f7cbb6e0] [d000000000bcbea8] scsi_dispatch_cmd+0xe8/0x2c0 [scsi_mod]
> [c0000000f7cbb760] [d000000000bceb0c] scsi_request_fn+0x50c/0x8b0 [scsi_mod]
> [c0000000f7cbb850] [c000000000407280] __blk_run_queue+0x60/0x90
> [c0000000f7cbb880] [c000000000413640] blk_execute_rq_nowait+0x100/0x1a0
> [c0000000f7cbb8d0] [c000000000413768] blk_execute_rq+0x88/0x170
> [c0000000f7cbb9b0] [d000000000bca048] scsi_execute+0x108/0x1d0 [scsi_mod]
> [c0000000f7cbba20] [d000000000bca2e8] scsi_execute_req_flags+0xc8/0x150 [scsi_mod]
> [c0000000f7cbbae0] [d0000000012209e4] sr_check_events+0xb4/0x340 [sr_mod]
> [c0000000f7cbbb90] [d0000000011c00b4] cdrom_check_events+0x44/0x80 [cdrom]
> [c0000000f7cbbbc0] [d000000001220fa4] sr_block_check_events+0x44/0x60 [sr_mod]
> [c0000000f7cbbbe0] [c0000000004222f8] disk_check_events+0x78/0x1b0
> [c0000000f7cbbc50] [c0000000000d0610] process_one_work+0x1a0/0x480
> [c0000000f7cbbce0] [c0000000000d0998] worker_thread+0xa8/0x5c0
> [c0000000f7cbbd80] [c0000000000d8c24] kthread+0x114/0x140
> [c0000000f7cbbe30] [c000000000009538] ret_from_kernel_thread+0x5c/0xa4
> Instruction dump:
> 396bc360 f8410018 e98b0020 7d8903a6 4e800420 00000000 00000000 0044fb30 
> c0000000 3d62fffe 396bc388 60000000 <e98b0020> 7d8903a6 4e800420 00000000 
> ---[ end trace 3b830c669dd7adb5 ]---
> 
> 
> Note that ibmvscsi_queuecommand() handle TOC and int_to_scsilun()
> does not handle TOC
> 
> $> objdump -hdr  drivers/scsi/ibmvscsi/ibmvscsi.ko
> 00000000000020c0 <ibmvscsi_queuecommand>:
>     20c0:       00 00 4c 3c     addis   r2,r12,0
>                         20c0: R_PPC64_REL16_HA  .TOC.
>     20c4:       00 00 42 38     addi    r2,r2,0
>                         20c4: R_PPC64_REL16_LO  .TOC.+0x4
>     20c8:       a6 02 08 7c     mflr    r0
>     20cc:       10 00 01 f8     std     r0,16(r1)
>     20d0:       01 00 00 48     bl      20d0 <ibmvscsi_queuecommand+0x10>
>                         20d0: R_PPC64_REL24     _mcount
>     20d4:       a6 02 08 7c     mflr    r0
> 
> 
> $> objdump -hdr  drivers/scsi/scsi_mod.ko
> [...]
> 000000000000ffe0 <int_to_scsilun>:
>     ffe0:       a6 02 08 7c     mflr    r0
>     ffe4:       10 00 01 f8     std     r0,16(r1)
>     ffe8:       01 00 00 48     bl      ffe8 <int_to_scsilun+0x8>
>                         ffe8: R_PPC64_REL24     _mcount
>     ffec:       a6 02 08 7c     mflr    r0
>     fff0:       10 00 01 f8     std     r0,16(r1)
>     fff4:       e1 ff 21 f8     stdu    r1,-32(r1)
> 
> 
> I am able to trace int_to_scsilun() if I add a global call
> into this fucntion.
> 
> I could trace any function when everything is built into the kernel.
> 
> 
> I am not sure where the problem is but even normal tracing does not
> work with this patchset and when -pg -mprofile-kernel is used.

When you call into ibmvscsi_queuecommand() it sets up r2 to hold the TOC for
ibmvscsi.ko.

It then calls int_to_scsilun(). int_to_scsilun() does no TOC setup, and tries
to call _mcount. It can't call _mcount() directly (because int_to_scsilun() is
in a module), it has to go via a stub.

That stub uses r2 to find the location of itself, but it only works if r2 holds
the TOC for scsi_mod.ko. In this case r2 still contains ibmvscsi.ko's TOC.

So when the stub calculates the address of itself it gets the wrong value, and
then when it does a load using that address it either faults (as in your case),
or you get some bogus value and jump into a random piece of code.

cheers

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

#1335263

FromKamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Date2016-02-16 11:40 +0100
Message-ID<r2Ld0-6Dh-13@gated-at.bofh.it>
In reply to#1335134
* Torsten Duwe <duwe@lst.de> [2016-02-16 09:23:02]:

> On Tue, Feb 16, 2016 at 11:17:02AM +0530, Kamalesh Babulal wrote:
> > * Petr Mladek <pmladek@suse.com> [2016-02-12 17:45:17]:
> > >  int test(int a)
> > >  {
> > > +	printk("%d\n", a);
> > >  	return ++a;
> > >  }
> > 
> > Thanks. This workaround, helped to load sample livepatch module.
> 
> N.b.: if you try to livepatch/trace such a leaf function without
> global dependencies, it will crash if that function got called with
> a different TOC value. Hence this whole testing.
> 

I am running out of ideas on how to generate this crash, any pointers
will be helpful.

> You may alternatively try my gcc patch ;-)

Thank you. I will give the patch a try.

Regards,
Kamalesh

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web