Re: [PATCH 2/9] ARM: traps: use get_kernel_nofault instead of set_fs()

Christoph Hellwig
2020-09-08T06:15:34Z

> +static void dump_mem(const char *, const char *, unsigned long, unsigned long, bool kernel_mode);

This adds a pointlessly long line.

And looking at the code I don't see why the argument is even needed.

dump_mem() currently does an unconditional set_fs(KERNEL_DS), so it
should always use get_kernel_nofault.

> +static void dump_instr(const char *lvl, struct pt_regs *regs)
> {
> unsigned long addr = instruction_pointer(regs);
> const int thumb = thumb_mode(regs);
> @@ -173,10 +169,20 @@ static void __dump_instr(const char *lvl, struct pt_regs *regs)
> for (i = -4; i < 1 + !!thumb; i++) {
> unsigned int val, bad;
>
> - if (thumb)
> - bad = get_user(val, &((u16 *)addr)[i]);
> - else
> - bad = get_user(val, &((u32 *)addr)[i]);
> + if (!user_mode(regs)) {
> + if (thumb) {
> + u16 val16;
> + bad = get_kernel_nofault(val16, &((u16 *)addr)[i]);
> + val = val16;
> + } else {
> + bad = get_kernel_nofault(val, &((u32 *)addr)[i]);
> + }
> + } else {
> + if (thumb)
> + bad = get_user(val, &((u16 *)addr)[i]);
> + else
> + bad = get_user(val, &((u32 *)addr)[i]);
> + }

When I looked at this earlier I just added a little helper to make
this a little easier to read. Here is my patch from an old tree: