| Message ID | 78480d05821d45e09fb234f61f9037e26d42f02d.1645096227.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc/ftrace: Reserve instructions from function entry for ftrace | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
Naveen N. Rao wrote: > On certain architectures, ftrace can reserve multiple instructions at > function entry. Rather than rejecting kprobe on addresses other than the > exact ftrace call instruction, use the address returned by ftrace to > probe at the correct address when CONFIG_KPROBES_ON_FTRACE is enabled. > > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > --- > kernel/kprobes.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index 94cab8c9ce56cc..0a797ede3fdf37 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -1497,6 +1497,10 @@ bool within_kprobe_blacklist(unsigned long addr) > static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr, > const char *symbol_name, unsigned int offset) > { > +#ifdef CONFIG_KPROBES_ON_FTRACE > + unsigned long ftrace_addr = 0; > +#endif > + > if ((symbol_name && addr) || (!symbol_name && !addr)) > goto invalid; > > @@ -1507,6 +1511,14 @@ static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr, > } > > addr = (kprobe_opcode_t *)(((char *)addr) + offset); > + > +#ifdef CONFIG_KPROBES_ON_FTRACE > + if (addr) > + ftrace_addr = ftrace_location((unsigned long)addr); > + if (ftrace_addr) > + return (kprobe_opcode_t *)ftrace_addr; > +#endif One of the side effects of this is that we'll now allow probes on non-instruction boundary within the full ftrace address range. It's not too much of an issue since we ensure that the probe location eventually lands on the actual ftrace instruction. But, I'm wondering if we should instead allow architectures to opt-in to this, by making this be architecture specific. Architectures can then do whatever validation is necessary. - Naveen
On Thu, 17 Feb 2022 17:06:25 +0530 "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote: > On certain architectures, ftrace can reserve multiple instructions at > function entry. Rather than rejecting kprobe on addresses other than the > exact ftrace call instruction, use the address returned by ftrace to > probe at the correct address when CONFIG_KPROBES_ON_FTRACE is enabled. > > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > --- > kernel/kprobes.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index 94cab8c9ce56cc..0a797ede3fdf37 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -1497,6 +1497,10 @@ bool within_kprobe_blacklist(unsigned long addr) > static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr, > const char *symbol_name, unsigned int offset) > { > +#ifdef CONFIG_KPROBES_ON_FTRACE > + unsigned long ftrace_addr = 0; > +#endif > + > if ((symbol_name && addr) || (!symbol_name && !addr)) > goto invalid; > > @@ -1507,6 +1511,14 @@ static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr, > } > > addr = (kprobe_opcode_t *)(((char *)addr) + offset); > + > +#ifdef CONFIG_KPROBES_ON_FTRACE > + if (addr) > + ftrace_addr = ftrace_location((unsigned long)addr); > + if (ftrace_addr) > + return (kprobe_opcode_t *)ftrace_addr; As I said, this must be if (ftrace_addr != addr) return -EILSEQ; This will prevent users from being confused by the results of probing that 'func' and 'func+4' are the same. (now only 'func' is allowed to be probed.) Thank you, > +#endif > + > if (addr) > return addr; > > -- > 2.35.1 >
diff --git a/kernel/kprobes.c b/kernel/kprobes.c index 94cab8c9ce56cc..0a797ede3fdf37 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -1497,6 +1497,10 @@ bool within_kprobe_blacklist(unsigned long addr) static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr, const char *symbol_name, unsigned int offset) { +#ifdef CONFIG_KPROBES_ON_FTRACE + unsigned long ftrace_addr = 0; +#endif + if ((symbol_name && addr) || (!symbol_name && !addr)) goto invalid; @@ -1507,6 +1511,14 @@ static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr, } addr = (kprobe_opcode_t *)(((char *)addr) + offset); + +#ifdef CONFIG_KPROBES_ON_FTRACE + if (addr) + ftrace_addr = ftrace_location((unsigned long)addr); + if (ftrace_addr) + return (kprobe_opcode_t *)ftrace_addr; +#endif + if (addr) return addr;
On certain architectures, ftrace can reserve multiple instructions at function entry. Rather than rejecting kprobe on addresses other than the exact ftrace call instruction, use the address returned by ftrace to probe at the correct address when CONFIG_KPROBES_ON_FTRACE is enabled. Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> --- kernel/kprobes.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)