Hi,

On Wed, 21 Aug 2019 11:07:39 +0900 Masami Hiramatsu wrote:

Hi Jisheng,

On Tue, 20 Aug 2019 03:53:31 +0000
Jisheng Zhang [off-list ref] wrote:

quoted

For KPROBES_ON_FTRACE case, we need to adjust the kprobe's addr
correspondingly.  

Either KPROBES_ON_FTRACE=y or not, ftrace_location() check must be
done correctly. If it failed, kprobes can modify the instruction
which can be modified by ftrace.

quoted

Signed-off-by: Jisheng Zhang <redacted>
---
 kernel/kprobes.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 9873fc627d61..3fd2f68644da 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c

@@ -1484,15 +1484,19 @@ static inline int check_kprobe_rereg(struct kprobe *p)

 int __weak arch_check_ftrace_location(struct kprobe *p)
 {
-     unsigned long ftrace_addr;
+     unsigned long ftrace_addr, addr = (unsigned long)p->addr;

-     ftrace_addr = ftrace_location((unsigned long)p->addr);
+#ifdef CONFIG_KPROBES_ON_FTRACE
+     addr = ftrace_call_adjust(addr);
+#endif
+     ftrace_addr = ftrace_location(addr);  

No, this is not right way to do. If we always need to adjust address
before calling ftrace_location(), something wrong with ftrace_location()
interface.
ftrace_location(addr) must check the address is within the range which
can be changed by ftrace. (dyn->ip <= addr <= dyn->ip+MCOUNT_INSN_SIZE)

yeah! I will try Naveen's suggestion, I.E patch kprobe_lookup_name() instead.

Thanks

quoted

      if (ftrace_addr) {
 #ifdef CONFIG_KPROBES_ON_FTRACE
              /* Given address is not on the instruction boundary */
-             if ((unsigned long)p->addr != ftrace_addr)
+             if (addr != ftrace_addr)
                      return -EILSEQ;
              p->flags |= KPROBE_FLAG_FTRACE;
+             p->addr = (kprobe_opcode_t *)addr;  

And again, please don't change the p->addr silently.

Thank you,

quoted

 #else        /* !CONFIG_KPROBES_ON_FTRACE */
              return -EINVAL;
 #endif
--
2.23.0.rc1
 

--
Masami Hiramatsu [off-list ref]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel