All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3
@ 2017-06-29 17:49 Naveen N. Rao
  2017-06-29 17:49 ` [PATCH v5 1/7] powerpc64/elfv1: Only dereference function descriptor for non-text symbols Naveen N. Rao
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Naveen N. Rao @ 2017-06-29 17:49 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nicholas Piggin, Ananth N Mavinakayanahalli, Masami Hiramatsu,
	Anton Blanchard, linuxppc-dev

This is the third in the series of patches to build out an appropriate
kprobes blacklist for powerpc. Since posting the second series (*),
there have been related changes to the code and I have brought that
series forward to account for those changes. As such, all patches from
the second series are included in this patchset.

This patchset now ensures that the newly added multiple kprobes test in
the ftrace testsuite passes on powerpc64. Tested on both Elfv1 and
Elfv2.

Changes since v4:
  - Patch 5 changed to move system_call_exit() symbol before the mtmsrd,
    along with an explanation for its placement.
  - Patch 7 reverted to previous version moving the new symbol before
    the mtmsrd as well.
  - All other patches remain unchanged from v4.

v4:
https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg120106.html

(*) series 2:
https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg117562.html


- Naveen


Naveen N. Rao (7):
  powerpc64/elfv1: Only dereference function descriptor for non-text
    symbols
  powerpc/64s: Convert .L__replay_interrupt_return to a local label
  powerpc/64s: Blacklist system_call() and system_call_common() from
    kprobes
  powerpc/64s: Move system_call() symbol to just after setting MSR_EE
  powerpc/64s: Un-blacklist system_call() from kprobes
  powerpc/64s: Blacklist functions invoked on a trap
  powerpc/64s: Blacklist rtas entry/exit from kprobes

 arch/powerpc/include/asm/code-patching.h | 10 +++-
 arch/powerpc/kernel/entry_64.S           | 87 +++++++++++++++++++++-----------
 arch/powerpc/kernel/exceptions-64s.S     |  6 ++-
 arch/powerpc/kernel/traps.c              |  3 ++
 4 files changed, 73 insertions(+), 33 deletions(-)

-- 
2.13.1

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

* [PATCH v5 1/7] powerpc64/elfv1: Only dereference function descriptor for non-text symbols
  2017-06-29 17:49 [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3 Naveen N. Rao
@ 2017-06-29 17:49 ` Naveen N. Rao
  2017-07-04 10:48   ` [v5, " Michael Ellerman
  2017-06-29 17:49 ` [PATCH v5 2/7] powerpc/64s: Convert .L__replay_interrupt_return to a local label Naveen N. Rao
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Naveen N. Rao @ 2017-06-29 17:49 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nicholas Piggin, Ananth N Mavinakayanahalli, Masami Hiramatsu,
	Anton Blanchard, linuxppc-dev

Currently, we assume that the function pointer we receive in
ppc_function_entry() points to a function descriptor. However, this is
not always the case. In particular, assembly symbols without the right
annotation do not have an associated function descriptor. Some of these
symbols are added to the kprobe blacklist using _ASM_NOKPROBE_SYMBOL().

When such addresses are subsequently processed through
arch_deref_entry_point() in populate_kprobe_blacklist(), we see the
below errors during bootup:
    [    0.663963] Failed to find blacklist at 7d9b02a648029b6c
    [    0.663970] Failed to find blacklist at a14d03d0394a0001
    [    0.663972] Failed to find blacklist at 7d5302a6f94d0388
    [    0.663973] Failed to find blacklist at 48027d11e8610178
    [    0.663974] Failed to find blacklist at f8010070f8410080
    [    0.663976] Failed to find blacklist at 386100704801f89d
    [    0.663977] Failed to find blacklist at 7d5302a6f94d00b0

Fix this by checking if the function pointer we receive in
ppc_function_entry() already points to kernel text. If so, we just
return it as is. If not, we assume that this is a function descriptor
and proceed to dereference it.

Suggested-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/code-patching.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index abef812de7f8..5482928eea1b 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -83,8 +83,16 @@ static inline unsigned long ppc_function_entry(void *func)
 	 * On PPC64 ABIv1 the function pointer actually points to the
 	 * function's descriptor. The first entry in the descriptor is the
 	 * address of the function text.
+	 *
+	 * However, we may also receive pointer to an assembly symbol. To
+	 * detect that, we first check if the function pointer we receive
+	 * already points to kernel/module text and we only dereference it
+	 * if it doesn't.
 	 */
-	return ((func_descr_t *)func)->entry;
+	if (kernel_text_address((unsigned long)func))
+		return (unsigned long)func;
+	else
+		return ((func_descr_t *)func)->entry;
 #else
 	return (unsigned long)func;
 #endif
-- 
2.13.1

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

* [PATCH v5 2/7] powerpc/64s: Convert .L__replay_interrupt_return to a local label
  2017-06-29 17:49 [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3 Naveen N. Rao
  2017-06-29 17:49 ` [PATCH v5 1/7] powerpc64/elfv1: Only dereference function descriptor for non-text symbols Naveen N. Rao
@ 2017-06-29 17:49 ` Naveen N. Rao
  2017-06-29 17:49 ` [PATCH v5 3/7] powerpc/64s: Blacklist system_call() and system_call_common() from kprobes Naveen N. Rao
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Naveen N. Rao @ 2017-06-29 17:49 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nicholas Piggin, Ananth N Mavinakayanahalli, Masami Hiramatsu,
	Anton Blanchard, linuxppc-dev

Commit b48bbb82e2b835 ("powerpc/64s: Don't unbalance the return branch
predictor in __replay_interrupt()") introduced __replay_interrupt_return
symbol with '.L' prefix in hopes of keeping it private. However, due to
the use of LOAD_REG_ADDR(), the assembler kept this symbol visible. Fix
the same by instead using the local label '1'.

Fixes: Commit b48bbb82e2b835 ("powerpc/64s: Don't unbalance the return branch
predictor in __replay_interrupt()")
Suggested-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/exceptions-64s.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 02a82777fd5b..3262e447b895 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1631,7 +1631,7 @@ _GLOBAL(__replay_interrupt)
 	 * we don't give a damn about, so we don't bother storing them.
 	 */
 	mfmsr	r12
-	LOAD_REG_ADDR(r11, .L__replay_interrupt_return)
+	LOAD_REG_ADDR(r11, 1f)
 	mfcr	r9
 	ori	r12,r12,MSR_EE
 	cmpwi	r3,0x900
@@ -1649,6 +1649,6 @@ FTR_SECTION_ELSE
 	cmpwi	r3,0xa00
 	beq	doorbell_super_common_msgclr
 ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE)
-.L__replay_interrupt_return:
+1:
 	blr
 
-- 
2.13.1

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

* [PATCH v5 3/7] powerpc/64s: Blacklist system_call() and system_call_common() from kprobes
  2017-06-29 17:49 [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3 Naveen N. Rao
  2017-06-29 17:49 ` [PATCH v5 1/7] powerpc64/elfv1: Only dereference function descriptor for non-text symbols Naveen N. Rao
  2017-06-29 17:49 ` [PATCH v5 2/7] powerpc/64s: Convert .L__replay_interrupt_return to a local label Naveen N. Rao
@ 2017-06-29 17:49 ` Naveen N. Rao
  2017-07-01  2:24   ` Michael Ellerman
  2017-06-29 17:49 ` [PATCH v5 4/7] powerpc/64s: Move system_call() symbol to just after setting MSR_EE Naveen N. Rao
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Naveen N. Rao @ 2017-06-29 17:49 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nicholas Piggin, Ananth N Mavinakayanahalli, Masami Hiramatsu,
	Anton Blanchard, linuxppc-dev

Convert some of the symbols into private symbols and blacklist
system_call_common() and system_call() from kprobes. We can't take a
trap at parts of these functions as either MSR_RI is unset or the kernel
stack pointer is not yet setup.

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/entry_64.S | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index da9486e2fd89..ef8e6615b8ba 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -52,12 +52,11 @@ exception_marker:
 	.section	".text"
 	.align 7
 
-	.globl system_call_common
-system_call_common:
+_GLOBAL(system_call_common)
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 BEGIN_FTR_SECTION
 	extrdi.	r10, r12, 1, (63-MSR_TS_T_LG) /* transaction active? */
-	bne	tabort_syscall
+	bne	.Ltabort_syscall
 END_FTR_SECTION_IFSET(CPU_FTR_TM)
 #endif
 	andi.	r10,r12,MSR_PR
@@ -152,9 +151,9 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLPAR)
 	CURRENT_THREAD_INFO(r11, r1)
 	ld	r10,TI_FLAGS(r11)
 	andi.	r11,r10,_TIF_SYSCALL_DOTRACE
