* [PATCH 1/3] tracing: uprobes: Re-enable $comm support for uprobe events
2019-01-17 13:47 [PATCH 0/3] tracing: probeevent: Fix probe argument parser and handler Masami Hiramatsu
@ 2019-01-17 13:48 ` Masami Hiramatsu
2019-01-17 13:48 ` [PATCH 2/3] tracing: probeevent: Do not accumulate on ret variable Masami Hiramatsu
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2019-01-17 13:48 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andreas Ziegler, Masami Hiramatsu, Ingo Molnar, linux-kernel
Since commit 533059281ee5 ("tracing: probeevent: Introduce new
argument fetching code") dropped the $comm support from uprobe
events, this re-enables it.
For $comm support, uses strlcpy() instead of strncpy_from_user()
to copy current task's comm. Because it is in the kernel space,
strncpy_from_user() always fails to copy the comm.
This also uses strlen() instead of strnlen_user() to measure the
length of the comm.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reported-by: Andreas Ziegler <andreas.ziegler@fau.de>
Acked-by: Andreas Ziegler <andreas.ziegler@fau.de>
---
kernel/trace/trace_uprobe.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 3a1d5ab6b4ba..b07e498ccbc6 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -156,7 +156,10 @@ fetch_store_string(unsigned long addr, void *dest, void *base)
if (unlikely(!maxlen))
return -ENOMEM;
- ret = strncpy_from_user(dst, src, maxlen);
+ if (addr == (unsigned long)current->comm)
+ ret = strlcpy(dst, current->comm, maxlen);
+ else
+ ret = strncpy_from_user(dst, src, maxlen);
if (ret >= 0) {
if (ret == maxlen)
dst[ret - 1] = '\0';
@@ -180,7 +183,12 @@ fetch_store_strlen(unsigned long addr)
int len;
void __user *vaddr = (void __force __user *) addr;
- len = strnlen_user(vaddr, MAX_STRING_SIZE);
+ if (addr == (unsigned long)current->comm) {
+ len = strlen(current->comm);
+ if (len)
+ len++;
+ } else
+ len = strnlen_user(vaddr, MAX_STRING_SIZE);
return (len > MAX_STRING_SIZE) ? 0 : len;
}
@@ -220,6 +228,9 @@ process_fetch_insn(struct fetch_insn *code, struct pt_regs *regs, void *dest,
case FETCH_OP_IMM:
val = code->immediate;
break;
+ case FETCH_OP_COMM:
+ val = (unsigned long)current->comm;
+ break;
case FETCH_OP_FOFFS:
val = translate_user_vaddr(code->immediate);
break;
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] tracing: probeevent: Do not accumulate on ret variable
2019-01-17 13:47 [PATCH 0/3] tracing: probeevent: Fix probe argument parser and handler Masami Hiramatsu
2019-01-17 13:48 ` [PATCH 1/3] tracing: uprobes: Re-enable $comm support for uprobe events Masami Hiramatsu
@ 2019-01-17 13:48 ` Masami Hiramatsu
2019-01-17 13:49 ` [PATCH 3/3] tracing: probeevent: Fix to make the type of $comm string Masami Hiramatsu
2019-01-17 14:23 ` [PATCH 0/3] tracing: probeevent: Fix probe argument parser and handler Steven Rostedt
3 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2019-01-17 13:48 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andreas Ziegler, Masami Hiramatsu, Ingo Molnar, linux-kernel
Do not accumulate strlen result on ret local variable, because
it is accumulated on total local variable for array case.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace_probe_tmpl.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h
index 5c56afc17cf8..e69b05a9f57c 100644
--- a/kernel/trace/trace_probe_tmpl.h
+++ b/kernel/trace/trace_probe_tmpl.h
@@ -88,7 +88,7 @@ process_fetch_insn_bottom(struct fetch_insn *code, unsigned long val,
/* 3rd stage: store value to buffer */
if (unlikely(!dest)) {
if (code->op == FETCH_OP_ST_STRING) {
- ret += fetch_store_strlen(val + code->offset);
+ ret = fetch_store_strlen(val + code->offset);
code++;
goto array;
} else
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] tracing: probeevent: Fix to make the type of $comm string
2019-01-17 13:47 [PATCH 0/3] tracing: probeevent: Fix probe argument parser and handler Masami Hiramatsu
2019-01-17 13:48 ` [PATCH 1/3] tracing: uprobes: Re-enable $comm support for uprobe events Masami Hiramatsu
2019-01-17 13:48 ` [PATCH 2/3] tracing: probeevent: Do not accumulate on ret variable Masami Hiramatsu
@ 2019-01-17 13:49 ` Masami Hiramatsu
2019-01-17 14:23 ` [PATCH 0/3] tracing: probeevent: Fix probe argument parser and handler Steven Rostedt
3 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2019-01-17 13:49 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andreas Ziegler, Masami Hiramatsu, Ingo Molnar, linux-kernel
Fix to make the type of $comm "string".
If we set the other type to $comm argument, it shows
meaningless value or wrong data. Currently probe events
allow us to set string array type (e.g. ":string[2]"), or
other digit types like x8 on $comm. But since clearly
$comm is just a string data, it should not be fetched
by other types including array.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace_probe.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 9962cb5da8ac..44f078cda0ac 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -405,13 +405,14 @@ static int traceprobe_parse_probe_arg_body(char *arg, ssize_t *size,
return -E2BIG;
}
}
- /*
- * The default type of $comm should be "string", and it can't be
- * dereferenced.
- */
- if (!t && strcmp(arg, "$comm") == 0)
+
+ /* Since $comm can not be dereferred, we can find $comm by strcmp */
+ if (strcmp(arg, "$comm") == 0) {
+ /* The type of $comm must be "string", and not an array. */
+ if (parg->count || (t && strcmp(t, "string")))
+ return -EINVAL;
parg->type = find_fetch_type("string");
- else
+ } else
parg->type = find_fetch_type(t);
if (!parg->type) {
pr_info("Unsupported type: %s\n", t);
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] tracing: probeevent: Fix probe argument parser and handler
2019-01-17 13:47 [PATCH 0/3] tracing: probeevent: Fix probe argument parser and handler Masami Hiramatsu
` (2 preceding siblings ...)
2019-01-17 13:49 ` [PATCH 3/3] tracing: probeevent: Fix to make the type of $comm string Masami Hiramatsu
@ 2019-01-17 14:23 ` Steven Rostedt
2019-01-17 23:30 ` Masami Hiramatsu
3 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2019-01-17 14:23 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: Andreas Ziegler, Ingo Molnar, linux-kernel
On Thu, 17 Jan 2019 22:47:58 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> Hi,
>
> This series fixes several bugs in probe event argument parser
> and handler routines.
Hi Masami,
Can you resend with a "Fixes:" tag to each of the patches, so that it
is easy to see what patch needs to be backported to which stable kernel
(or not any stable release if the patch fixes something in 5.0-rc).
Thanks!
-- Steve
>
> Recently I got 2 issues reported by Andreas, see
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1899098.html
>
> One issue is already fixed by Andreas (Thanks!) but $comm handling
> issue still exists on uprobe event. [1/3] fixes it.
> And I found other issues around that, [2/3] is just a trivial cleanup,
> [3/3] fixes $comm type issue which occurs not only uprobe events but
> also on kprobe events. Anyway, after this series applied, $comm must
> be "string" type and not be an array.
>
> Thank you,
>
> ---
>
> Masami Hiramatsu (3):
> tracing: uprobes: Re-enable $comm support for uprobe events
> tracing: probeevent: Do not accumulate on ret variable
> tracing: probeevent: Fix to make the type of $comm string
>
>
> kernel/trace/trace_probe.c | 13 +++++++------
> kernel/trace/trace_probe_tmpl.h | 2 +-
> kernel/trace/trace_uprobe.c | 15 +++++++++++++--
> 3 files changed, 21 insertions(+), 9 deletions(-)
>
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] tracing: probeevent: Fix probe argument parser and handler
2019-01-17 14:23 ` [PATCH 0/3] tracing: probeevent: Fix probe argument parser and handler Steven Rostedt
@ 2019-01-17 23:30 ` Masami Hiramatsu
0 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2019-01-17 23:30 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Andreas Ziegler, Ingo Molnar, linux-kernel
On Thu, 17 Jan 2019 09:23:02 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Thu, 17 Jan 2019 22:47:58 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> > Hi,
> >
> > This series fixes several bugs in probe event argument parser
> > and handler routines.
>
> Hi Masami,
>
> Can you resend with a "Fixes:" tag to each of the patches, so that it
> is easy to see what patch needs to be backported to which stable kernel
> (or not any stable release if the patch fixes something in 5.0-rc).
OK, I'll resend it with Fixed tags!
And maybe I also need to enhance ftracetest so that it can catch
this bug.
Thanks,
>
> Thanks!
>
> -- Steve
>
> >
> > Recently I got 2 issues reported by Andreas, see
> > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1899098.html
> >
> > One issue is already fixed by Andreas (Thanks!) but $comm handling
> > issue still exists on uprobe event. [1/3] fixes it.
> > And I found other issues around that, [2/3] is just a trivial cleanup,
> > [3/3] fixes $comm type issue which occurs not only uprobe events but
> > also on kprobe events. Anyway, after this series applied, $comm must
> > be "string" type and not be an array.
> >
> > Thank you,
> >
> > ---
> >
> > Masami Hiramatsu (3):
> > tracing: uprobes: Re-enable $comm support for uprobe events
> > tracing: probeevent: Do not accumulate on ret variable
> > tracing: probeevent: Fix to make the type of $comm string
> >
> >
> > kernel/trace/trace_probe.c | 13 +++++++------
> > kernel/trace/trace_probe_tmpl.h | 2 +-
> > kernel/trace/trace_uprobe.c | 15 +++++++++++++--
> > 3 files changed, 21 insertions(+), 9 deletions(-)
> >
> > --
> > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread