diff mbox series

[08/17] powerpc/ftrace: Refactor ftrace_modify_code()

Message ID a8daa49712b44ff539e6c22a2ea649a540386798.1687166935.git.naveen@kernel.org (mailing list archive)
State Accepted
Commit f4fcbf2e093e25a7faa8a3c2a5097524114e9547
Headers show
Series powerpc/ftrace: refactor and add support for -fpatchable-function-entry | expand

Commit Message

Naveen N Rao June 19, 2023, 9:47 a.m. UTC
Split up ftrace_modify_code() into a few helpers for future use. Also
update error messages accordingly.

Signed-off-by: Naveen N Rao <naveen@kernel.org>
---
 arch/powerpc/kernel/trace/ftrace.c | 51 +++++++++++++++++-------------
 1 file changed, 29 insertions(+), 22 deletions(-)

Comments

Christophe Leroy June 23, 2023, 5:27 a.m. UTC | #1
Le 19/06/2023 à 11:47, Naveen N Rao a écrit :
> Split up ftrace_modify_code() into a few helpers for future use. Also
> update error messages accordingly.
> 
> Signed-off-by: Naveen N Rao <naveen@kernel.org>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/powerpc/kernel/trace/ftrace.c | 51 +++++++++++++++++-------------
>   1 file changed, 29 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
> index 913c7aa63d3fa3..ef4e49c2c37781 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -50,32 +50,39 @@ ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
>   	return op;
>   }
>   
> -static inline int
> -ftrace_modify_code(unsigned long ip, ppc_inst_t old, ppc_inst_t new)
> +static inline int ftrace_read_inst(unsigned long ip, ppc_inst_t *op)
>   {
> -	ppc_inst_t replaced;
> -
> -	/*
> -	 * Note:
> -	 * We are paranoid about modifying text, as if a bug was to happen, it
> -	 * could cause us to read or write to someplace that could cause harm.
> -	 * Carefully read and modify the code with probe_kernel_*(), and make
> -	 * sure what we read is what we expected it to be before modifying it.
> -	 */
> -
> -	/* read the text we want to modify */
> -	if (copy_inst_from_kernel_nofault(&replaced, (void *)ip))
> +	if (copy_inst_from_kernel_nofault(op, (void *)ip)) {
> +		pr_err("0x%lx: fetching instruction failed\n", ip);
>   		return -EFAULT;
> -
> -	/* Make sure it is what we expect it to be */
> -	if (!ppc_inst_equal(replaced, old)) {
> -		pr_err("%p: replaced (%08lx) != old (%08lx)", (void *)ip,
> -		       ppc_inst_as_ulong(replaced), ppc_inst_as_ulong(old));
> -		return -EINVAL;
>   	}
>   
> -	/* replace the text with the new text */
> -	return patch_instruction((u32 *)ip, new);
> +	return 0;
> +}
> +
> +static inline int ftrace_validate_inst(unsigned long ip, ppc_inst_t inst)
> +{
> +	ppc_inst_t op;
> +	int ret;
> +
> +	ret = ftrace_read_inst(ip, &op);
> +	if (!ret && !ppc_inst_equal(op, inst)) {
> +		pr_err("0x%lx: expected (%08lx) != found (%08lx)\n",
> +		       ip, ppc_inst_as_ulong(inst), ppc_inst_as_ulong(op));
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static inline int ftrace_modify_code(unsigned long ip, ppc_inst_t old, ppc_inst_t new)
> +{
> +	int ret = ftrace_validate_inst(ip, old);
> +
> +	if (!ret)
> +		ret = patch_instruction((u32 *)ip, new);
> +
> +	return ret;
>   }
>   
>   /*
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 913c7aa63d3fa3..ef4e49c2c37781 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -50,32 +50,39 @@  ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
 	return op;
 }
 
-static inline int
-ftrace_modify_code(unsigned long ip, ppc_inst_t old, ppc_inst_t new)
+static inline int ftrace_read_inst(unsigned long ip, ppc_inst_t *op)
 {
-	ppc_inst_t replaced;
-
-	/*
-	 * Note:
-	 * We are paranoid about modifying text, as if a bug was to happen, it
-	 * could cause us to read or write to someplace that could cause harm.
-	 * Carefully read and modify the code with probe_kernel_*(), and make
-	 * sure what we read is what we expected it to be before modifying it.
-	 */
-
-	/* read the text we want to modify */
-	if (copy_inst_from_kernel_nofault(&replaced, (void *)ip))
+	if (copy_inst_from_kernel_nofault(op, (void *)ip)) {
+		pr_err("0x%lx: fetching instruction failed\n", ip);
 		return -EFAULT;
-
-	/* Make sure it is what we expect it to be */
-	if (!ppc_inst_equal(replaced, old)) {
-		pr_err("%p: replaced (%08lx) != old (%08lx)", (void *)ip,
-		       ppc_inst_as_ulong(replaced), ppc_inst_as_ulong(old));
-		return -EINVAL;
 	}
 
-	/* replace the text with the new text */
-	return patch_instruction((u32 *)ip, new);
+	return 0;
+}
+
+static inline int ftrace_validate_inst(unsigned long ip, ppc_inst_t inst)
+{
+	ppc_inst_t op;
+	int ret;
+
+	ret = ftrace_read_inst(ip, &op);
+	if (!ret && !ppc_inst_equal(op, inst)) {
+		pr_err("0x%lx: expected (%08lx) != found (%08lx)\n",
+		       ip, ppc_inst_as_ulong(inst), ppc_inst_as_ulong(op));
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static inline int ftrace_modify_code(unsigned long ip, ppc_inst_t old, ppc_inst_t new)
+{
+	int ret = ftrace_validate_inst(ip, old);
+
+	if (!ret)
+		ret = patch_instruction((u32 *)ip, new);
+
+	return ret;
 }
 
 /*