linux-kernel - Re: [PATCH v1 07/22] powerpc/ftrace: Use patch

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: <1650262058.altfknhzto.naveen@linux.ibm.com>
Date:   Mon, 18 Apr 2022 11:51:16 +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>,
        Ingo Molnar <mingo@...hat.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Paul Mackerras <paulus@...ba.org>,
        Steven Rostedt <rostedt@...dmis.org>
Cc:     linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v1 07/22] powerpc/ftrace: Use patch_instruction() return
 directly

Christophe Leroy wrote:
> Instead of returning -EPERM when patch_instruction() fails,
> just return what patch_instruction returns.
> 
> That simplifies ftrace_modify_code():
> 
> 	   0:	94 21 ff c0 	stwu    r1,-64(r1)
> 	   4:	93 e1 00 3c 	stw     r31,60(r1)
> 	   8:	7c 7f 1b 79 	mr.     r31,r3
> 	   c:	40 80 00 30 	bge     3c <ftrace_modify_code+0x3c>
> 	  10:	93 c1 00 38 	stw     r30,56(r1)
> 	  14:	7c 9e 23 78 	mr      r30,r4
> 	  18:	7c a4 2b 78 	mr      r4,r5
> 	  1c:	80 bf 00 00 	lwz     r5,0(r31)
> 	  20:	7c 1e 28 40 	cmplw   r30,r5
> 	  24:	40 82 00 34 	bne     58 <ftrace_modify_code+0x58>
> 	  28:	83 c1 00 38 	lwz     r30,56(r1)
> 	  2c:	7f e3 fb 78 	mr      r3,r31
> 	  30:	83 e1 00 3c 	lwz     r31,60(r1)
> 	  34:	38 21 00 40 	addi    r1,r1,64
> 	  38:	48 00 00 00 	b       38 <ftrace_modify_code+0x38>
> 				38: R_PPC_REL24	patch_instruction
> 
> Before:
> 
> 	   0:	94 21 ff c0 	stwu    r1,-64(r1)
> 	   4:	93 e1 00 3c 	stw     r31,60(r1)
> 	   8:	7c 7f 1b 79 	mr.     r31,r3
> 	   c:	40 80 00 4c 	bge     58 <ftrace_modify_code+0x58>
> 	  10:	93 c1 00 38 	stw     r30,56(r1)
> 	  14:	7c 9e 23 78 	mr      r30,r4
> 	  18:	7c a4 2b 78 	mr      r4,r5
> 	  1c:	80 bf 00 00 	lwz     r5,0(r31)
> 	  20:	7c 08 02 a6 	mflr    r0
> 	  24:	90 01 00 44 	stw     r0,68(r1)
> 	  28:	7c 1e 28 40 	cmplw   r30,r5
> 	  2c:	40 82 00 48 	bne     74 <ftrace_modify_code+0x74>
> 	  30:	7f e3 fb 78 	mr      r3,r31
> 	  34:	48 00 00 01 	bl      34 <ftrace_modify_code+0x34>
> 				34: R_PPC_REL24	patch_instruction
> 	  38:	80 01 00 44 	lwz     r0,68(r1)
> 	  3c:	20 63 00 00 	subfic  r3,r3,0
> 	  40:	83 c1 00 38 	lwz     r30,56(r1)
> 	  44:	7c 63 19 10 	subfe   r3,r3,r3
> 	  48:	7c 08 03 a6 	mtlr    r0
> 	  4c:	83 e1 00 3c 	lwz     r31,60(r1)
> 	  50:	38 21 00 40 	addi    r1,r1,64
> 	  54:	4e 80 00 20 	blr
> 
> It improves ftrace activation/deactivation duration by about 3%.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
> ---
>  arch/powerpc/kernel/trace/ftrace.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
> index 98e82fa4980f..1b05d33f96c6 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -78,10 +78,7 @@ ftrace_modify_code(unsigned long ip, ppc_inst_t old, ppc_inst_t new)
>  	}
> 
>  	/* replace the text with the new text */
> -	if (patch_instruction((u32 *)ip, new))
> -		return -EPERM;
> -
> -	return 0;
> +	return patch_instruction((u32 *)ip, new);

I think the reason we were returning -EPERM is so that ftrace_bug() can 
throw the right error message. That will change due to this patch, 
though I'm not sure how much it matters. -EFAULT and -EPERM seem to 
print almost the same error message.

- Naveen