Messages in this thread Patch in this message | | | From | Mark-PK Tsai <> | Subject | Re: [PATCH] arm64: ftrace: don't dereference a probably invalid address | Date | Tue, 8 Jun 2021 02:14:03 +0800 |
| |
> On Mon, 7 Jun 2021 22:15:22 +0800 > Mark-PK Tsai <mark-pk.tsai@mediatek.com> wrote: > > > And the crash is becuase kernel trying to read *rec->ip in print_ip_ins() if > > ftrace_bug() get error code -EINVAL. > > Right, so the actual fix should be something like this: > > [ not tested, nor even compiled ] > > -- Steve >
I just test this patch, and turns out it work well. All my modification is only rename copy_from_kernel_nofault into probe_kernel_read because the older kernel version I use. Then the kernel will show the error mesage instead of panic as following.
[ 0.000000] ftrace failed to modify [ 0.000000] [<0000000000000020>] 0x20 [ 0.000000] actual: [FAULT](0000000000000020)
Should I resend this patch as v2? Or you will upstream this fix?
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 2e8a3fde7104..72ef4dccbcc4 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -1967,12 +1967,18 @@ static int ftrace_hash_ipmodify_update(struct ftrace_ops *ops, > > static void print_ip_ins(const char *fmt, const unsigned char *p) > { > + char ins[MCOUNT_INSN_SIZE]; > int i; > > + if (copy_from_kernel_nofault(ins, p, MCOUNT_INSN_SIZE)) { > + printk(KERN_CONT "%s[FAULT](%px)", fmt, p); > + return; > + } > + > printk(KERN_CONT "%s", fmt); > > for (i = 0; i < MCOUNT_INSN_SIZE; i++) > - printk(KERN_CONT "%s%02x", i ? ":" : "", p[i]); > + printk(KERN_CONT "%s%02x", i ? ":" : "", ins[i]); > } > > enum ftrace_bug_type ftrace_bug_type;
| |