lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1618900760.son2fwciv1.naveen@linux.ibm.com>
Date:   Tue, 20 Apr 2021 12:21:39 +0530
From:   "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
To:     Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Christophe Leroy <christophe.leroy@...roup.eu>,
        Michael Ellerman <mpe@...erman.id.au>,
        Paul Mackerras <paulus@...ba.org>
Cc:     linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v1 2/2] powerpc: Enable OPTPROBES on PPC32

Christophe Leroy wrote:
> For that, create a 32 bits version of patch_imm64_load_insns()
> and create a patch_imm_load_insns() which calls
> patch_imm32_load_insns() on PPC32 and patch_imm64_load_insns()
> on PPC64.
> 
> Adapt optprobes_head.S for PPC32. Use PPC_LL/PPC_STL macros instead
> of raw ld/std, opt out things linked to paca and use stmw/lmw to
> save/restore registers.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
> ---
>  arch/powerpc/Kconfig                 |  2 +-
>  arch/powerpc/kernel/optprobes.c      | 24 +++++++++++++--
>  arch/powerpc/kernel/optprobes_head.S | 46 +++++++++++++++++++---------
>  3 files changed, 53 insertions(+), 19 deletions(-)

Thanks for adding support for ppc32. It is good to see that it works 
without too many changes.

> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index c1344c05226c..49b538e54efb 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -227,7 +227,7 @@ config PPC
>  	select HAVE_MOD_ARCH_SPECIFIC
>  	select HAVE_NMI				if PERF_EVENTS || (PPC64 && PPC_BOOK3S)
>  	select HAVE_HARDLOCKUP_DETECTOR_ARCH	if PPC64 && PPC_BOOK3S && SMP
> -	select HAVE_OPTPROBES			if PPC64
> +	select HAVE_OPTPROBES
>  	select HAVE_PERF_EVENTS
>  	select HAVE_PERF_EVENTS_NMI		if PPC64
>  	select HAVE_HARDLOCKUP_DETECTOR_PERF	if PERF_EVENTS && HAVE_PERF_EVENTS_NMI && !HAVE_HARDLOCKUP_DETECTOR_ARCH
> diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
> index 58fdb9f66e0f..cdf87086fa33 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -141,11 +141,21 @@ void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
>  	}
>  }
> 
> +static void patch_imm32_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
> +{
> +	patch_instruction((struct ppc_inst *)addr,
> +			  ppc_inst(PPC_RAW_LIS(reg, IMM_H(val))));
> +	addr++;
> +
> +	patch_instruction((struct ppc_inst *)addr,
> +			  ppc_inst(PPC_RAW_ORI(reg, reg, IMM_L(val))));
> +}
> +
>  /*
>   * Generate instructions to load provided immediate 64-bit value
>   * to register 'reg' and patch these instructions at 'addr'.
>   */
> -static void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
> +static void patch_imm64_load_insns(unsigned long long val, int reg, kprobe_opcode_t *addr)

Do you really need this?

>  {
>  	/* lis reg,(op)@highest */
>  	patch_instruction((struct ppc_inst *)addr,
> @@ -177,6 +187,14 @@ static void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *
>  				   ___PPC_RS(reg) | (val & 0xffff)));
>  }
> 
> +static void patch_imm_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
> +{
> +	if (IS_ENABLED(CONFIG_PPC64))
> +		patch_imm64_load_insns(val, reg, addr);
> +	else
> +		patch_imm32_load_insns(val, reg, addr);
> +}
> +
>  int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
>  {
>  	struct ppc_inst branch_op_callback, branch_emulate_step, temp;
> @@ -230,7 +248,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
>  	 * Fixup the template with instructions to:
>  	 * 1. load the address of the actual probepoint
>  	 */
> -	patch_imm64_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
> +	patch_imm_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
> 
>  	/*
>  	 * 2. branch to optimized_callback() and emulate_step()
> @@ -264,7 +282,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
>  	 * 3. load instruction to be emulated into relevant register, and
>  	 */
>  	temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
> -	patch_imm64_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
> +	patch_imm_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
> 
>  	/*
>  	 * 4. branch back from trampoline
> diff --git a/arch/powerpc/kernel/optprobes_head.S b/arch/powerpc/kernel/optprobes_head.S
> index ff8ba4d3824d..49f31e554573 100644
> --- a/arch/powerpc/kernel/optprobes_head.S
> +++ b/arch/powerpc/kernel/optprobes_head.S
> @@ -30,39 +30,47 @@ optinsn_slot:
>  	.global optprobe_template_entry
>  optprobe_template_entry:
>  	/* Create an in-memory pt_regs */
> -	stdu	r1,-INT_FRAME_SIZE(r1)
> +	PPC_STLU	r1,-INT_FRAME_SIZE(r1)
>  	SAVE_GPR(0,r1)
>  	/* Save the previous SP into stack */
>  	addi	r0,r1,INT_FRAME_SIZE
> -	std	r0,GPR1(r1)
> +	PPC_STL	r0,GPR1(r1)
> +#ifdef CONFIG_PPC64
>  	SAVE_10GPRS(2,r1)
>  	SAVE_10GPRS(12,r1)
>  	SAVE_10GPRS(22,r1)
> +#else
> +	stmw	r2, GPR2(r1)
> +#endif
>  	/* Save SPRS */
>  	mfmsr	r5
> -	std	r5,_MSR(r1)
> +	PPC_STL	r5,_MSR(r1)
>  	li	r5,0x700
> -	std	r5,_TRAP(r1)
> +	PPC_STL	r5,_TRAP(r1)
>  	li	r5,0
> -	std	r5,ORIG_GPR3(r1)
> -	std	r5,RESULT(r1)
> +	PPC_STL	r5,ORIG_GPR3(r1)
> +	PPC_STL	r5,RESULT(r1)
>  	mfctr	r5
> -	std	r5,_CTR(r1)
> +	PPC_STL	r5,_CTR(r1)
>  	mflr	r5
> -	std	r5,_LINK(r1)
> +	PPC_STL	r5,_LINK(r1)
>  	mfspr	r5,SPRN_XER
> -	std	r5,_XER(r1)
> +	PPC_STL	r5,_XER(r1)
>  	mfcr	r5
> -	std	r5,_CCR(r1)
> +	PPC_STL	r5,_CCR(r1)
> +#ifdef CONFIG_PPC64
>  	lbz     r5,PACAIRQSOFTMASK(r13)
>  	std     r5,SOFTE(r1)
> +#endif
> 
>  	/*
>  	 * We may get here from a module, so load the kernel TOC in r2.
>  	 * The original TOC gets restored when pt_regs is restored
>  	 * further below.
>  	 */
> +#ifdef CONFIG_PPC64
>  	ld	r2,PACATOC(r13)
> +#endif

Are the ISA and ABI documents for ppc32 available publicly? I would have 
thought that the TOC issues apply to ppc32 as well, but want to 
understand why this isn't a problem there.

> 
>  	.global optprobe_template_op_address
>  optprobe_template_op_address:
> @@ -72,9 +80,11 @@ optprobe_template_op_address:
>  	 */
>  	nop
>  	nop
> +#ifdef CONFIG_PPC64
>  	nop
>  	nop
>  	nop
> +#endif
>  	/* 2. pt_regs pointer in r4 */
>  	addi	r4,r1,STACK_FRAME_OVERHEAD
> 
> @@ -94,9 +104,11 @@ optprobe_template_insn:
>  	/* 2, Pass instruction to be emulated in r4 */
>  	nop
>  	nop
> +#ifdef CONFIG_PPC64
>  	nop
>  	nop
>  	nop
> +#endif

It would be nice to put these behind a macro so as to avoid these #ifdef 
blocks here, as well as with the register save/restore sequence.

- Naveen

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