lkml.org 
[lkml]   [2019]   [Aug]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4] arm64: implement KPROBES_ON_FTRACE
Jisheng Zhang wrote:
> Hi,
>
> On Thu, 22 Aug 2019 12:23:58 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>> Jisheng Zhang wrote:
...
>> > +/* Ftrace callback handler for kprobes -- called under preepmt
>> > disabed */
>> > +void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
>> > + struct ftrace_ops *ops, struct pt_regs *regs)
>> > +{
>> > + struct kprobe *p;
>> > + struct kprobe_ctlblk *kcb;
>> > +
>> > + /* Preempt is disabled by ftrace */
>> > + p = get_kprobe((kprobe_opcode_t *)ip);
>> > + if (unlikely(!p) || kprobe_disabled(p))
>> > + return;
>> > +
>> > + kcb = get_kprobe_ctlblk();
>> > + if (kprobe_running()) {
>> > + kprobes_inc_nmissed_count(p);
>> > + } else {
>> > + unsigned long orig_ip = instruction_pointer(regs);
>> > + /* Kprobe handler expects regs->pc = pc + 4 as breakpoint hit */
>> > + instruction_pointer_set(regs, ip + sizeof(kprobe_opcode_t));
>>
>> Just want to make sure that you've confirmed that this is what happens
>> with a regular trap/brk based kprobe on ARM64. The reason for setting
>> the instruction pointer here is to ensure that it is set to the same
>> value as would be set if there was a trap/brk instruction at the ftrace
>> location. This ensures that the kprobe pre handler sees the same value
>> regardless.
>
> Due to the arm64's DYNAMIC_FTRACE_WITH_REGS implementation, the code itself
> is correct. But this doesn't look like "there was a trap instruction at
> the ftrace location".
>
> W/O KPROBE_ON_FTRACE:
>
> foo:
> 00 insA
> 04 insB
> 08 insC
>
> kprobe's pre_handler() will see pc points to 00.

In this case, the probe will be placed at foo+0x00, so pre_handler()
seeing that address in pt_regs is correct behavior - as long as arm64
'brk' instruction causes an exception with the instruction pointer set
*to* the 'brk' instruction. This is similar to how powerpc 'trap' works.
However, x86 'int3' causes an exception *after* execution of the
instruction.

>
> W/ KPROBE_ON_FTRACE:
>
> foo:
> 00 lr saver
> 04 nop // will be modified to ftrace call ins when KPROBE is armed
> 08 insA
> 0c insB

In this case, if user asks for a probe to be placed at 'foo', we will
choose foo+0x04 and from that point on, the behavior should reflect that
a kprobe was placed at foo+0x04. In particular, the pre_handler() should
see foo+0x04 in pt_regs. The post_handler() would then see foo+0x08.

>
> later, kprobe_ftrace_handler() will see pc points to 04, so pc + 4 will
> point to 08 the same as the one w/o KPROBE_ON_FTRACE.

I didn't mean to compare regular trap/brk based kprobes with
KPROBES_ON_FTRACE. The only important aspect is that the handlers see
consistent pt_regs in both cases, depending on where the kprobe was
placed. Choosing a different address/offset to place a kprobe during its
registration is an orthogonal aspect.

>
> It seems I need to fix the comment.

Given your explanation above, I think you can simply drop the first
adjustment to the instruction pointer before the pre handler invocation.
The rest of the code looks fine.


- Naveen

\
 
 \ /
  Last update: 2019-08-22 12:23    [W:0.076 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site