* [PATCH v3 1/3] tracing: uprobes: Re-enable $comm support for uprobe events
2019-05-07 13:55 [PATCH v3 0/3] tracing: probeevent: Fix probe argument parser and handler Masami Hiramatsu
@ 2019-05-07 13:55 ` Masami Hiramatsu
2019-05-07 13:55 ` [PATCH v3 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-05-07 13:55 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.
Note that this uses -ECOMM as a token value to fetch the comm
string. If the user-space pointer points -ECOMM, it will be
translated to task->comm.
Fixes: 533059281ee5 ("tracing: probeevent: Introduce new argument fetching code")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reported-by: Andreas Ziegler <andreas.ziegler@fau.de>
Acked-by: Andreas Ziegler <andreas.ziegler@fau.de>
---
Changes in v3
- Always add +1 to strlen() result.
- Use -ECOMM as a token to specify current->comm.
---
kernel/trace/trace_probe.h | 1 +
kernel/trace/trace_uprobe.c | 13 +++++++++++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index b7737666c1a8..f9a8c632188b 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -124,6 +124,7 @@ struct fetch_insn {
/* fetch + deref*N + store + mod + end <= 16, this allows N=12, enough */
#define FETCH_INSN_MAX 16
+#define FETCH_TOKEN_COMM (-ECOMM)
/* Fetch type information table */
struct fetch_type {
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index cd8750a72768..eb7e06b54741 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 == FETCH_TOKEN_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,10 @@ fetch_store_strlen(unsigned long addr)
int len;
void __user *vaddr = (void __force __user *) addr;
- len = strnlen_user(vaddr, MAX_STRING_SIZE);
+ if (addr == FETCH_TOKEN_COMM)
+ len = strlen(current->comm) + 1;
+ else
+ len = strnlen_user(vaddr, MAX_STRING_SIZE);
return (len > MAX_STRING_SIZE) ? 0 : len;
}
@@ -220,6 +226,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 = FETCH_TOKEN_COMM;
+ break;
case FETCH_OP_FOFFS:
val = translate_user_vaddr(code->immediate);
break;
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 2/3] tracing: probeevent: Do not accumulate on ret variable
2019-05-07 13:55 [PATCH v3 0/3] tracing: probeevent: Fix probe argument parser and handler Masami Hiramatsu
2019-05-07 13:55 ` [PATCH v3 1/3] tracing: uprobes: Re-enable $comm support for uprobe events Masami Hiramatsu
@ 2019-05-07 13:55 ` Masami Hiramatsu
2019-05-07 13:56 ` [PATCH v3 3/3] tracing: probeevent: Fix to make the type of $comm string Masami Hiramatsu
2019-05-08 2:10 ` [PATCH v3 0/3] tracing: probeevent: Fix probe argument parser and handler Steven Rostedt
3 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2019-05-07 13:55 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.
Fixes: 40b53b771806 ("tracing: probeevent: Add array type support")
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 4737bb8c07a3..c30c61f12ddd 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 v3 3/3] tracing: probeevent: Fix to make the type of $comm string
2019-05-07 13:55 [PATCH v3 0/3] tracing: probeevent: Fix probe argument parser and handler Masami Hiramatsu
2019-05-07 13:55 ` [PATCH v3 1/3] tracing: uprobes: Re-enable $comm support for uprobe events Masami Hiramatsu
2019-05-07 13:55 ` [PATCH v3 2/3] tracing: probeevent: Do not accumulate on ret variable Masami Hiramatsu
@ 2019-05-07 13:56 ` Masami Hiramatsu
2019-05-08 2:10 ` [PATCH v3 0/3] tracing: probeevent: Fix probe argument parser and handler Steven Rostedt
3 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2019-05-07 13:56 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.
Fixes: 35abb67de744 ("tracing: expose current->comm to [ku]probe events")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: stable@vger.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 4cc2d467d34c..e0d1d5353464 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -533,13 +533,14 @@ static int traceprobe_parse_probe_arg_body(char *arg, ssize_t *size,
}
}
}
- /*
- * 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) {
trace_probe_log_err(offset + (t ? (t - arg) : 0), BAD_TYPE);
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3 0/3] tracing: probeevent: Fix probe argument parser and handler
2019-05-07 13:55 [PATCH v3 0/3] tracing: probeevent: Fix probe argument parser and handler Masami Hiramatsu
` (2 preceding siblings ...)
2019-05-07 13:56 ` [PATCH v3 3/3] tracing: probeevent: Fix to make the type of $comm string Masami Hiramatsu
@ 2019-05-08 2:10 ` Steven Rostedt
2019-05-08 4:59 ` Masami Hiramatsu
3 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2019-05-08 2:10 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: Andreas Ziegler, Ingo Molnar, linux-kernel
On Tue, 7 May 2019 22:55:22 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> Hi,
>
> Here is the 3rd version of series to fix several bugs in probe event
> argument parser and handler routines.
>
> In this version I updated patch [1/3] according to Steve's comment.
>
>
> 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.
>
Thanks Masami,
I applied these to my queue.
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3 0/3] tracing: probeevent: Fix probe argument parser and handler
2019-05-08 2:10 ` [PATCH v3 0/3] tracing: probeevent: Fix probe argument parser and handler Steven Rostedt
@ 2019-05-08 4:59 ` Masami Hiramatsu
0 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2019-05-08 4:59 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Andreas Ziegler, Ingo Molnar, linux-kernel
On Tue, 7 May 2019 22:10:06 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Tue, 7 May 2019 22:55:22 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> > Hi,
> >
> > Here is the 3rd version of series to fix several bugs in probe event
> > argument parser and handler routines.
> >
> > In this version I updated patch [1/3] according to Steve's comment.
> >
> >
> > 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.
> >
>
> Thanks Masami,
>
> I applied these to my queue.
Thank you, Steve! I'll rebase other series on that.
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread