2020-04-08 08:13:56

by Zong Li

Subject: [PATCH v5 3/9] riscv: patch code by fixmap mapping

On strict kernel memory permission, the ftrace have to change the
permission of text for dynamic patching the intructions. Use
riscv_patch_text_nosync() to patch code instead of probe_kernel_write.

Signed-off-by: Zong Li <[email protected]>
Suggested-by: Masami Hiramatsu <[email protected]>
Reviewed-by: Masami Hiramatsu <[email protected]>
---
arch/riscv/kernel/ftrace.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index c40fdcdeb950..08396614d6f4 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -7,9 +7,23 @@

#include <linux/ftrace.h>
#include <linux/uaccess.h>
+#include <linux/memory.h>
#include <asm/cacheflush.h>
+#include <asm/patch.h>

#ifdef CONFIG_DYNAMIC_FTRACE
+int ftrace_arch_code_modify_prepare(void) __acquires(&text_mutex)
+{
+ mutex_lock(&text_mutex);
+ return 0;
+}
+
+int ftrace_arch_code_modify_post_process(void) __releases(&text_mutex)
+{
+ mutex_unlock(&text_mutex);
+ return 0;
+}
+
static int ftrace_check_current_call(unsigned long hook_pos,
unsigned int *expected)
{
@@ -46,20 +60,14 @@ static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target,
{
unsigned int call[2];
unsigned int nops[2] = {NOP4, NOP4};
- int ret = 0;

make_call(hook_pos, target, call);

- /* replace the auipc-jalr pair at once */
- ret = probe_kernel_write((void *)hook_pos, enable ? call : nops,
- MCOUNT_INSN_SIZE);
- /* return must be -EPERM on write error */
- if (ret)
+ /* Replace the auipc-jalr pair at once. Return -EPERM on write error. */
+ if (patch_text_nosync
+ ((void *)hook_pos, enable ? call : nops, MCOUNT_INSN_SIZE))
return -EPERM;

- smp_mb();
- flush_icache_range((void *)hook_pos, (void *)hook_pos + MCOUNT_INSN_SIZE);
-
return 0;
}

--
2.26.0

2020-04-18 00:24:48

by Palmer Dabbelt

Subject: Re: [PATCH v5 3/9] riscv: patch code by fixmap mapping

On Wed, 08 Apr 2020 00:56:58 PDT (-0700), [email protected] wrote:
> On strict kernel memory permission, the ftrace have to change the
> permission of text for dynamic patching the intructions. Use
> riscv_patch_text_nosync() to patch code instead of probe_kernel_write.
>
> Signed-off-by: Zong Li <[email protected]>
> Suggested-by: Masami Hiramatsu <[email protected]>
> Reviewed-by: Masami Hiramatsu <[email protected]>
> ---
> arch/riscv/kernel/ftrace.c | 26 +++++++++++++++++---------
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
> index c40fdcdeb950..08396614d6f4 100644
> --- a/arch/riscv/kernel/ftrace.c
> +++ b/arch/riscv/kernel/ftrace.c
> @@ -7,9 +7,23 @@
>
> #include <linux/ftrace.h>
> #include <linux/uaccess.h>
> +#include <linux/memory.h>
> #include <asm/cacheflush.h>
> +#include <asm/patch.h>
>
> #ifdef CONFIG_DYNAMIC_FTRACE
> +int ftrace_arch_code_modify_prepare(void) __acquires(&text_mutex)
> +{
> + mutex_lock(&text_mutex);
> + return 0;
> +}
> +
> +int ftrace_arch_code_modify_post_process(void) __releases(&text_mutex)
> +{
> + mutex_unlock(&text_mutex);
> + return 0;
> +}
> +
> static int ftrace_check_current_call(unsigned long hook_pos,
> unsigned int *expected)
> {
> @@ -46,20 +60,14 @@ static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target,
> {
> unsigned int call[2];
> unsigned int nops[2] = {NOP4, NOP4};
> - int ret = 0;
>
> make_call(hook_pos, target, call);
>
> - /* replace the auipc-jalr pair at once */
> - ret = probe_kernel_write((void *)hook_pos, enable ? call : nops,
> - MCOUNT_INSN_SIZE);
> - /* return must be -EPERM on write error */
> - if (ret)
> + /* Replace the auipc-jalr pair at once. Return -EPERM on write error. */
> + if (patch_text_nosync
> + ((void *)hook_pos, enable ? call : nops, MCOUNT_INSN_SIZE))
> return -EPERM;
>
> - smp_mb();
> - flush_icache_range((void *)hook_pos, (void *)hook_pos + MCOUNT_INSN_SIZE);
> -
> return 0;
> }

Reviewed-by: Palmer Dabbelt <[email protected]>