| Message ID | 20200217083223.2011-9-zong.li@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support strict kernel memory permissions for security | expand |
On Mon, 17 Feb 2020 00:32:23 PST (-0800), zong.li@sifive.com wrote: > After the text section be mask as non-writable, the ftrace have to > change the permission of text for dynamic patching the intructions. > Add ftrace_arch_code_modify_prepare and > ftrace_arch_code_modify_post_process to change permission. > > Signed-off-by: Zong Li <zong.li@sifive.com> > --- > arch/riscv/kernel/ftrace.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c > index c40fdcdeb950..576df0807200 100644 > --- a/arch/riscv/kernel/ftrace.c > +++ b/arch/riscv/kernel/ftrace.c > @@ -7,9 +7,27 @@ > > #include <linux/ftrace.h> > #include <linux/uaccess.h> > +#include <linux/memory.h> > +#include <asm/set_memory.h> > #include <asm/cacheflush.h> > > #ifdef CONFIG_DYNAMIC_FTRACE > +int ftrace_arch_code_modify_prepare(void) __acquires(&text_mutex) > +{ > + mutex_lock(&text_mutex); > + set_kernel_text_rw(); > + set_all_modules_text_rw(); > + return 0; > +} None of the other architectures are doing anything remotely like this in these functions, despite many supporting STRICT_KERNEL_RWX. Having a function that maps all text as RW seems super dangerous, as one stack attack means NX is gone. Looks like FIX_TEXT_POKE0 is the magic that makes the other ports work. > + > +int ftrace_arch_code_modify_post_process(void) __releases(&text_mutex) > +{ > + set_all_modules_text_ro(); > + set_kernel_text_ro(); > + mutex_unlock(&text_mutex); > + return 0; > +} Presumably this needs a icache flush as well? Probably better to do so when installing the instructions, though. > + > static int ftrace_check_current_call(unsigned long hook_pos, > unsigned int *expected) > {
Palmer Dabbelt <palmer@dabbelt.com> 於 2020年3月5日 週四 上午9:44寫道： > > On Mon, 17 Feb 2020 00:32:23 PST (-0800), zong.li@sifive.com wrote: > > After the text section be mask as non-writable, the ftrace have to > > change the permission of text for dynamic patching the intructions. > > Add ftrace_arch_code_modify_prepare and > > ftrace_arch_code_modify_post_process to change permission. > > > > Signed-off-by: Zong Li <zong.li@sifive.com> > > --- > > arch/riscv/kernel/ftrace.c | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c > > index c40fdcdeb950..576df0807200 100644 > > --- a/arch/riscv/kernel/ftrace.c > > +++ b/arch/riscv/kernel/ftrace.c > > @@ -7,9 +7,27 @@ > > > > #include <linux/ftrace.h> > > #include <linux/uaccess.h> > > +#include <linux/memory.h> > > +#include <asm/set_memory.h> > > #include <asm/cacheflush.h> > > > > #ifdef CONFIG_DYNAMIC_FTRACE > > +int ftrace_arch_code_modify_prepare(void) __acquires(&text_mutex) > > +{ > > + mutex_lock(&text_mutex); > > + set_kernel_text_rw(); > > + set_all_modules_text_rw(); > > + return 0; > > +} > > None of the other architectures are doing anything remotely like this in these > functions, despite many supporting STRICT_KERNEL_RWX. Having a function that > maps all text as RW seems super dangerous, as one stack attack means NX is > gone. > > Looks like FIX_TEXT_POKE0 is the magic that makes the other ports work. > Your concern is right. Let me change the way. > > + > > +int ftrace_arch_code_modify_post_process(void) __releases(&text_mutex) > > +{ > > + set_all_modules_text_ro(); > > + set_kernel_text_ro(); > > + mutex_unlock(&text_mutex); > > + return 0; > > +} > > Presumably this needs a icache flush as well? Probably better to do so when > installing the instructions, though. > Yes, I think I lost it. Thanks. > > + > > static int ftrace_check_current_call(unsigned long hook_pos, > > unsigned int *expected) > > { >
diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c index c40fdcdeb950..576df0807200 100644 --- a/arch/riscv/kernel/ftrace.c +++ b/arch/riscv/kernel/ftrace.c @@ -7,9 +7,27 @@ #include <linux/ftrace.h> #include <linux/uaccess.h> +#include <linux/memory.h> +#include <asm/set_memory.h> #include <asm/cacheflush.h> #ifdef CONFIG_DYNAMIC_FTRACE +int ftrace_arch_code_modify_prepare(void) __acquires(&text_mutex) +{ + mutex_lock(&text_mutex); + set_kernel_text_rw(); + set_all_modules_text_rw(); + return 0; +} + +int ftrace_arch_code_modify_post_process(void) __releases(&text_mutex) +{ + set_all_modules_text_ro(); + set_kernel_text_ro(); + mutex_unlock(&text_mutex); + return 0; +} + static int ftrace_check_current_call(unsigned long hook_pos, unsigned int *expected) {
After the text section be mask as non-writable, the ftrace have to change the permission of text for dynamic patching the intructions. Add ftrace_arch_code_modify_prepare and ftrace_arch_code_modify_post_process to change permission. Signed-off-by: Zong Li <zong.li@sifive.com> --- arch/riscv/kernel/ftrace.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)