Jisheng Zhang wrote:
Hi,
On Thu, 22 Aug 2019 12:23:58 +0530
"Naveen N. Rao" <naveen.n.rao@xxxxxxxxxxxxxxxxxx> 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
