RE: [PATCH v3] tracing: precise log info for kretprobe addr err

[Jianlin Lv]

> -----Original Message-----
> From: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> Sent: Wednesday, January 27, 2021 9:28 PM
> To: Jianlin Lv <Jianlin.Lv@xxxxxxx>
> Cc: Oleg Nesterov <oleg@xxxxxxxxxx>; Steven Rostedt
> <rostedt@xxxxxxxxxxx>; mingo@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v3] tracing: precise log info for kretprobe addr err
>
> On Wed, 27 Jan 2021 02:46:10 +0000
> Jianlin Lv <Jianlin.Lv@xxxxxxx> wrote:
>
> >
> >
> > > -----Original Message-----
> > > From: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> > > Sent: Wednesday, January 27, 2021 10:02 AM
> > > To: Oleg Nesterov <oleg@xxxxxxxxxx>
> > > Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>; Jianlin Lv
> > > <Jianlin.Lv@xxxxxxx>; mingo@xxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> > > Subject: Re: [PATCH v3] tracing: precise log info for kretprobe addr
> > > err
> > >
> > > On Tue, 26 Jan 2021 21:20:59 +0100
> > > Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> > >
> > > > On 01/26, Masami Hiramatsu wrote:
> > > > >
> > > > > > >
> > > > > > > IOW, the "offset != 0" check removed by this patch is
> > > > > > > obviously wrong,
> > > right?
> > > > > > >
> > > > >
> > > > > No, not wrong. Even offset != 0, if the symbol exists in the
> > > > > kernel,
> > > > > kprobe_on_func_entry() will check it.
> > > >
> > > > Yes, but unless I am totally confused... if kprobe_on_func_entry()
> > > > returns false, then trace_kprobe_create() should fail with
> > > > BAD_RETPROBE
> > > even if offset == 0 ?
> > >
> > > Yes, if kprobe_on_func_entry() returns false, register_kretprobe()
> > > also returns an error.
> > >
> > > -----
> > > int register_kretprobe(struct kretprobe *rp) {
> > >         int ret = 0;
> > >         struct kretprobe_instance *inst;
> > >         int i;
> > >         void *addr;
> > >
> > >         if (!kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name,
> > > rp-
> > > >kp.offset))
> > >                 return -EINVAL;
> > >
> > > -----
> > >
> > > Thank you,
> > >
> > > --
> > > Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> >
> >
> > If register_kretprobe()returns an error -EINVAL.
> > This means that __register_trace_kprobe return -EINVAL,
> >
> > ---
> > ret = __register_trace_kprobe(tk);
> > if (ret == -ENOENT && !trace_kprobe_module_exist(tk)) { pr_warn("This
> > probe might be able to register after target module is loaded.
> > Continue.\n"); ret = 0; }
> > ---
> > As code show, cannot enable kretprobe for an unloaded module.
> >
> > This is consistent with my test results (no VXLAN module is loaded).
> >
> > # perf probe -m /lib/modules/5.11.0-rc2+/kernel/drivers/net/vxlan.ko
> > \ 'vxlan_xmit%return $retval'
> > Failed to write event: Invalid argument
> >   Error: Failed to add events.
> >
> > Is this a bug?
>
> Oops, good catch!
> It seems that the bug has been introduced when I added
> kprobe_on_func_entry() to register_Kretprobe.
> Let me fix it.
>
> Thank you!
>
>
> --
> Masami Hiramatsu <mhiramat@xxxxxxxxxx>

After confirming this problem, my worries are eliminated,
and the current patch will be updated later.

I am also investigating this bug, and I think this process will deepen
my understanding of kernel probes.

Jianlin

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.