[PATCH 0/3] powerpc/module_64: Fix _mcount() stub

[PATCH 0/3] powerpc/module_64: Fix _mcount() stub

[Naveen N. Rao]

This series addresses the crash reported by Qian Cai on ppc64le with 
-mprofile-kernel here:
https://lore.kernel.org/r/15AC5B0E-A221-4B8C-9039-FA96B8EF7C88@lca.pw

While fixing patch_instruction() should address the crash, we should 
still change the default stub we setup for _mcount() for cases where a 
kernel is built without ftrace.

- Naveen


Naveen N. Rao (3):
  powerpc/module_64: Consolidate ftrace code
  powerpc/module_64: Simplify check for -mprofile-kernel ftrace
    relocations
  powerpc/module_64: Use special stub for _mcount() with
    -mprofile-kernel

 arch/powerpc/include/asm/module.h |   3 -
 arch/powerpc/kernel/module_64.c   | 281 ++++++++++++++----------------
 2 files changed, 127 insertions(+), 157 deletions(-)


base-commit: a9aa21d05c33c556e48c5062b6632a9b94906570
-- 
2.25.1

[PATCH 1/3] powerpc/module_64: Consolidate ftrace code

[Naveen N. Rao]

module_trampoline_target() is only used by ftrace. Move the prototype
within the appropriate #ifdef in the header. Also, move the function
body to the end of module_64.c so as to consolidate all ftrace code in
one place.

No functional changes.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/module.h |  3 --
 arch/powerpc/kernel/module_64.c   | 69 +++++++++++++++----------------
 2 files changed, 33 insertions(+), 39 deletions(-)

diff --git a/arch/powerpc/include/asm/module.h b/arch/powerpc/include/asm/module.h
index 356658711a86..6b99d773f522 100644
--- a/arch/powerpc/include/asm/module.h
+++ b/arch/powerpc/include/asm/module.h
@@ -90,12 +90,9 @@ struct mod_arch_specific {
 #    ifdef MODULE
 	asm(".section .ftrace.tramp,\"ax\",@nobits; .align 3; .previous");
 #    endif	/* MODULE */
-#endif
 
 int module_trampoline_target(struct module *mod, unsigned long trampoline,
 			     unsigned long *target);
-
-#ifdef CONFIG_DYNAMIC_FTRACE
 int module_finalize_ftrace(struct module *mod, const Elf_Shdr *sechdrs);
 #else
 static inline int module_finalize_ftrace(struct module *mod, const Elf_Shdr *sechdrs)
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 007606a48fd9..54d9d830f4a4 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -144,42 +144,6 @@ static u32 ppc64_stub_insns[] = {
 	PPC_INST_BCTR,
 };
 
-#ifdef CONFIG_DYNAMIC_FTRACE
-int module_trampoline_target(struct module *mod, unsigned long addr,
-			     unsigned long *target)
-{
-	struct ppc64_stub_entry *stub;
-	func_desc_t funcdata;
-	u32 magic;
-
-	if (!within_module_core(addr, mod)) {
-		pr_err("%s: stub %lx not in module %s\n", __func__, addr, mod->name);
-		return -EFAULT;
-	}
-
-	stub = (struct ppc64_stub_entry *)addr;
-
-	if (probe_kernel_read(&magic, &stub->magic, sizeof(magic))) {
-		pr_err("%s: fault reading magic for stub %lx for %s\n", __func__, addr, mod->name);
-		return -EFAULT;
-	}
-
-	if (magic != STUB_MAGIC) {
-		pr_err("%s: bad magic for stub %lx for %s\n", __func__, addr, mod->name);
-		return -EFAULT;
-	}
-
-	if (probe_kernel_read(&funcdata, &stub->funcdata, sizeof(funcdata))) {
-		pr_err("%s: fault reading funcdata for stub %lx for %s\n", __func__, addr, mod->name);
-                return -EFAULT;
-	}
-
-	*target = stub_func_addr(funcdata);
-
-	return 0;
-}
-#endif
-
 /* Count how many different 24-bit relocations (different symbol,
    different addend) */
 static unsigned int count_relocs(const Elf64_Rela *rela, unsigned int num)
@@ -745,6 +709,39 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 }
 
 #ifdef CONFIG_DYNAMIC_FTRACE
+int module_trampoline_target(struct module *mod, unsigned long addr,
+			     unsigned long *target)
+{
+	struct ppc64_stub_entry *stub;
+	func_desc_t funcdata;
+	u32 magic;
+
+	if (!within_module_core(addr, mod)) {
+		pr_err("%s: stub %lx not in module %s\n", __func__, addr, mod->name);
+		return -EFAULT;
+	}
+
+	stub = (struct ppc64_stub_entry *)addr;
+
+	if (probe_kernel_read(&magic, &stub->magic, sizeof(magic))) {
+		pr_err("%s: fault reading magic for stub %lx for %s\n", __func__, addr, mod->name);
+		return -EFAULT;
+	}
+
+	if (magic != STUB_MAGIC) {
+		pr_err("%s: bad magic for stub %lx for %s\n", __func__, addr, mod->name);
+		return -EFAULT;
+	}
+
+	if (probe_kernel_read(&funcdata, &stub->funcdata, sizeof(funcdata))) {
+		pr_err("%s: fault reading funcdata for stub %lx for %s\n", __func__, addr, mod->name);
+                return -EFAULT;
+	}
+
+	*target = stub_func_addr(funcdata);
+
+	return 0;
+}
 
 #ifdef CONFIG_MPROFILE_KERNEL
 
-- 
2.25.1

[PATCH 2/3] powerpc/module_64: Simplify check for -mprofile-kernel ftrace relocations

[Naveen N. Rao]

For -mprofile-kernel, we need special handling when generating stubs for
ftrace calls such as _mcount(). To faciliate this, we check if a
R_PPC64_REL24 relocation is for a symbol named "_mcount()" along with
also checking the instruction sequence. The latter is not really
required since "_mcount()" is an exported symbol and kernel modules
cannot use it. As such, drop the additional checking and simplify the
code. This helps unify stub creation for ftrace stubs with
-mprofile-kernel and aids in code reuse.

Also rename is_mprofile_mcount_callsite() to is_mprofile_ftrace_call()
to reflect the checking being done.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/module_64.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 54d9d830f4a4..7f4cc5346387 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -414,19 +414,9 @@ static unsigned long stub_for_addr(const Elf64_Shdr *sechdrs,
 }
 
 #ifdef CONFIG_MPROFILE_KERNEL
-static bool is_mprofile_mcount_callsite(const char *name, u32 *instruction)
+static bool is_mprofile_ftrace_call(const char *name)
 {
-	if (strcmp("_mcount", name))
-		return false;
-
-	/*
-	 * Check if this is one of the -mprofile-kernel sequences.
-	 */
-	if (instruction[-1] == PPC_INST_STD_LR &&
-	    instruction[-2] == PPC_INST_MFLR)
-		return true;
-
-	if (instruction[-1] == PPC_INST_MFLR)
+	if (!strcmp("_mcount", name))
 		return true;
 
 	return false;
@@ -450,7 +440,7 @@ static void squash_toc_save_inst(const char *name, unsigned long addr)
 #else
 static void squash_toc_save_inst(const char *name, unsigned long addr) { }
 
-static bool is_mprofile_mcount_callsite(const char *name, u32 *instruction)
+static bool is_mprofile_ftrace_call(const char *name)
 {
 	return false;
 }
@@ -462,7 +452,7 @@ static int restore_r2(const char *name, u32 *instruction, struct module *me)
 {
 	u32 *prev_insn = instruction - 1;
 
-	if (is_mprofile_mcount_callsite(name, prev_insn))
+	if (is_mprofile_ftrace_call(name))
 		return 1;
 
 	/*
-- 
2.25.1

[PATCH 3/3] powerpc/module_64: Use special stub for _mcount() with -mprofile-kernel

[Naveen N. Rao]

Since commit c55d7b5e64265f ("powerpc: Remove STRICT_KERNEL_RWX
incompatibility with RELOCATABLE"), powerpc kernels with
-mprofile-kernel can crash in certain scenarios with a trace like below:

    BUG: Unable to handle kernel instruction fetch (NULL pointer?)
    Faulting instruction address: 0x00000000
    Oops: Kernel access of bad area, sig: 11 [#1]
    LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
    <snip>
    NIP [0000000000000000] 0x0
    LR [c0080000102c0048] ext4_iomap_end+0x8/0x30 [ext4]
    Call Trace:
     iomap_apply+0x20c/0x920 (unreliable)
     iomap_bmap+0xfc/0x160
     ext4_bmap+0xa4/0x180 [ext4]
     bmap+0x4c/0x80
     jbd2_journal_init_inode+0x44/0x1a0 [jbd2]
     ext4_load_journal+0x440/0x860 [ext4]
     ext4_fill_super+0x342c/0x3ab0 [ext4]
     mount_bdev+0x25c/0x290
     ext4_mount+0x28/0x50 [ext4]
     legacy_get_tree+0x4c/0xb0
     vfs_get_tree+0x4c/0x130
     do_mount+0xa18/0xc50
     sys_mount+0x158/0x180
     system_call+0x5c/0x68

The NIP points to NULL, or a random location (data even), while the LR
always points to the LEP of a function (with an offset of 8), indicating
that something went wrong with ftrace. However, ftrace is not
necessarily active when such crashes occur.

The kernel OOPS sometimes follows a warning from ftrace indicating that
some module functions could not be patched with a nop. Other times, if a
module is loaded early during boot, instruction patching can fail due to
a separate bug, but the error is not reported due to missing error
reporting.

In all the above cases when instruction patching fails, ftrace will be
disabled but certain kernel module functions will be left with default
calls to _mcount(). This is not a problem with ELFv1. However, with
-mprofile-kernel, the default stub is problematic since it depends on a
valid module TOC in r2. If the kernel (or a different module) calls into
a function that does not use the TOC, the function won't have a prologue
to setup the module TOC. When that function calls into _mcount(), we
will end up in the relocation stub that will use the previous TOC, and
end up trying to jump into a random location. From the above trace:

	iomap_apply+0x20c/0x920 [kernel TOC]
			|
			V
	ext4_iomap_end+0x8/0x30 [no GEP == kernel TOC]
			|
			V
		_mcount() stub
	[uses kernel TOC -> random entry]

To address this, let's change over to using the special stub that is
used for ftrace_[regs_]caller() for _mcount(). This ensures that we are
not dependent on a valid module TOC in r2 for default _mcount()
handling.

Reported-by: Qian Cai <cai@lca.pw>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/module_64.c | 222 +++++++++++++++-----------------
 1 file changed, 104 insertions(+), 118 deletions(-)

diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 7f4cc5346387..cc7e990e8376 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -348,6 +348,92 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
 	return 0;
 }
 
+#ifdef CONFIG_MPROFILE_KERNEL
+
+#define PACATOC offsetof(struct paca_struct, kernel_toc)
+
+/*
+ * ld      r12,PACATOC(r13)
+ * addis   r12,r12,<high>
+ * addi    r12,r12,<low>
+ * mtctr   r12
+ * bctr
+ */
+static u32 stub_insns[] = {
+	PPC_INST_LD | __PPC_RT(R12) | __PPC_RA(R13) | PACATOC,
+	PPC_INST_ADDIS | __PPC_RT(R12) | __PPC_RA(R12),
+	PPC_INST_ADDI | __PPC_RT(R12) | __PPC_RA(R12),
+	PPC_INST_MTCTR | __PPC_RS(R12),
+	PPC_INST_BCTR,
+};
+
+/*
+ * For mprofile-kernel we use a special stub for ftrace_caller() because we
+ * can't rely on r2 containing this module's TOC when we enter the stub.
+ *
+ * That can happen if the function calling us didn't need to use the toc. In
+ * that case it won't have setup r2, and the r2 value will be either the
+ * kernel's toc, or possibly another modules toc.
+ *
+ * To deal with that this stub uses the kernel toc, which is always accessible
+ * via the paca (in r13). The target (ftrace_caller()) is responsible for
+ * saving and restoring the toc before returning.
+ */
+static inline int create_ftrace_stub(struct ppc64_stub_entry *entry,
+					unsigned long addr,
+					struct module *me)
+{
+	long reladdr;
+
+	memcpy(entry->jump, stub_insns, sizeof(stub_insns));
+
+	/* Stub uses address relative to kernel toc (from the paca) */
+	reladdr = addr - kernel_toc_addr();
+	if (reladdr > 0x7FFFFFFF || reladdr < -(0x80000000L)) {
+		pr_err("%s: Address of %ps out of range of kernel_toc.\n",
+							me->name, (void *)addr);
+		return 0;
+	}
+
+	entry->jump[1] |= PPC_HA(reladdr);
+	entry->jump[2] |= PPC_LO(reladdr);
+
+	/* Eventhough we don't use funcdata in the stub, it's needed elsewhere. */
+	entry->funcdata = func_desc(addr);
+	entry->magic = STUB_MAGIC;
+
+	return 1;
+}
+
+static bool is_mprofile_ftrace_call(const char *name)
+{
+	if (!strcmp("_mcount", name))
+		return true;
+#ifdef CONFIG_DYNAMIC_FTRACE
+	if (!strcmp("ftrace_caller", name))
+		return true;
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	if (!strcmp("ftrace_regs_caller", name))
+		return true;
+#endif
+#endif
+
+	return false;
+}
+#else
+static inline int create_ftrace_stub(struct ppc64_stub_entry *entry,
+					unsigned long addr,
+					struct module *me)
+{
+	return 0;
+}
+
+static bool is_mprofile_ftrace_call(const char *name)
+{
+	return false;
+}
+#endif
+
 /*
  * r2 is the TOC pointer: it actually points 0x8000 into the TOC (this gives the
  * value maximum span in an instruction which uses a signed offset). Round down
@@ -363,10 +449,14 @@ static inline unsigned long my_r2(const Elf64_Shdr *sechdrs, struct module *me)
 static inline int create_stub(const Elf64_Shdr *sechdrs,
 			      struct ppc64_stub_entry *entry,
 			      unsigned long addr,
-			      struct module *me)
+			      struct module *me,
+			      const char *name)
 {
 	long reladdr;
 
+	if (is_mprofile_ftrace_call(name))
+		return create_ftrace_stub(entry, addr, me);
+
 	memcpy(entry->jump, ppc64_stub_insns, sizeof(ppc64_stub_insns));
 
 	/* Stub uses address relative to r2. */
@@ -390,7 +480,8 @@ static inline int create_stub(const Elf64_Shdr *sechdrs,
    stub to set up the TOC ptr (r2) for the function. */
 static unsigned long stub_for_addr(const Elf64_Shdr *sechdrs,
 				   unsigned long addr,
-				   struct module *me)
+				   struct module *me,
+				   const char *name)
 {
 	struct ppc64_stub_entry *stubs;
 	unsigned int i, num_stubs;
@@ -407,45 +498,12 @@ static unsigned long stub_for_addr(const Elf64_Shdr *sechdrs,
 			return (unsigned long)&stubs[i];
 	}
 
-	if (!create_stub(sechdrs, &stubs[i], addr, me))
+	if (!create_stub(sechdrs, &stubs[i], addr, me, name))
 		return 0;
 
 	return (unsigned long)&stubs[i];
 }
 
-#ifdef CONFIG_MPROFILE_KERNEL
-static bool is_mprofile_ftrace_call(const char *name)
-{
-	if (!strcmp("_mcount", name))
-		return true;
-
-	return false;
-}
-
-/*
- * In case of _mcount calls, do not save the current callee's TOC (in r2) into
- * the original caller's stack frame. If we did we would clobber the saved TOC
- * value of the original caller.
- */
-static void squash_toc_save_inst(const char *name, unsigned long addr)
-{
-	struct ppc64_stub_entry *stub = (struct ppc64_stub_entry *)addr;
-
-	/* Only for calls to _mcount */
-	if (strcmp("_mcount", name) != 0)
-		return;
-
-	stub->jump[2] = PPC_INST_NOP;
-}
-#else
-static void squash_toc_save_inst(const char *name, unsigned long addr) { }
-
-static bool is_mprofile_ftrace_call(const char *name)
-{
-	return false;
-}
-#endif
-
 /* We expect a noop next: if it is, replace it with instruction to
    restore r2. */
 static int restore_r2(const char *name, u32 *instruction, struct module *me)
@@ -590,14 +648,13 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 			if (sym->st_shndx == SHN_UNDEF ||
 			    sym->st_shndx == SHN_LIVEPATCH) {
 				/* External: go via stub */
-				value = stub_for_addr(sechdrs, value, me);
+				value = stub_for_addr(sechdrs, value, me,
+						strtab + sym->st_name);
 				if (!value)
 					return -ENOENT;
 				if (!restore_r2(strtab + sym->st_name,
 							(u32 *)location + 1, me))
 					return -ENOEXEC;
-
-				squash_toc_save_inst(strtab + sym->st_name, value);
 			} else
 				value += local_entry_offset(sym);
 
@@ -733,88 +790,17 @@ int module_trampoline_target(struct module *mod, unsigned long addr,
 	return 0;
 }
 
-#ifdef CONFIG_MPROFILE_KERNEL
-
-#define PACATOC offsetof(struct paca_struct, kernel_toc)
-
-/*
- * For mprofile-kernel we use a special stub for ftrace_caller() because we
- * can't rely on r2 containing this module's TOC when we enter the stub.
- *
- * That can happen if the function calling us didn't need to use the toc. In
- * that case it won't have setup r2, and the r2 value will be either the
- * kernel's toc, or possibly another modules toc.
- *
- * To deal with that this stub uses the kernel toc, which is always accessible
- * via the paca (in r13). The target (ftrace_caller()) is responsible for
- * saving and restoring the toc before returning.
- */
-static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs,
-				struct module *me, unsigned long addr)
-{
-	struct ppc64_stub_entry *entry;
-	unsigned int i, num_stubs;
-	/*
-	 * ld      r12,PACATOC(r13)
-	 * addis   r12,r12,<high>
-	 * addi    r12,r12,<low>
-	 * mtctr   r12
-	 * bctr
-	 */
-	static u32 stub_insns[] = {
-		PPC_INST_LD | __PPC_RT(R12) | __PPC_RA(R13) | PACATOC,
-		PPC_INST_ADDIS | __PPC_RT(R12) | __PPC_RA(R12),
-		PPC_INST_ADDI | __PPC_RT(R12) | __PPC_RA(R12),
-		PPC_INST_MTCTR | __PPC_RS(R12),
-		PPC_INST_BCTR,
-	};
-	long reladdr;
-
-	num_stubs = sechdrs[me->arch.stubs_section].sh_size / sizeof(*entry);
-
-	/* Find the next available stub entry */
-	entry = (void *)sechdrs[me->arch.stubs_section].sh_addr;
-	for (i = 0; i < num_stubs && stub_func_addr(entry->funcdata); i++, entry++);
-
-	if (i >= num_stubs) {
-		pr_err("%s: Unable to find a free slot for ftrace stub.\n", me->name);
-		return 0;
-	}
-
-	memcpy(entry->jump, stub_insns, sizeof(stub_insns));
-
-	/* Stub uses address relative to kernel toc (from the paca) */
-	reladdr = addr - kernel_toc_addr();
-	if (reladdr > 0x7FFFFFFF || reladdr < -(0x80000000L)) {
-		pr_err("%s: Address of %ps out of range of kernel_toc.\n",
-							me->name, (void *)addr);
-		return 0;
-	}
-
-	entry->jump[1] |= PPC_HA(reladdr);
-	entry->jump[2] |= PPC_LO(reladdr);
-
-	/* Eventhough we don't use funcdata in the stub, it's needed elsewhere. */
-	entry->funcdata = func_desc(addr);
-	entry->magic = STUB_MAGIC;
-
-	return (unsigned long)entry;
-}
-#else
-static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs,
-				struct module *me, unsigned long addr)
-{
-	return stub_for_addr(sechdrs, addr, me);
-}
-#endif
-
 int module_finalize_ftrace(struct module *mod, const Elf_Shdr *sechdrs)
 {
-	mod->arch.tramp = create_ftrace_stub(sechdrs, mod,
-					(unsigned long)ftrace_caller);
+	mod->arch.tramp = stub_for_addr(sechdrs,
+					(unsigned long)ftrace_caller,
+					mod,
+					"ftrace_caller");
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-	mod->arch.tramp_regs = create_ftrace_stub(sechdrs, mod,
-					(unsigned long)ftrace_regs_caller);
+	mod->arch.tramp_regs = stub_for_addr(sechdrs,
+					(unsigned long)ftrace_regs_caller,
+					mod,
+					"ftrace_regs_caller");
 	if (!mod->arch.tramp_regs)
 		return -ENOENT;
 #endif
-- 
2.25.1

Re: [PATCH 3/3] powerpc/module_64: Use special stub for _mcount() with -mprofile-kernel

[Qian Cai]

> On Apr 21, 2020, at 1:35 PM, Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
> Since commit c55d7b5e64265f ("powerpc: Remove STRICT_KERNEL_RWX
> incompatibility with RELOCATABLE"), powerpc kernels with
> -mprofile-kernel can crash in certain scenarios with a trace like below:
> 
>    BUG: Unable to handle kernel instruction fetch (NULL pointer?)
>    Faulting instruction address: 0x00000000
>    Oops: Kernel access of bad area, sig: 11 [#1]
>    LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
>    <snip>
>    NIP [0000000000000000] 0x0
>    LR [c0080000102c0048] ext4_iomap_end+0x8/0x30 [ext4]
>    Call Trace:
>     iomap_apply+0x20c/0x920 (unreliable)
>     iomap_bmap+0xfc/0x160
>     ext4_bmap+0xa4/0x180 [ext4]
>     bmap+0x4c/0x80
>     jbd2_journal_init_inode+0x44/0x1a0 [jbd2]
>     ext4_load_journal+0x440/0x860 [ext4]
>     ext4_fill_super+0x342c/0x3ab0 [ext4]
>     mount_bdev+0x25c/0x290
>     ext4_mount+0x28/0x50 [ext4]
>     legacy_get_tree+0x4c/0xb0
>     vfs_get_tree+0x4c/0x130
>     do_mount+0xa18/0xc50
>     sys_mount+0x158/0x180
>     system_call+0x5c/0x68
> 
> The NIP points to NULL, or a random location (data even), while the LR
> always points to the LEP of a function (with an offset of 8), indicating
> that something went wrong with ftrace. However, ftrace is not
> necessarily active when such crashes occur.
> 
> The kernel OOPS sometimes follows a warning from ftrace indicating that
> some module functions could not be patched with a nop. Other times, if a
> module is loaded early during boot, instruction patching can fail due to
> a separate bug, but the error is not reported due to missing error
> reporting.
> 
> In all the above cases when instruction patching fails, ftrace will be
> disabled but certain kernel module functions will be left with default
> calls to _mcount(). This is not a problem with ELFv1. However, with
> -mprofile-kernel, the default stub is problematic since it depends on a
> valid module TOC in r2. If the kernel (or a different module) calls into
> a function that does not use the TOC, the function won't have a prologue
> to setup the module TOC. When that function calls into _mcount(), we
> will end up in the relocation stub that will use the previous TOC, and
> end up trying to jump into a random location. From the above trace:
> 
> 	iomap_apply+0x20c/0x920 [kernel TOC]
> 			|
> 			V
> 	ext4_iomap_end+0x8/0x30 [no GEP == kernel TOC]
> 			|
> 			V
> 		_mcount() stub
> 	[uses kernel TOC -> random entry]
> 
> To address this, let's change over to using the special stub that is
> used for ftrace_[regs_]caller() for _mcount(). This ensures that we are
> not dependent on a valid module TOC in r2 for default _mcount()
> handling.
> 
> Reported-by: Qian Cai <cai@lca.pw>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Feel free to add,

Tested-by: Qian Cai <cai@lca.pw>

Re: [PATCH 0/3] powerpc/module_64: Fix _mcount() stub

[Naveen N. Rao]

Naveen N. Rao wrote:
> This series addresses the crash reported by Qian Cai on ppc64le with 
> -mprofile-kernel here:
> https://lore.kernel.org/r/15AC5B0E-A221-4B8C-9039-FA96B8EF7C88@lca.pw
> 
> While fixing patch_instruction() should address the crash, we should 
> still change the default stub we setup for _mcount() for cases where a 
> kernel is built without ftrace.

I notice that I didn't write that properly yesterday. I meant that this 
is required for cases where ftrace gets disabled for any reason, 
resulting in the default calls to _mcount() remaining in the kernel 
modules.

- Naveen

Re: [PATCH 0/3] powerpc/module_64: Fix _mcount() stub

[Michael Ellerman]

On Tue, 21 Apr 2020 23:05:42 +0530, Naveen N. Rao wrote:
> This series addresses the crash reported by Qian Cai on ppc64le with
> -mprofile-kernel here:
> https://lore.kernel.org/r/15AC5B0E-A221-4B8C-9039-FA96B8EF7C88@lca.pw
> 
> While fixing patch_instruction() should address the crash, we should
> still change the default stub we setup for _mcount() for cases where a
> kernel is built without ftrace.
> 
> [...]

Applied to powerpc/next.

[1/3] powerpc/module_64: Consolidate ftrace code
      https://git.kernel.org/powerpc/c/03b51416e876aea5e7638947e50831b6c988c246
[2/3] powerpc/module_64: Simplify check for -mprofile-kernel ftrace relocations
      https://git.kernel.org/powerpc/c/1f2aaed2db03150428dbcd2ddee02ae6cb4bac52
[3/3] powerpc/module_64: Use special stub for _mcount() with -mprofile-kernel
      https://git.kernel.org/powerpc/c/bd55e792de0844631d34487d43eaf3f13294ebfe

cheers