Re: [PATCH] unicore32: use get_kernel_nofault in dump mem and dump_instr
On Tue, Jul 21, 2020 at 10:09:20AM +0800, Guan Xuetao wrote: > Very good to remove ds/fs in unicore arch. > Could u send me full patch for test? This is the full patch for now. The actual removal will need more core instrastructure and at least one more merge window.
Re: [PATCH] unicore32: use get_kernel_nofault in dump mem and dump_instr
Very good to remove ds/fs in unicore arch. Could u send me full patch for test? Guan Xuetao > -原始邮件- > 发件人: "Christoph Hellwig" > 发送时间: 2020-07-20 19:43:58 (星期一) > 收件人: g...@pku.edu.cn > 抄送: linux-kernel@vger.kernel.org > 主题: [PATCH] unicore32: use get_kernel_nofault in dump mem and dump_instr > > Use the proper get_kernel_nofault helper to access an unsafe kernel > pointer without faulting instead of playing with set_fs and get_user. > > Signed-off-by: Christoph Hellwig > --- > arch/unicore32/kernel/traps.c | 28 +++- > 1 file changed, 3 insertions(+), 25 deletions(-) > > diff --git a/arch/unicore32/kernel/traps.c b/arch/unicore32/kernel/traps.c > index a3ac01df1a2e43..c1964a4a1edbb0 100644 > --- a/arch/unicore32/kernel/traps.c > +++ b/arch/unicore32/kernel/traps.c > @@ -65,17 +65,8 @@ static void dump_mem(const char *lvl, const char *str, > unsigned long bottom, >unsigned long top) > { > unsigned long first; > - mm_segment_t fs; > int i; > > - /* > - * We need to switch to kernel mode so that we can use __get_user > - * to safely read from kernel space. Note that we now dump the > - * code first, just in case the backtrace kills us. > - */ > - fs = get_fs(); > - set_fs(KERNEL_DS); > - > printk(KERN_DEFAULT "%s%s(0x%08lx to 0x%08lx)\n", > lvl, str, bottom, top); > > @@ -89,7 +80,8 @@ static void dump_mem(const char *lvl, const char *str, > unsigned long bottom, > for (p = first, i = 0; i < 8 && p < top; i++, p += 4) { > if (p >= bottom && p < top) { > unsigned long val; > - if (__get_user(val, (unsigned long *)p) == 0) > + if (get_kernel_nofault(val, (unsigned long *)p) > + == 0) > sprintf(str + i * 9, " %08lx", val); > else > sprintf(str + i * 9, " "); > @@ -97,31 +89,19 @@ static void dump_mem(const char *lvl, const char *str, > unsigned long bottom, > } > printk(KERN_DEFAULT "%s%04lx:%s\n", lvl, first & 0x, str); > } > - > - set_fs(fs); > } > > static void dump_instr(const char *lvl, struct pt_regs *regs) > { > unsigned long addr = instruction_pointer(regs); > const int width = 8; > - mm_segment_t fs; > char str[sizeof(" ") * 5 + 2 + 1], *p = str; > int i; > > - /* > - * We need to switch to kernel mode so that we can use __get_user > - * to safely read from kernel space. Note that we now dump the > - * code first, just in case the backtrace kills us. > - */ > - fs = get_fs(); > - set_fs(KERNEL_DS); > - > for (i = -4; i < 1; i++) { > unsigned int val, bad; > > - bad = __get_user(val, &((u32 *)addr)[i]); > - > + bad = get_kernel_nofault(val, &((u32 *)addr)[i]); > if (!bad) > p += sprintf(p, i == 0 ? "(%0*x) " : "%0*x ", > width, val); > @@ -131,8 +111,6 @@ static void dump_instr(const char *lvl, struct pt_regs > *regs) > } > } > printk(KERN_DEFAULT "%sCode: %s\n", lvl, str); > - > - set_fs(fs); > } > > static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk, > -- > 2.27.0
[PATCH] unicore32: use get_kernel_nofault in dump mem and dump_instr
Use the proper get_kernel_nofault helper to access an unsafe kernel pointer without faulting instead of playing with set_fs and get_user. Signed-off-by: Christoph Hellwig --- arch/unicore32/kernel/traps.c | 28 +++- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/arch/unicore32/kernel/traps.c b/arch/unicore32/kernel/traps.c index a3ac01df1a2e43..c1964a4a1edbb0 100644 --- a/arch/unicore32/kernel/traps.c +++ b/arch/unicore32/kernel/traps.c @@ -65,17 +65,8 @@ static void dump_mem(const char *lvl, const char *str, unsigned long bottom, unsigned long top) { unsigned long first; - mm_segment_t fs; int i; - /* -* We need to switch to kernel mode so that we can use __get_user -* to safely read from kernel space. Note that we now dump the -* code first, just in case the backtrace kills us. -*/ - fs = get_fs(); - set_fs(KERNEL_DS); - printk(KERN_DEFAULT "%s%s(0x%08lx to 0x%08lx)\n", lvl, str, bottom, top); @@ -89,7 +80,8 @@ static void dump_mem(const char *lvl, const char *str, unsigned long bottom, for (p = first, i = 0; i < 8 && p < top; i++, p += 4) { if (p >= bottom && p < top) { unsigned long val; - if (__get_user(val, (unsigned long *)p) == 0) + if (get_kernel_nofault(val, (unsigned long *)p) + == 0) sprintf(str + i * 9, " %08lx", val); else sprintf(str + i * 9, " "); @@ -97,31 +89,19 @@ static void dump_mem(const char *lvl, const char *str, unsigned long bottom, } printk(KERN_DEFAULT "%s%04lx:%s\n", lvl, first & 0x, str); } - - set_fs(fs); } static void dump_instr(const char *lvl, struct pt_regs *regs) { unsigned long addr = instruction_pointer(regs); const int width = 8; - mm_segment_t fs; char str[sizeof(" ") * 5 + 2 + 1], *p = str; int i; - /* -* We need to switch to kernel mode so that we can use __get_user -* to safely read from kernel space. Note that we now dump the -* code first, just in case the backtrace kills us. -*/ - fs = get_fs(); - set_fs(KERNEL_DS); - for (i = -4; i < 1; i++) { unsigned int val, bad; - bad = __get_user(val, &((u32 *)addr)[i]); - + bad = get_kernel_nofault(val, &((u32 *)addr)[i]); if (!bad) p += sprintf(p, i == 0 ? "(%0*x) " : "%0*x ", width, val); @@ -131,8 +111,6 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) } } printk(KERN_DEFAULT "%sCode: %s\n", lvl, str); - - set_fs(fs); } static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk, -- 2.27.0