-	bne	syscall_dotrace		/* does not return */
+	bne	.Lsyscall_dotrace		/* does not return */
 	cmpldi	0,r0,NR_syscalls
-	bge-	syscall_enosys
+	bge-	.Lsyscall_enosys
 
 system_call:			/* label this so stack traces look sane */
 /*
@@ -208,7 +207,7 @@ system_call:			/* label this so stack traces look sane */
 	ld	r9,TI_FLAGS(r12)
 	li	r11,-MAX_ERRNO
 	andi.	r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK)
-	bne-	syscall_exit_work
+	bne-	.Lsyscall_exit_work
 
 	/* If MSR_FP and MSR_VEC are set in user msr, then no need to restore */
 	li	r7,MSR_FP
@@ -217,12 +216,12 @@ system_call:			/* label this so stack traces look sane */
 #endif
 	and	r0,r8,r7
 	cmpd	r0,r7
-	bne	syscall_restore_math
+	bne	.Lsyscall_restore_math
 .Lsyscall_restore_math_cont:
 
 	cmpld	r3,r11
 	ld	r5,_CCR(r1)
-	bge-	syscall_error
+	bge-	.Lsyscall_error
 .Lsyscall_error_cont:
 	ld	r7,_NIP(r1)
 BEGIN_FTR_SECTION
@@ -248,13 +247,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	RFI
 	b	.	/* prevent speculative execution */
 
-syscall_error:	
+.Lsyscall_error:
 	oris	r5,r5,0x1000	/* Set SO bit in CR */
 	neg	r3,r3
 	std	r5,_CCR(r1)
 	b	.Lsyscall_error_cont
 
-syscall_restore_math:
+.Lsyscall_restore_math:
 	/*
 	 * Some initial tests from restore_math to avoid the heavyweight
 	 * C code entry and MSR manipulations.
@@ -289,7 +288,7 @@ syscall_restore_math:
 	b	.Lsyscall_restore_math_cont
 
 /* Traced system call support */
-syscall_dotrace:
+.Lsyscall_dotrace:
 	bl	save_nvgprs
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	do_syscall_trace_enter
@@ -322,11 +321,11 @@ syscall_dotrace:
 	b	.Lsyscall_exit
 
 
-syscall_enosys:
+.Lsyscall_enosys:
 	li	r3,-ENOSYS
 	b	.Lsyscall_exit
 	
-syscall_exit_work:
+.Lsyscall_exit_work:
 #ifdef CONFIG_PPC_BOOK3S
 	li	r10,MSR_RI
 	mtmsrd	r10,1		/* Restore RI */
@@ -386,7 +385,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	b	ret_from_except
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-tabort_syscall:
+.Ltabort_syscall:
 	/* Firstly we need to enable TM in the kernel */
 	mfmsr	r10
 	li	r9, 1
@@ -412,6 +411,8 @@ tabort_syscall:
 	rfid
 	b	.	/* prevent speculative execution */
 #endif
+_ASM_NOKPROBE_SYMBOL(system_call_common);
+_ASM_NOKPROBE_SYMBOL(system_call);
 
 /* Save non-volatile GPRs, if not already saved. */
 _GLOBAL(save_nvgprs)
-- 
2.13.1

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

* [PATCH v5 4/7] powerpc/64s: Move system_call() symbol to just after setting MSR_EE
  2017-06-29 17:49 [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3 Naveen N. Rao
                   ` (2 preceding siblings ...)
  2017-06-29 17:49 ` [PATCH v5 3/7] powerpc/64s: Blacklist system_call() and system_call_common() from kprobes Naveen N. Rao
@ 2017-06-29 17:49 ` Naveen N. Rao
  2017-06-29 17:49 ` [PATCH v5 5/7] powerpc/64s: Un-blacklist system_call() from kprobes Naveen N. Rao
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Naveen N. Rao @ 2017-06-29 17:49 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nicholas Piggin, Ananth N Mavinakayanahalli, Masami Hiramatsu,
	Anton Blanchard, linuxppc-dev

It is common to get a PMU interrupt right after the mtmsr instruction that
enables interrupts. Due to this, the stack trace profile gets needlessly split
across system_call_common() and system_call().

Previously, system_call() symbol was at the current place to hide a few
earlier symbols which have since been made private or removed entirely.

So, let's move system_call() slightly higher up, right after the mtmsr
instruction that enables interrupts. Convert existing references to
system_call to a local syscall symbol.

Suggested-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/entry_64.S | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index ef8e6615b8ba..c39436706555 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -142,6 +142,7 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLPAR)
 	mtmsrd	r11,1
 #endif /* CONFIG_PPC_BOOK3E */
 
+system_call:			/* label this so stack traces look sane */
 	/* We do need to set SOFTE in the stack frame or the return
 	 * from interrupt will be painful
 	 */
@@ -155,7 +156,7 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLPAR)
 	cmpldi	0,r0,NR_syscalls
 	bge-	.Lsyscall_enosys
 
-system_call:			/* label this so stack traces look sane */
+.Lsyscall:
 /*
  * Need to vector to 32 Bit or default sys_call_table here,
  * based on caller's run-mode / personality.
@@ -309,13 +310,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	ld	r7,GPR7(r1)
 	ld	r8,GPR8(r1)
 
-	/* Repopulate r9 and r10 for the system_call path */
+	/* Repopulate r9 and r10 for the syscall path */
 	addi	r9,r1,STACK_FRAME_OVERHEAD
 	CURRENT_THREAD_INFO(r10, r1)
 	ld	r10,TI_FLAGS(r10)
 
 	cmpldi	r0,NR_syscalls
-	blt+	system_call
+	blt+	.Lsyscall
 
 	/* Return code is already in r3 thanks to do_syscall_trace_enter() */
 	b	.Lsyscall_exit
-- 
2.13.1

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

* [PATCH v5 5/7] powerpc/64s: Un-blacklist system_call() from kprobes
  2017-06-29 17:49 [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3 Naveen N. Rao
                   ` (3 preceding siblings ...)
  2017-06-29 17:49 ` [PATCH v5 4/7] powerpc/64s: Move system_call() symbol to just after setting MSR_EE Naveen N. Rao
@ 2017-06-29 17:49 ` Naveen N. Rao
  2017-06-29 17:49 ` [PATCH v5 6/7] powerpc/64s: Blacklist functions invoked on a trap Naveen N. Rao
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Naveen N. Rao @ 2017-06-29 17:49 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nicholas Piggin, Ananth N Mavinakayanahalli, Masami Hiramatsu,
	Anton Blanchard, linuxppc-dev

It is actually safe to probe system_call() in entry_64.S, but only till
we unset MSR_RI. To allow this, add a new symbol system_call_exit()
after the mtmsrd and blacklist that.

Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/entry_64.S | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index c39436706555..5b919b2a62da 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -187,6 +187,18 @@ system_call:			/* label this so stack traces look sane */
 	andi.	r10,r8,MSR_RI
 	beq-	unrecov_restore
 #endif
+
+/*
+ * This is a few instructions into the actual syscall exit path (which actually
+ * starts at .Lsyscall_exit) to cater to kprobe blacklisting and to reduce the
+ * number of visible symbols for profiling purposes.
+ *
+ * We can probe from system_call until this point as MSR_RI is set. But once it
+ * is cleared below, we won't be able to take a trap.
+ *
+ * This is blacklisted from kprobes further below with _ASM_NOKPROBE_SYMBOL().
+ */
+system_call_exit:
 	/*
 	 * Disable interrupts so current_thread_info()->flags can't change,
 	 * and so that we don't get interrupted after loading SRR0/1.
@@ -413,7 +425,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	b	.	/* prevent speculative execution */
 #endif
 _ASM_NOKPROBE_SYMBOL(system_call_common);
-_ASM_NOKPROBE_SYMBOL(system_call);
+_ASM_NOKPROBE_SYMBOL(system_call_exit);
 
 /* Save non-volatile GPRs, if not already saved. */
 _GLOBAL(save_nvgprs)
-- 
2.13.1

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

* [PATCH v5 6/7] powerpc/64s: Blacklist functions invoked on a trap
  2017-06-29 17:49 [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3 Naveen N. Rao
                   ` (4 preceding siblings ...)
  2017-06-29 17:49 ` [PATCH v5 5/7] powerpc/64s: Un-blacklist system_call() from kprobes Naveen N. Rao
@ 2017-06-29 17:49 ` Naveen N. Rao
  2017-06-29 17:49 ` [PATCH v5 7/7] powerpc/64s: Blacklist rtas entry/exit from kprobes Naveen N. Rao
  2017-06-30  7:07 ` [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3 Nicholas Piggin
  7 siblings, 0 replies; 13+ messages in thread
From: Naveen N. Rao @ 2017-06-29 17:49 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nicholas Piggin, Ananth N Mavinakayanahalli, Masami Hiramatsu,
	Anton Blanchard, linuxppc-dev

Blacklist all functions involved while handling a trap. We:
- convert some of the symbols into private symbols, and
- blacklist most functions involved while handling a trap.

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/entry_64.S       | 35 ++++++++++++++++++++++-------------
 arch/powerpc/kernel/exceptions-64s.S |  2 ++
 arch/powerpc/kernel/traps.c          |  3 +++
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 5b919b2a62da..d1690ccfde0a 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -185,7 +185,7 @@ system_call:			/* label this so stack traces look sane */
 #ifdef CONFIG_PPC_BOOK3S
 	/* No MSR:RI on BookE */
 	andi.	r10,r8,MSR_RI
-	beq-	unrecov_restore
+	beq-	.Lunrecov_restore
 #endif
 
 /*
@@ -436,6 +436,7 @@ _GLOBAL(save_nvgprs)
 	clrrdi	r0,r11,1
 	std	r0,_TRAP(r1)
 	blr
+_ASM_NOKPROBE_SYMBOL(save_nvgprs);
 
 	
 /*
@@ -806,11 +807,11 @@ restore:
 	ld	r5,SOFTE(r1)
 	lbz	r6,PACASOFTIRQEN(r13)
 	cmpwi	cr0,r5,0
-	beq	restore_irq_off
+	beq	.Lrestore_irq_off
 
 	/* We are enabling, were we already enabled ? Yes, just return */
 	cmpwi	cr0,r6,1
-	beq	cr0,do_restore
+	beq	cr0,.Ldo_restore
 
 	/*
 	 * We are about to soft-enable interrupts (we are hard disabled
@@ -819,14 +820,14 @@ restore:
 	 */
 	lbz	r0,PACAIRQHAPPENED(r13)
 	cmpwi	cr0,r0,0
-	bne-	restore_check_irq_replay
+	bne-	.Lrestore_check_irq_replay
 
 	/*
 	 * Get here when nothing happened while soft-disabled, just
 	 * soft-enable and move-on. We will hard-enable as a side
 	 * effect of rfi
 	 */
-restore_no_replay:
+.Lrestore_no_replay:
 	TRACE_ENABLE_INTS
 	li	r0,1
 	stb	r0,PACASOFTIRQEN(r13);
@@ -834,7 +835,7 @@ restore_no_replay:
 	/*
 	 * Final return path. BookE is handled in a different file
 	 */
-do_restore:
+.Ldo_restore:
 #ifdef CONFIG_PPC_BOOK3E
 	b	exception_return_book3e
 #else
@@ -868,7 +869,7 @@ fast_exception_return:
 	REST_8GPRS(5, r1)
 
 	andi.	r0,r3,MSR_RI
-	beq-	unrecov_restore
+	beq-	.Lunrecov_restore
 
 	/* Load PPR from thread struct before we clear MSR:RI */
 BEGIN_FTR_SECTION
@@ -926,7 +927,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	 * make sure that in this case, we also clear PACA_IRQ_HARD_DIS
 	 * or that bit can get out of sync and bad things will happen
 	 */
-restore_irq_off:
+.Lrestore_irq_off:
 	ld	r3,_MSR(r1)
 	lbz	r7,PACAIRQHAPPENED(r13)
 	andi.	r0,r3,MSR_EE
@@ -936,13 +937,13 @@ restore_irq_off:
 1:	li	r0,0
 	stb	r0,PACASOFTIRQEN(r13);
 	TRACE_DISABLE_INTS
-	b	do_restore
+	b	.Ldo_restore
 
 	/*
 	 * Something did happen, check if a re-emit is needed
 	 * (this also clears paca->irq_happened)
 	 */
-restore_check_irq_replay:
+.Lrestore_check_irq_replay:
 	/* XXX: We could implement a fast path here where we check
 	 * for irq_happened being just 0x01, in which case we can
 	 * clear it and return. That means that we would potentially
@@ -952,7 +953,7 @@ restore_check_irq_replay:
 	 */
 	bl	__check_irq_replay
 	cmpwi	cr0,r3,0
- 	beq	restore_no_replay
+	beq	.Lrestore_no_replay
  
 	/*
 	 * We need to re-emit an interrupt. We do so by re-using our
@@ -1001,10 +1002,18 @@ restore_check_irq_replay:
 #endif /* CONFIG_PPC_DOORBELL */
 1:	b	ret_from_except /* What else to do here ? */
  
-unrecov_restore:
+.Lunrecov_restore:
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	unrecoverable_exception
-	b	unrecov_restore
+	b	.Lunrecov_restore
+
+_ASM_NOKPROBE_SYMBOL(ret_from_except);
+_ASM_NOKPROBE_SYMBOL(ret_from_except_lite);
+_ASM_NOKPROBE_SYMBOL(resume_kernel);
+_ASM_NOKPROBE_SYMBOL(fast_exc_return_irq);
+_ASM_NOKPROBE_SYMBOL(restore);
+_ASM_NOKPROBE_SYMBOL(fast_exception_return);
+
 
 #ifdef CONFIG_PPC_RTAS
 /*
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 3262e447b895..20318ceeea6a 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1591,6 +1591,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
 1:	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	kernel_bad_stack
 	b	1b
+_ASM_NOKPROBE_SYMBOL(bad_stack);
 
 /*
  * When doorbell is triggered from system reset wakeup, the message is
@@ -1652,3 +1653,4 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE)
 1:
 	blr
 
+_ASM_NOKPROBE_SYMBOL(__replay_interrupt)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index d4e545d27ef9..bfcfd9ef09f2 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -237,6 +237,7 @@ void die(const char *str, struct pt_regs *regs, long err)
 		err = 0;
 	oops_end(flags, regs, err);
 }
+NOKPROBE_SYMBOL(die);
 
 void user_single_step_siginfo(struct task_struct *tsk,
 				struct pt_regs *regs, siginfo_t *info)
@@ -1968,6 +1969,7 @@ void unrecoverable_exception(struct pt_regs *regs)
 	       regs->trap, regs->nip);
 	die("Unrecoverable exception", regs, SIGABRT);
 }
+NOKPROBE_SYMBOL(unrecoverable_exception);
 
 #if defined(CONFIG_BOOKE_WDT) || defined(CONFIG_40x)
 /*
@@ -1998,6 +2000,7 @@ void kernel_bad_stack(struct pt_regs *regs)
 	       regs->gpr[1], regs->nip);
 	die("Bad kernel stack pointer", regs, SIGABRT);
 }
+NOKPROBE_SYMBOL(kernel_bad_stack);
 
 void __init trap_init(void)
 {
-- 
2.13.1

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

* [PATCH v5 7/7] powerpc/64s: Blacklist rtas entry/exit from kprobes
  2017-06-29 17:49 [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3 Naveen N. Rao
                   ` (5 preceding siblings ...)
  2017-06-29 17:49 ` [PATCH v5 6/7] powerpc/64s: Blacklist functions invoked on a trap Naveen N. Rao
@ 2017-06-29 17:49 ` Naveen N. Rao
  2017-06-30  7:07 ` [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3 Nicholas Piggin
  7 siblings, 0 replies; 13+ messages in thread
From: Naveen N. Rao @ 2017-06-29 17:49 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nicholas Piggin, Ananth N Mavinakayanahalli, Masami Hiramatsu,
	Anton Blanchard, linuxppc-dev

We can't take traps with relocation off, so blacklist enter_rtas() and
rtas_return_loc(). However, instead of blacklisting all of enter_rtas(),
introduce a new symbol __enter_rtas from where on we can't take a trap
and blacklist that.

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

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index d1690ccfde0a..dc93452fb1fa 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -1090,6 +1090,8 @@ _GLOBAL(enter_rtas)
         rldicr  r9,r9,MSR_SF_LG,(63-MSR_SF_LG)
 	ori	r9,r9,MSR_IR|MSR_DR|MSR_FE0|MSR_FE1|MSR_FP|MSR_RI|MSR_LE
 	andc	r6,r0,r9
+
+__enter_rtas:
 	sync				/* disable interrupts so SRR0/1 */
 	mtmsrd	r0			/* don't get trashed */
 
@@ -1126,6 +1128,8 @@ rtas_return_loc:
 	mtspr	SPRN_SRR1,r4
 	rfid
 	b	.	/* prevent speculative execution */
+_ASM_NOKPROBE_SYMBOL(__enter_rtas)
+_ASM_NOKPROBE_SYMBOL(rtas_return_loc)
 
 	.align	3
 1:	.llong	rtas_restore_regs
-- 
2.13.1

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

* Re: [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3
  2017-06-29 17:49 [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3 Naveen N. Rao
                   ` (6 preceding siblings ...)
  2017-06-29 17:49 ` [PATCH v5 7/7] powerpc/64s: Blacklist rtas entry/exit from kprobes Naveen N. Rao
@ 2017-06-30  7:07 ` Nicholas Piggin
  7 siblings, 0 replies; 13+ messages in thread
From: Nicholas Piggin @ 2017-06-30  7:07 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: Michael Ellerman, Ananth N Mavinakayanahalli, Masami Hiramatsu,
	Anton Blanchard, linuxppc-dev

On Thu, 29 Jun 2017 23:19:13 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> This is the third in the series of patches to build out an appropriate
> kprobes blacklist for powerpc. Since posting the second series (*),
> there have been related changes to the code and I have brought that
> series forward to account for those changes. As such, all patches from
> the second series are included in this patchset.
> 
> This patchset now ensures that the newly added multiple kprobes test in
> the ftrace testsuite passes on powerpc64. Tested on both Elfv1 and
> Elfv2.
> 
> Changes since v4:
>   - Patch 5 changed to move system_call_exit() symbol before the mtmsrd,
>     along with an explanation for its placement.
>   - Patch 7 reverted to previous version moving the new symbol before
>     the mtmsrd as well.
>   - All other patches remain unchanged from v4.

These all look good to me now.

Thanks,
Nick

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

* Re: [PATCH v5 3/7] powerpc/64s: Blacklist system_call() and system_call_common() from kprobes
  2017-06-29 17:49 ` [PATCH v5 3/7] powerpc/64s: Blacklist system_call() and system_call_common() from kprobes Naveen N. Rao
@ 2017-07-01  2:24   ` Michael Ellerman
  2017-07-02 12:40     ` Nicholas Piggin
  0 siblings, 1 reply; 13+ messages in thread
From: Michael Ellerman @ 2017-07-01  2:24 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: Nicholas Piggin, Ananth N Mavinakayanahalli, Masami Hiramatsu,
	Anton Blanchard, linuxppc-dev

"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:

> Convert some of the symbols into private symbols and blacklist
> system_call_common() and system_call() from kprobes. We can't take a
> trap at parts of these functions as either MSR_RI is unset or the kernel
> stack pointer is not yet setup.
>
> Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/entry_64.S | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index da9486e2fd89..ef8e6615b8ba 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -52,12 +52,11 @@ exception_marker:
>  	.section	".text"
>  	.align 7
>  
> -	.globl system_call_common
> -system_call_common:
> +_GLOBAL(system_call_common)

Looks good.

Yet ...

Power6:

  [    0.313058] Bad kernel stack pointer 7fffdd061cf0 at c00000000000cd80
  [    0.313068] Oops: Bad kernel stack pointer, sig: 6 [#1]
  [    0.313072] SMP NR_CPUS=2048 
  [    0.313072] NUMA 
  [    0.313076] pSeries
  [    0.313081] Modules linked in:
  [    0.313087] CPU: 1 PID: 1 Comm: init Not tainted 4.12.0-rc3-gcc_ubuntu_be-g464970a #1
  [    0.313093] task: c000000049480000 task.stack: c0000000494c0000
  [    0.313097] NIP: c00000000000cd80 LR: 00007fff8e8a13b0 CTR: 00007fff8e8a1370
  [    0.313103] REGS: c000000007fa3d40 TRAP: 0300   Not tainted  (4.12.0-rc3-gcc_ubuntu_be-g464970a)
  [    0.313108] MSR: 8000000000001032 <SF,ME,IR,DR,RI>
  [    0.313114]   CR: 84000022  XER: 00000000
  [    0.313120] CFAR: 00000000000087c0 DAR: 0000000000001e64 DSISR: 42000000 SOFTE: 0 
  [    0.313120] GPR00: 000000000000002d 00007fffdd061cf0 00007fff8e8c8af8 0000000000000000 
  [    0.313120] GPR04: 00000000000019c0 00007fff8e860000 0000000000000001 00007fff8e8c2a30 
  [    0.313120] GPR08: 00000000493e1528 0000000000000000 b000000000001032 00007fff8e8a1ea8 
  [    0.313120] GPR12: 800000000000d032 c000000006b30500 0000000000000000 0000000000000000 
  [    0.313120] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
  [    0.313120] GPR20: 0000000000000000 0000000000000000 0000000000000080 0000000000010000 
  [    0.313120] GPR24: 0000000000000000 00007fffdd0626c9 00000000493a0040 0000000000000009 
  [    0.313120] GPR28: 00007fff8e8bfe58 00007fff8e8bfb88 00007fff8e8bfe40 00007fff8e8c0148 
  [    0.313190] NIP [c00000000000cd80] .load_up_vsx+0x1c/0x2c
  [    0.313195] LR [00007fff8e8a13b0] 0x7fff8e8a13b0
  [    0.313199] Call Trace:
  [    0.313201] Instruction dump:
  [    0.313206] 7fe721ce 10000604 38800200 7c0439ce 4e800020 71852000 41a2f7c5 75850200 
  [    0.313217] 41a2fd71 e88d0250 388419c0 38c00001 <90c404a4> 658c0080 f9810178 4bffefc4 
  [    0.313229] ---[ end trace 3ca7930ed36d9399 ]---

Power7:

  [    1.770801] Bad kernel stack pointer ff9be8b0 at 1f7f79590
  [    1.770811] Oops: Bad kernel stack pointer, sig: 6 [#1]
  [    1.770814] SMP NR_CPUS=2048 
  [    1.770815] NUMA 
  [    1.770819] pSeries
  [    1.770825] Modules linked in:
  [    1.770832] CPU: 26 PID: 1 Comm: init Not tainted 4.12.0-rc3-gcc6-g464970a #1
  [    1.770838] task: c000000e18300000 task.stack: c000000e18380000
  [    1.770843] NIP: 00000001f7f79590 LR: 00000000f7f79358 CTR: 00000001f7f79590
  [    1.770848] REGS: c00000000ed47d40 TRAP: 0400   Not tainted  (4.12.0-rc3-gcc6-g464970a)
  [    1.770853] MSR: 8000000040001032 <SF,ME,IR,DR,RI>
  [    1.770860]   CR: 24000888  XER: 20000000
  [    1.770866] CFAR: 00000000000087c0 SOFTE: 0 
  [    1.770866] GPR00: 00000000f7f7998c 00000000ff9be8b0 0000000000000000 0000000000000000 
  [    1.770866] GPR04: 00000000f7f9f5b8 0000000000000005 0000000000000072 0000000000000072 
  [    1.770866] GPR08: fffffffffeff0000 0000000000000000 00000001f7f79590 00000000f7f7ac30 
  [    1.770866] GPR12: 0000000000000001 c00000000ec08200 0000000000010000 0000000000000000 
  [    1.770866] GPR16: 00000000ff9bed49 0000000000000000 00000000dc0065c2 0000000000000000 
  [    1.770866] GPR20: 0000000020000000 0000000000000064 0000000000000001 0000000000000001 
  [    1.770866] GPR24: 0000000000000001 0000000000000001 00000000f7f62b40 0000000020620034 
  [    1.770866] GPR28: 000000000000000a 00000000ff9bed49 00000000f7f9fff4 00000000f7f9f308 
  [    1.770929] NIP [00000001f7f79590] 0x1f7f79590
  [    1.770933] LR [00000000f7f79358] 0xf7f79358
  [    1.770937] Call Trace:
  [    1.770940] Instruction dump:
  [    1.770945] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX 
  [    1.770953] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX 
  [    1.770964] ---[ end trace 4eb52706b24fb9c6 ]---

Cell:

  [   21.359058] init[1]: unhandled signal 11 at 00000000 nip 00000000 lr c00000000000b01c code 30001
  [   21.412082] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
  [   21.412082] 
  [   21.466866] CPU: 1 PID: 1 Comm: init Not tainted 4.12.0-rc3-gcc6x-g464970a #1
  [   21.509619] Call Trace:
  [   21.524265] [c0000007fb9838d0] [c000000000440a28] .dump_stack+0xa8/0xe8 (unreliable)
  [   21.570715] [c0000007fb983960] [c0000000001d0258] .panic+0x13c/0x2ec
  [   21.608787] [c0000007fb983a10] [c0000000000a59c0] .do_exit+0xb88/0xb90
  [   21.647945] [c0000007fb983ae0] [c0000000000a6cd4] .do_group_exit+0x64/0x100
  [   21.689666] [c0000007fb983b70] [c0000000000b5dfc] .get_signal+0x3bc/0x748
  [   21.730338] [c0000007fb983c70] [c00000000001742c] .do_signal+0x64/0x2b0
  [   21.769969] [c0000007fb983db0] [c000000000017870] .do_notify_resume+0xc0/0xe8
  [   21.812733] [c0000007fb983e30] [c00000000000abb0] .ret_from_except_lite+0x5c/0x60
  [   21.857649] Rebooting in 180 seconds..

And G5 too, but I don't have the trace.

Strangely it doesn't break on all my BE systems. I suspect AIL=1 avoids it.

One to investigate on Monday :)

cheers

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

* Re: [PATCH v5 3/7] powerpc/64s: Blacklist system_call() and system_call_common() from kprobes
  2017-07-01  2:24   ` Michael Ellerman
@ 2017-07-02 12:40     ` Nicholas Piggin
  2017-07-02 17:25       ` Naveen N. Rao
  0 siblings, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2017-07-02 12:40 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Naveen N. Rao, Ananth N Mavinakayanahalli, Masami Hiramatsu,
	Anton Blanchard, linuxppc-dev

On Sat, 01 Jul 2017 12:24:02 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:

> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> 
> > Convert some of the symbols into private symbols and blacklist
> > system_call_common() and system_call() from kprobes. We can't take a
> > trap at parts of these functions as either MSR_RI is unset or the kernel
> > stack pointer is not yet setup.
> >
> > Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
> > Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > ---
> >  arch/powerpc/kernel/entry_64.S | 29 +++++++++++++++--------------
> >  1 file changed, 15 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> > index da9486e2fd89..ef8e6615b8ba 100644
> > --- a/arch/powerpc/kernel/entry_64.S
> > +++ b/arch/powerpc/kernel/entry_64.S
> > @@ -52,12 +52,11 @@ exception_marker:
> >  	.section	".text"
> >  	.align 7
> >  
> > -	.globl system_call_common
> > -system_call_common:
> > +_GLOBAL(system_call_common)  
> 
> Looks good.
> 
> Yet ...
> 
> Power6:
> 
>   [    0.313058] Bad kernel stack pointer 7fffdd061cf0 at c00000000000cd80
>   [    0.313068] Oops: Bad kernel stack pointer, sig: 6 [#1]
>   [    0.313072] SMP NR_CPUS=2048 
>   [    0.313072] NUMA 
>   [    0.313076] pSeries
>   [    0.313081] Modules linked in:
>   [    0.313087] CPU: 1 PID: 1 Comm: init Not tainted 4.12.0-rc3-gcc_ubuntu_be-g464970a #1
>   [    0.313093] task: c000000049480000 task.stack: c0000000494c0000
>   [    0.313097] NIP: c00000000000cd80 LR: 00007fff8e8a13b0 CTR: 00007fff8e8a1370
>   [    0.313103] REGS: c000000007fa3d40 TRAP: 0300   Not tainted  (4.12.0-rc3-gcc_ubuntu_be-g464970a)
>   [    0.313108] MSR: 8000000000001032 <SF,ME,IR,DR,RI>
>   [    0.313114]   CR: 84000022  XER: 00000000
>   [    0.313120] CFAR: 00000000000087c0 DAR: 0000000000001e64 DSISR: 42000000 SOFTE: 0 
>   [    0.313120] GPR00: 000000000000002d 00007fffdd061cf0 00007fff8e8c8af8 0000000000000000 
>   [    0.313120] GPR04: 00000000000019c0 00007fff8e860000 0000000000000001 00007fff8e8c2a30 
>   [    0.313120] GPR08: 00000000493e1528 0000000000000000 b000000000001032 00007fff8e8a1ea8 
>   [    0.313120] GPR12: 800000000000d032 c000000006b30500 0000000000000000 0000000000000000 
>   [    0.313120] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
>   [    0.313120] GPR20: 0000000000000000 0000000000000000 0000000000000080 0000000000010000 
>   [    0.313120] GPR24: 0000000000000000 00007fffdd0626c9 00000000493a0040 0000000000000009 
>   [    0.313120] GPR28: 00007fff8e8bfe58 00007fff8e8bfb88 00007fff8e8bfe40 00007fff8e8c0148 
>   [    0.313190] NIP [c00000000000cd80] .load_up_vsx+0x1c/0x2c
>   [    0.313195] LR [00007fff8e8a13b0] 0x7fff8e8a13b0
>   [    0.313199] Call Trace:
>   [    0.313201] Instruction dump:
>   [    0.313206] 7fe721ce 10000604 38800200 7c0439ce 4e800020 71852000 41a2f7c5 75850200 
>   [    0.313217] 41a2fd71 e88d0250 388419c0 38c00001 <90c404a4> 658c0080 f9810178 4bffefc4 
>   [    0.313229] ---[ end trace 3ca7930ed36d9399 ]---
> 
> Power7:
> 
>   [    1.770801] Bad kernel stack pointer ff9be8b0 at 1f7f79590
>   [    1.770811] Oops: Bad kernel stack pointer, sig: 6 [#1]
>   [    1.770814] SMP NR_CPUS=2048 
>   [    1.770815] NUMA 
>   [    1.770819] pSeries
>   [    1.770825] Modules linked in:
>   [    1.770832] CPU: 26 PID: 1 Comm: init Not tainted 4.12.0-rc3-gcc6-g464970a #1
>   [    1.770838] task: c000000e18300000 task.stack: c000000e18380000
>   [    1.770843] NIP: 00000001f7f79590 LR: 00000000f7f79358 CTR: 00000001f7f79590
>   [    1.770848] REGS: c00000000ed47d40 TRAP: 0400   Not tainted  (4.12.0-rc3-gcc6-g464970a)
>   [    1.770853] MSR: 8000000040001032 <SF,ME,IR,DR,RI>
>   [    1.770860]   CR: 24000888  XER: 20000000
>   [    1.770866] CFAR: 00000000000087c0 SOFTE: 0 
>   [    1.770866] GPR00: 00000000f7f7998c 00000000ff9be8b0 0000000000000000 0000000000000000 
>   [    1.770866] GPR04: 00000000f7f9f5b8 0000000000000005 0000000000000072 0000000000000072 
>   [    1.770866] GPR08: fffffffffeff0000 0000000000000000 00000001f7f79590 00000000f7f7ac30 
>   [    1.770866] GPR12: 0000000000000001 c00000000ec08200 0000000000010000 0000000000000000 
>   [    1.770866] GPR16: 00000000ff9bed49 0000000000000000 00000000dc0065c2 0000000000000000 
>   [    1.770866] GPR20: 0000000020000000 0000000000000064 0000000000000001 0000000000000001 
>   [    1.770866] GPR24: 0000000000000001 0000000000000001 00000000f7f62b40 0000000020620034 
>   [    1.770866] GPR28: 000000000000000a 00000000ff9bed49 00000000f7f9fff4 00000000f7f9f308 
>   [    1.770929] NIP [00000001f7f79590] 0x1f7f79590
>   [    1.770933] LR [00000000f7f79358] 0xf7f79358
>   [    1.770937] Call Trace:
>   [    1.770940] Instruction dump:
>   [    1.770945] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX 
>   [    1.770953] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX 
>   [    1.770964] ---[ end trace 4eb52706b24fb9c6 ]---
> 
> Cell:
> 
>   [   21.359058] init[1]: unhandled signal 11 at 00000000 nip 00000000 lr c00000000000b01c code 30001
>   [   21.412082] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>   [   21.412082] 
>   [   21.466866] CPU: 1 PID: 1 Comm: init Not tainted 4.12.0-rc3-gcc6x-g464970a #1
>   [   21.509619] Call Trace:
>   [   21.524265] [c0000007fb9838d0] [c000000000440a28] .dump_stack+0xa8/0xe8 (unreliable)
>   [   21.570715] [c0000007fb983960] [c0000000001d0258] .panic+0x13c/0x2ec
>   [   21.608787] [c0000007fb983a10] [c0000000000a59c0] .do_exit+0xb88/0xb90
>   [   21.647945] [c0000007fb983ae0] [c0000000000a6cd4] .do_group_exit+0x64/0x100
>   [   21.689666] [c0000007fb983b70] [c0000000000b5dfc] .get_signal+0x3bc/0x748
>   [   21.730338] [c0000007fb983c70] [c00000000001742c] .do_signal+0x64/0x2b0
>   [   21.769969] [c0000007fb983db0] [c000000000017870] .do_notify_resume+0xc0/0xe8
>   [   21.812733] [c0000007fb983e30] [c00000000000abb0] .ret_from_except_lite+0x5c/0x60
>   [   21.857649] Rebooting in 180 seconds..
> 
> And G5 too, but I don't have the trace.
> 
> Strangely it doesn't break on all my BE systems. I suspect AIL=1 avoids it.
> 
> One to investigate on Monday :)

  .type system_call_common,@function

Seems to make the linker resolve system_call_common to address of its
function descriptor. I guess the load handler macros should be using
DOTSYM()?

Thanks,
Nick

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

* Re: [PATCH v5 3/7] powerpc/64s: Blacklist system_call() and system_call_common() from kprobes
  2017-07-02 12:40     ` Nicholas Piggin
@ 2017-07-02 17:25       ` Naveen N. Rao
  0 siblings, 0 replies; 13+ messages in thread
From: Naveen N. Rao @ 2017-07-02 17:25 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Michael Ellerman, Ananth N Mavinakayanahalli, Masami Hiramatsu,
	Anton Blanchard, linuxppc-dev

On 2017/07/02 10:40PM, Nicholas Piggin wrote:
> On Sat, 01 Jul 2017 12:24:02 +1000
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> > 
> > > Convert some of the symbols into private symbols and blacklist
> > > system_call_common() and system_call() from kprobes. We can't take a
> > > trap at parts of these functions as either MSR_RI is unset or the kernel
> > > stack pointer is not yet setup.
> > >
> > > Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> > > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > > ---
> > >  arch/powerpc/kernel/entry_64.S | 29 +++++++++++++++--------------
> > >  1 file changed, 15 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> > > index da9486e2fd89..ef8e6615b8ba 100644
> > > --- a/arch/powerpc/kernel/entry_64.S
> > > +++ b/arch/powerpc/kernel/entry_64.S
> > > @@ -52,12 +52,11 @@ exception_marker:
> > >  	.section	".text"
> > >  	.align 7
> > >  
> > > -	.globl system_call_common
> > > -system_call_common:
> > > +_GLOBAL(system_call_common)  
> > 
> > Looks good.
> > 
> > Yet ...
> > 
> > Power6:
> > 
> >   [    0.313058] Bad kernel stack pointer 7fffdd061cf0 at c00000000000cd80
> >   [    0.313068] Oops: Bad kernel stack pointer, sig: 6 [#1]
> >   [    0.313072] SMP NR_CPUS=2048 
> >   [    0.313072] NUMA 
> >   [    0.313076] pSeries
> >   [    0.313081] Modules linked in:
> >   [    0.313087] CPU: 1 PID: 1 Comm: init Not tainted 4.12.0-rc3-gcc_ubuntu_be-g464970a #1
> >   [    0.313093] task: c000000049480000 task.stack: c0000000494c0000
> >   [    0.313097] NIP: c00000000000cd80 LR: 00007fff8e8a13b0 CTR: 00007fff8e8a1370
> >   [    0.313103] REGS: c000000007fa3d40 TRAP: 0300   Not tainted  (4.12.0-rc3-gcc_ubuntu_be-g464970a)
> >   [    0.313108] MSR: 8000000000001032 <SF,ME,IR,DR,RI>
> >   [    0.313114]   CR: 84000022  XER: 00000000
> >   [    0.313120] CFAR: 00000000000087c0 DAR: 0000000000001e64 DSISR: 42000000 SOFTE: 0 
> >   [    0.313120] GPR00: 000000000000002d 00007fffdd061cf0 00007fff8e8c8af8 0000000000000000 
> >   [    0.313120] GPR04: 00000000000019c0 00007fff8e860000 0000000000000001 00007fff8e8c2a30 
> >   [    0.313120] GPR08: 00000000493e1528 0000000000000000 b000000000001032 00007fff8e8a1ea8 
> >   [    0.313120] GPR12: 800000000000d032 c000000006b30500 0000000000000000 0000000000000000 
> >   [    0.313120] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
> >   [    0.313120] GPR20: 0000000000000000 0000000000000000 0000000000000080 0000000000010000 
> >   [    0.313120] GPR24: 0000000000000000 00007fffdd0626c9 00000000493a0040 0000000000000009 
> >   [    0.313120] GPR28: 00007fff8e8bfe58 00007fff8e8bfb88 00007fff8e8bfe40 00007fff8e8c0148 
> >   [    0.313190] NIP [c00000000000cd80] .load_up_vsx+0x1c/0x2c
> >   [    0.313195] LR [00007fff8e8a13b0] 0x7fff8e8a13b0
> >   [    0.313199] Call Trace:
> >   [    0.313201] Instruction dump:
> >   [    0.313206] 7fe721ce 10000604 38800200 7c0439ce 4e800020 71852000 41a2f7c5 75850200 
> >   [    0.313217] 41a2fd71 e88d0250 388419c0 38c00001 <90c404a4> 658c0080 f9810178 4bffefc4 
> >   [    0.313229] ---[ end trace 3ca7930ed36d9399 ]---
> > 
> > Power7:
> > 
> >   [    1.770801] Bad kernel stack pointer ff9be8b0 at 1f7f79590
> >   [    1.770811] Oops: Bad kernel stack pointer, sig: 6 [#1]
> >   [    1.770814] SMP NR_CPUS=2048 
> >   [    1.770815] NUMA 
> >   [    1.770819] pSeries
> >   [    1.770825] Modules linked in:
> >   [    1.770832] CPU: 26 PID: 1 Comm: init Not tainted 4.12.0-rc3-gcc6-g464970a #1
> >   [    1.770838] task: c000000e18300000 task.stack: c000000e18380000
> >   [    1.770843] NIP: 00000001f7f79590 LR: 00000000f7f79358 CTR: 00000001f7f79590
> >   [    1.770848] REGS: c00000000ed47d40 TRAP: 0400   Not tainted  (4.12.0-rc3-gcc6-g464970a)
> >   [    1.770853] MSR: 8000000040001032 <SF,ME,IR,DR,RI>
> >   [    1.770860]   CR: 24000888  XER: 20000000
> >   [    1.770866] CFAR: 00000000000087c0 SOFTE: 0 
> >   [    1.770866] GPR00: 00000000f7f7998c 00000000ff9be8b0 0000000000000000 0000000000000000 
> >   [    1.770866] GPR04: 00000000f7f9f5b8 0000000000000005 0000000000000072 0000000000000072 
> >   [    1.770866] GPR08: fffffffffeff0000 0000000000000000 00000001f7f79590 00000000f7f7ac30 
> >   [    1.770866] GPR12: 0000000000000001 c00000000ec08200 0000000000010000 0000000000000000 
> >   [    1.770866] GPR16: 00000000ff9bed49 0000000000000000 00000000dc0065c2 0000000000000000 
> >   [    1.770866] GPR20: 0000000020000000 0000000000000064 0000000000000001 0000000000000001 
> >   [    1.770866] GPR24: 0000000000000001 0000000000000001 00000000f7f62b40 0000000020620034 
> >   [    1.770866] GPR28: 000000000000000a 00000000ff9bed49 00000000f7f9fff4 00000000f7f9f308 
> >   [    1.770929] NIP [00000001f7f79590] 0x1f7f79590
> >   [    1.770933] LR [00000000f7f79358] 0xf7f79358
> >   [    1.770937] Call Trace:
> >   [    1.770940] Instruction dump:
> >   [    1.770945] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX 
> >   [    1.770953] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX 
> >   [    1.770964] ---[ end trace 4eb52706b24fb9c6 ]---
> > 
> > Cell:
> > 
> >   [   21.359058] init[1]: unhandled signal 11 at 00000000 nip 00000000 lr c00000000000b01c code 30001
> >   [   21.412082] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> >   [   21.412082] 
> >   [   21.466866] CPU: 1 PID: 1 Comm: init Not tainted 4.12.0-rc3-gcc6x-g464970a #1
> >   [   21.509619] Call Trace:
> >   [   21.524265] [c0000007fb9838d0] [c000000000440a28] .dump_stack+0xa8/0xe8 (unreliable)
> >   [   21.570715] [c0000007fb983960] [c0000000001d0258] .panic+0x13c/0x2ec
> >   [   21.608787] [c0000007fb983a10] [c0000000000a59c0] .do_exit+0xb88/0xb90
> >   [   21.647945] [c0000007fb983ae0] [c0000000000a6cd4] .do_group_exit+0x64/0x100
> >   [   21.689666] [c0000007fb983b70] [c0000000000b5dfc] .get_signal+0x3bc/0x748
> >   [   21.730338] [c0000007fb983c70] [c00000000001742c] .do_signal+0x64/0x2b0
> >   [   21.769969] [c0000007fb983db0] [c000000000017870] .do_notify_resume+0xc0/0xe8
> >   [   21.812733] [c0000007fb983e30] [c00000000000abb0] .ret_from_except_lite+0x5c/0x60
> >   [   21.857649] Rebooting in 180 seconds..
> > 
> > And G5 too, but I don't have the trace.
> > 
> > Strangely it doesn't break on all my BE systems. I suspect AIL=1 avoids it.
> > 
> > One to investigate on Monday :)

Ugh! I am able to reproduce with CONFIG_RELOCATABLE...

> 
>   .type system_call_common,@function
> 
> Seems to make the linker resolve system_call_common to address of its
> function descriptor. I guess the load handler macros should be using
> DOTSYM()?

Thanks for looking into this, Nick. That does seem to be the problem.  
The below patch fixes it for me:

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 20318ceeea6a..0b64c543cdc7 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -862,7 +862,7 @@ EXC_COMMON(trap_0b_common, 0xb00, unknown_exception)
 #endif

 #define LOAD_SYSCALL_HANDLER(reg)                                      \
-       __LOAD_HANDLER(reg, system_call_common)
+       __LOAD_HANDLER(reg, DOTSYM(system_call_common))

 #define SYSCALL_FASTENDIAN_TEST                                        \
 BEGIN_FTR_SECTION                                              \


We can't convert all of __LOAD_HANDLER() since others' use assembly 
symbols defined through EXC_COMMON_BEGIN(), which doesn't have the 
function annotation.

Michael,
Let me know how you want to proceed with this. We can drop the 
conversion to _GLOBAL() from this series, or I can put out a separate 
patch to do just that (including the above hunk) and re-send.

Thanks,
Naveen

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

* Re: [v5, 1/7] powerpc64/elfv1: Only dereference function descriptor for non-text symbols
  2017-06-29 17:49 ` [PATCH v5 1/7] powerpc64/elfv1: Only dereference function descriptor for non-text symbols Naveen N. Rao
@ 2017-07-04 10:48   ` Michael Ellerman
  0 siblings, 0 replies; 13+ messages in thread
From: Michael Ellerman @ 2017-07-04 10:48 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: linuxppc-dev, Masami Hiramatsu, Nicholas Piggin, Anton Blanchard

On Thu, 2017-06-29 at 17:49:14 UTC, "Naveen N. Rao" wrote:
> Currently, we assume that the function pointer we receive in
> ppc_function_entry() points to a function descriptor. However, this is
> not always the case. In particular, assembly symbols without the right
> annotation do not have an associated function descriptor. Some of these
> symbols are added to the kprobe blacklist using _ASM_NOKPROBE_SYMBOL().
> 
> When such addresses are subsequently processed through
> arch_deref_entry_point() in populate_kprobe_blacklist(), we see the
> below errors during bootup:
>     [    0.663963] Failed to find blacklist at 7d9b02a648029b6c
>     [    0.663970] Failed to find blacklist at a14d03d0394a0001
>     [    0.663972] Failed to find blacklist at 7d5302a6f94d0388
>     [    0.663973] Failed to find blacklist at 48027d11e8610178
>     [    0.663974] Failed to find blacklist at f8010070f8410080
>     [    0.663976] Failed to find blacklist at 386100704801f89d
>     [    0.663977] Failed to find blacklist at 7d5302a6f94d00b0
> 
> Fix this by checking if the function pointer we receive in
> ppc_function_entry() already points to kernel text. If so, we just
> return it as is. If not, we assume that this is a function descriptor
> and proceed to dereference it.
> 
> Suggested-by: Nicholas Piggin <npiggin@gmail.com>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/83e840c770f2c578bbbff478d62a44

cheers

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

end of thread, other threads:[~2017-07-04 10:48 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-29 17:49 [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3 Naveen N. Rao
2017-06-29 17:49 ` [PATCH v5 1/7] powerpc64/elfv1: Only dereference function descriptor for non-text symbols Naveen N. Rao
2017-07-04 10:48   ` [v5, " Michael Ellerman
2017-06-29 17:49 ` [PATCH v5 2/7] powerpc/64s: Convert .L__replay_interrupt_return to a local label Naveen N. Rao
2017-06-29 17:49 ` [PATCH v5 3/7] powerpc/64s: Blacklist system_call() and system_call_common() from kprobes Naveen N. Rao
2017-07-01  2:24   ` Michael Ellerman
2017-07-02 12:40     ` Nicholas Piggin
2017-07-02 17:25       ` Naveen N. Rao
2017-06-29 17:49 ` [PATCH v5 4/7] powerpc/64s: Move system_call() symbol to just after setting MSR_EE Naveen N. Rao
2017-06-29 17:49 ` [PATCH v5 5/7] powerpc/64s: Un-blacklist system_call() from kprobes Naveen N. Rao
2017-06-29 17:49 ` [PATCH v5 6/7] powerpc/64s: Blacklist functions invoked on a trap Naveen N. Rao
2017-06-29 17:49 ` [PATCH v5 7/7] powerpc/64s: Blacklist rtas entry/exit from kprobes Naveen N. Rao
2017-06-30  7:07 ` [PATCH v5 0/7] powerpc: build out kprobes blacklist -- series 3 Nicholas Piggin

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