2021-01-26 07:47:58

by Jianlin Lv

[permalink] [raw]
Subject: [PATCH v3] tracing: precise log info for kretprobe addr err

When trying to create kretprobe with the wrong function symbol in tracefs;
The error is triggered in the register_trace_kprobe() and recorded as
FAIL_REG_PROBE issue,

Example:
$ cd /sys/kernel/debug/tracing
$ echo 'r:myprobe ERROR_SYMBOL_XXX ret=%x0' >> kprobe_events
bash: echo: write error: Invalid argument
$ cat error_log
[142797.347877] trace_kprobe: error: Failed to register probe event
Command: r:myprobe ERROR_SYMBOL_XXX ret=%x0
^

This error can be detected in the parameter parsing stage, the effect of
applying this patch is as follows:

$ echo 'r:myprobe ERROR_SYMBOL_XXX ret=%x0' >> kprobe_events
bash: echo: write error: Invalid argument
$ cat error_log
[415.89]trace_kprobe: error: Retprobe address must be an function entry
Command: r:myprobe ERROR_SYMBOL_XXX ret=%x0
^

Signed-off-by: Jianlin Lv <[email protected]>
---
v2:add !strchr(symbol, ':') to check really bad symbol or not.
---
kernel/trace/trace_kprobe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index e6fba1798771..bce63d5ecaec 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -830,7 +830,7 @@ static int trace_kprobe_create(int argc, const char *argv[])
flags |= TPARG_FL_RETURN;
if (kprobe_on_func_entry(NULL, symbol, offset))
flags |= TPARG_FL_FENTRY;
- if (offset && is_return && !(flags & TPARG_FL_FENTRY)) {
+ if (!strchr(symbol, ':') && is_return && !(flags & TPARG_FL_FENTRY)) {
trace_probe_log_err(0, BAD_RETPROBE);
goto parse_error;
}
--
2.25.1


2021-01-27 20:58:40

by Jianlin Lv

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



> -----Original Message-----
> From: Masami Hiramatsu <[email protected]>
> Sent: Wednesday, January 27, 2021 10:02 AM
> To: Oleg Nesterov <[email protected]>
> Cc: Steven Rostedt <[email protected]>; Jianlin Lv <[email protected]>;
> [email protected]; [email protected]
> Subject: Re: [PATCH v3] tracing: precise log info for kretprobe addr err
>
> On Tue, 26 Jan 2021 21:20:59 +0100
> Oleg Nesterov <[email protected]> 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 <[email protected]>


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?

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.

2021-01-27 23:58:27

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH v3] tracing: precise log info for kretprobe addr err

On Wed, 27 Jan 2021 02:46:10 +0000
Jianlin Lv <[email protected]> wrote:

>
>
> > -----Original Message-----
> > From: Masami Hiramatsu <[email protected]>
> > Sent: Wednesday, January 27, 2021 10:02 AM
> > To: Oleg Nesterov <[email protected]>
> > Cc: Steven Rostedt <[email protected]>; Jianlin Lv <[email protected]>;
> > [email protected]; [email protected]
> > Subject: Re: [PATCH v3] tracing: precise log info for kretprobe addr err
> >
> > On Tue, 26 Jan 2021 21:20:59 +0100
> > Oleg Nesterov <[email protected]> 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 <[email protected]>
>
>
> 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 <[email protected]>

2021-01-28 00:00:18

by Jianlin Lv

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



> -----Original Message-----
> From: Masami Hiramatsu <[email protected]>
> Sent: Wednesday, January 27, 2021 9:28 PM
> To: Jianlin Lv <[email protected]>
> Cc: Oleg Nesterov <[email protected]>; Steven Rostedt
> <[email protected]>; [email protected]; [email protected]
> Subject: Re: [PATCH v3] tracing: precise log info for kretprobe addr err
>
> On Wed, 27 Jan 2021 02:46:10 +0000
> Jianlin Lv <[email protected]> wrote:
>
> >
> >
> > > -----Original Message-----
> > > From: Masami Hiramatsu <[email protected]>
> > > Sent: Wednesday, January 27, 2021 10:02 AM
> > > To: Oleg Nesterov <[email protected]>
> > > Cc: Steven Rostedt <[email protected]>; Jianlin Lv
> > > <[email protected]>; [email protected]; linux-
> [email protected]
> > > Subject: Re: [PATCH v3] tracing: precise log info for kretprobe addr
> > > err
> > >
> > > On Tue, 26 Jan 2021 21:20:59 +0100
> > > Oleg Nesterov <[email protected]> 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 <[email protected]>
> >
> >
> > 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 <[email protected]>

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.