[PATCH v3 3/6] powerpc/64s: Blacklist system_call() and system_call_common() from kprobes
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 Signed-off-by: Naveen N. Rao --- 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 cmpdr0,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: orisr5,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 addir3,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
Re: [PATCH v3 3/6] powerpc/64s: Blacklist system_call() and system_call_common() from kprobes
On Thu, 22 Jun 2017 00:08:39 +0530 "Naveen N. Rao" wrote: > 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 > Signed-off-by: Naveen N. Rao I don't have a problem with this bunch of system call labels going private. They've never added much for me in profiles. Reviewed-by: Nicholas Piggin Semi-related question, why is system_call: where it is? Should we move it up to right after the mtmsrd / wrteei instruction? (obviously for another patch). It's pretty common to get PMU interrupts coming in right after mtmsr and this makes profiles split the syscall into two which is annoying. Thanks, Nick
Re: [PATCH v3 3/6] powerpc/64s: Blacklist system_call() and system_call_common() from kprobes
Nicholas Piggin writes: > On Thu, 22 Jun 2017 00:08:39 +0530 > "Naveen N. Rao" wrote: > >> 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 >> Signed-off-by: Naveen N. Rao > > I don't have a problem with this bunch of system call labels > going private. They've never added much for me in profiles. > > Reviewed-by: Nicholas Piggin > > Semi-related question, why is system_call: where it is? Ancient history. We used to have: bne syscall_dotrace syscall_dotrace_cont: cmpldi 0,r0,NR_syscalls bge-syscall_enosys system_call:/* label this so stack traces look sane */ So it was there to hide syscall_dotrace_cont from back traces. But we made syscall_dotrace_cont local in 2012 and then removed it entirely in 2015. > Should we move it up to right after the mtmsrd / wrteei instruction? > (obviously for another patch). It's pretty common to get PMU > interrupts coming in right after mtmsr and this makes profiles split > the syscall into two which is annoying. Move it wherever makes sense and gives good back traces. cheers
Re: [PATCH v3 3/6] powerpc/64s: Blacklist system_call() and system_call_common() from kprobes
On Thu, 22 Jun 2017 21:07:46 +1000 Michael Ellerman wrote: > Nicholas Piggin writes: > > > On Thu, 22 Jun 2017 00:08:39 +0530 > > "Naveen N. Rao" wrote: > > > >> 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 > >> Signed-off-by: Naveen N. Rao > > > > I don't have a problem with this bunch of system call labels > > going private. They've never added much for me in profiles. > > > > Reviewed-by: Nicholas Piggin > > > > Semi-related question, why is system_call: where it is? > > Ancient history. > > We used to have: > > bne syscall_dotrace > syscall_dotrace_cont: > cmpldi 0,r0,NR_syscalls > bge-syscall_enosys > > system_call: /* label this so stack traces look sane */ > > > So it was there to hide syscall_dotrace_cont from back traces. > > But we made syscall_dotrace_cont local in 2012 and then removed it > entirely in 2015. > > > Should we move it up to right after the mtmsrd / wrteei instruction? > > (obviously for another patch). It's pretty common to get PMU > > interrupts coming in right after mtmsr and this makes profiles split > > the syscall into two which is annoying. > > Move it wherever makes sense and gives good back traces. I'd be in favour of moving it to right after the interurpt enable. I suppose you'd want a separate patch for that though. But we could put it in this series since we're changing a lot of labels. Thanks, Nick
Re: [PATCH v3 3/6] powerpc/64s: Blacklist system_call() and system_call_common() from kprobes
On 2017/06/22 11:08PM, Nicholas Piggin wrote: > On Thu, 22 Jun 2017 21:07:46 +1000 > Michael Ellerman wrote: > > > Nicholas Piggin writes: > > > > > On Thu, 22 Jun 2017 00:08:39 +0530 > > > "Naveen N. Rao" wrote: > > > > > >> 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 > > >> Signed-off-by: Naveen N. Rao > > > > > > I don't have a problem with this bunch of system call labels > > > going private. They've never added much for me in profiles. > > > > > > Reviewed-by: Nicholas Piggin Thanks for the review. > > > > > > Semi-related question, why is system_call: where it is? > > > > Ancient history. > > > > We used to have: > > > > bne syscall_dotrace > > syscall_dotrace_cont: > > cmpldi 0,r0,NR_syscalls > > bge-syscall_enosys > > > > system_call:/* label this so stack traces look sane > > */ > > > > > > So it was there to hide syscall_dotrace_cont from back traces. > > > > But we made syscall_dotrace_cont local in 2012 and then removed it > > entirely in 2015. > > > > > Should we move it up to right after the mtmsrd / wrteei instruction? > > > (obviously for another patch). It's pretty common to get PMU > > > interrupts coming in right after mtmsr and this makes profiles split > > > the syscall into two which is annoying. > > > > Move it wherever makes sense and gives good back traces. > > I'd be in favour of moving it to right after the interurpt enable. > I suppose you'd want a separate patch for that though. But we could > put it in this series since we're changing a lot of labels. Sure, I'll add a patch that does that. - Naveen
