[PATCH v3] tracing: expose current->comm to [ku]probe events

[PATCH v3] tracing: expose current->comm to [ku]probe events

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

ftrace is very quick to give up on saving the task command line (see
`trace_save_cmdline()`). The workaround for events which really care
about the command line is to explicitly assign it as part of the entry.
However, this doesn't work for kprobe events, as there's no
straightforward way to get access to current->comm. Add a kprobe/uprobe
event variable $comm which provides exactly that.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
Alexei pointed out to me that this same functionality can be acheived
with BPF and the bpf_get_current_comm() helper, so strictly speaking,
this patch isn't necessary. It's still convenient for ad-hoc stuff,
though, so I figured I'd throw this up and see what everyone thought.

 Documentation/trace/kprobetrace.txt  |  3 +++
 Documentation/trace/uprobetracer.txt |  3 +++
 kernel/trace/trace_kprobe.c          |  1 +
 kernel/trace/trace_probe.c           | 33 +++++++++++++++++++++++++++++++++
 kernel/trace/trace_probe.h           | 10 ++++++++++
 5 files changed, 50 insertions(+)

diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
index d68ea5fc812b..ea52ec1f8484 100644
--- a/Documentation/trace/kprobetrace.txt
+++ b/Documentation/trace/kprobetrace.txt
@@ -40,6 +40,7 @@ Synopsis of kprobe_events
   $stackN	: Fetch Nth entry of stack (N >= 0)
   $stack	: Fetch stack address.
   $retval	: Fetch return value.(*)
+  $comm		: Fetch current task comm.
   +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
   NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
   FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
@@ -62,6 +63,8 @@ offset, and container-size (usually 32). The syntax is;
 
  b<bit-width>@<bit-offset>/<container-size>
 
+For $comm, the default type is "string"; any other type is invalid.
+
 
 Per-Probe Event Filtering
 -------------------------
diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
index f1cf9a34ad9d..72d1cd4f7bf3 100644
--- a/Documentation/trace/uprobetracer.txt
+++ b/Documentation/trace/uprobetracer.txt
@@ -36,6 +36,7 @@ Synopsis of uprobe_tracer
    $stackN	: Fetch Nth entry of stack (N >= 0)
    $stack	: Fetch stack address.
    $retval	: Fetch return value.(*)
+   $comm	: Fetch current task comm.
    +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
    NAME=FETCHARG     : Set NAME as the argument name of FETCHARG.
    FETCHARG:TYPE     : Set TYPE as the type of FETCHARG. Currently, basic types
@@ -57,6 +58,8 @@ offset, and container-size (usually 32). The syntax is;
 
  b<bit-width>@<bit-offset>/<container-size>
 
+For $comm, the default type is "string"; any other type is invalid.
+
 
 Event Profiling
 ---------------
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 5546eec0505f..9aedb0b06683 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -587,6 +587,7 @@ static int create_trace_kprobe(int argc, char **argv)
 	 *  $retval	: fetch return value
 	 *  $stack	: fetch stack address
 	 *  $stackN	: fetch Nth of stack (N:0-)
+	 *  $comm       : fetch current task comm
 	 *  @ADDR	: fetch memory at ADDR (ADDR should be in kernel)
 	 *  @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data symbol)
 	 *  %REG	: fetch register REG
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 1d372fa6fefb..74e80a582c28 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -218,6 +218,28 @@ free_bitfield_fetch_param(struct bitfield_fetch_param *data)
 	kfree(data);
 }
 
+void FETCH_FUNC_NAME(comm, string)(struct pt_regs *regs,
+					  void *data, void *dest)
+{
+	int maxlen = get_rloc_len(*(u32 *)dest);
+	u8 *dst = get_rloc_data(dest);
+	long ret;
+
+	if (!maxlen)
+		return;
+
+	ret = strlcpy(dst, current->comm, maxlen);
+	*(u32 *)dest = make_data_rloc(ret, get_rloc_offs(*(u32 *)dest));
+}
+NOKPROBE_SYMBOL(FETCH_FUNC_NAME(comm, string));
+
+void FETCH_FUNC_NAME(comm, string_size)(struct pt_regs *regs,
+					       void *data, void *dest)
+{
+	*(u32 *)dest = strlen(current->comm) + 1;
+}
+NOKPROBE_SYMBOL(FETCH_FUNC_NAME(comm, string_size));
+
 static const struct fetch_type *find_fetch_type(const char *type,
 						const struct fetch_type *ftbl)
 {
@@ -348,6 +370,11 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
 			}
 		} else
 			ret = -EINVAL;
+	} else if (strcmp(arg, "comm") == 0) {
+		if (strcmp(t->name, "string") != 0 &&
+		    strcmp(t->name, "string_size") != 0)
+			return -EINVAL;
+		f->fn = t->fetch[FETCH_MTD_comm];
 	} else
 		ret = -EINVAL;
 
@@ -522,6 +549,12 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
 		arg[t - parg->comm] = '\0';
 		t++;
 	}
+	/*
+	 * The default type of $comm should be "string", and it can't be
+	 * dereferenced.
+	 */
+	if (!t && strcmp(arg, "$comm") == 0)
+		t = "string";
 	parg->type = find_fetch_type(t, ftbl);
 	if (!parg->type) {
 		pr_info("Unsupported type: %s\n", t);
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index f6398db09114..45400ca5ded1 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -102,6 +102,7 @@ enum {
 	FETCH_MTD_reg = 0,
 	FETCH_MTD_stack,
 	FETCH_MTD_retval,
+	FETCH_MTD_comm,
 	FETCH_MTD_memory,
 	FETCH_MTD_symbol,
 	FETCH_MTD_deref,
@@ -183,6 +184,14 @@ DECLARE_BASIC_FETCH_FUNCS(bitfield);
 #define fetch_bitfield_string			NULL
 #define fetch_bitfield_string_size		NULL
 
+/* comm only makes sense as a string */
+#define fetch_comm_u8		NULL
+#define fetch_comm_u16		NULL
+#define fetch_comm_u32		NULL
+#define fetch_comm_u64		NULL
+DECLARE_FETCH_FUNC(comm, string);
+DECLARE_FETCH_FUNC(comm, string_size);
+
 /*
  * Define macro for basic types - we don't need to define s* types, because
  * we have to care only about bitwidth at recording time.
@@ -213,6 +222,7 @@ DEFINE_FETCH_##method(u64)
 ASSIGN_FETCH_FUNC(reg, ftype),				\
 ASSIGN_FETCH_FUNC(stack, ftype),			\
 ASSIGN_FETCH_FUNC(retval, ftype),			\
+ASSIGN_FETCH_FUNC(comm, ftype),				\
 ASSIGN_FETCH_FUNC(memory, ftype),			\
 ASSIGN_FETCH_FUNC(symbol, ftype),			\
 ASSIGN_FETCH_FUNC(deref, ftype),			\
-- 
2.8.3

Re: [PATCH v3] tracing: expose current->comm to [ku]probe events

[Masami Hiramatsu]

On Wed,  8 Jun 2016 18:38:02 -0700
Omar Sandoval <osandov@osandov.com> wrote:

> From: Omar Sandoval <osandov@fb.com>
> 
> ftrace is very quick to give up on saving the task command line (see
> `trace_save_cmdline()`). The workaround for events which really care
> about the command line is to explicitly assign it as part of the entry.
> However, this doesn't work for kprobe events, as there's no
> straightforward way to get access to current->comm. Add a kprobe/uprobe
> event variable $comm which provides exactly that.
> 
> Signed-off-by: Omar Sandoval <osandov@fb.com>

Looks good to me:)

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

> ---
> Alexei pointed out to me that this same functionality can be acheived
> with BPF and the bpf_get_current_comm() helper, so strictly speaking,
> this patch isn't necessary. It's still convenient for ad-hoc stuff,
> though, so I figured I'd throw this up and see what everyone thought.

Anyway, I'd like to pull this patch since this still be useful
in handy way to get comm on ftrace :)

Thank you!

> 
>  Documentation/trace/kprobetrace.txt  |  3 +++
>  Documentation/trace/uprobetracer.txt |  3 +++
>  kernel/trace/trace_kprobe.c          |  1 +
>  kernel/trace/trace_probe.c           | 33 +++++++++++++++++++++++++++++++++
>  kernel/trace/trace_probe.h           | 10 ++++++++++
>  5 files changed, 50 insertions(+)
> 
> diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
> index d68ea5fc812b..ea52ec1f8484 100644
> --- a/Documentation/trace/kprobetrace.txt
> +++ b/Documentation/trace/kprobetrace.txt
> @@ -40,6 +40,7 @@ Synopsis of kprobe_events
>    $stackN	: Fetch Nth entry of stack (N >= 0)
>    $stack	: Fetch stack address.
>    $retval	: Fetch return value.(*)
> +  $comm		: Fetch current task comm.
>    +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
>    NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
>    FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
> @@ -62,6 +63,8 @@ offset, and container-size (usually 32). The syntax is;
>  
>   b<bit-width>@<bit-offset>/<container-size>
>  
> +For $comm, the default type is "string"; any other type is invalid.
> +
>  
>  Per-Probe Event Filtering
>  -------------------------
> diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
> index f1cf9a34ad9d..72d1cd4f7bf3 100644
> --- a/Documentation/trace/uprobetracer.txt
> +++ b/Documentation/trace/uprobetracer.txt
> @@ -36,6 +36,7 @@ Synopsis of uprobe_tracer
>     $stackN	: Fetch Nth entry of stack (N >= 0)
>     $stack	: Fetch stack address.
>     $retval	: Fetch return value.(*)
> +   $comm	: Fetch current task comm.
>     +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
>     NAME=FETCHARG     : Set NAME as the argument name of FETCHARG.
>     FETCHARG:TYPE     : Set TYPE as the type of FETCHARG. Currently, basic types
> @@ -57,6 +58,8 @@ offset, and container-size (usually 32). The syntax is;
>  
>   b<bit-width>@<bit-offset>/<container-size>
>  
> +For $comm, the default type is "string"; any other type is invalid.
> +
>  
>  Event Profiling
>  ---------------
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5546eec0505f..9aedb0b06683 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -587,6 +587,7 @@ static int create_trace_kprobe(int argc, char **argv)
>  	 *  $retval	: fetch return value
>  	 *  $stack	: fetch stack address
>  	 *  $stackN	: fetch Nth of stack (N:0-)
> +	 *  $comm       : fetch current task comm
>  	 *  @ADDR	: fetch memory at ADDR (ADDR should be in kernel)
>  	 *  @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data symbol)
>  	 *  %REG	: fetch register REG
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 1d372fa6fefb..74e80a582c28 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -218,6 +218,28 @@ free_bitfield_fetch_param(struct bitfield_fetch_param *data)
>  	kfree(data);
>  }
>  
> +void FETCH_FUNC_NAME(comm, string)(struct pt_regs *regs,
> +					  void *data, void *dest)
> +{
> +	int maxlen = get_rloc_len(*(u32 *)dest);
> +	u8 *dst = get_rloc_data(dest);
> +	long ret;
> +
> +	if (!maxlen)
> +		return;
> +
> +	ret = strlcpy(dst, current->comm, maxlen);
> +	*(u32 *)dest = make_data_rloc(ret, get_rloc_offs(*(u32 *)dest));
> +}
> +NOKPROBE_SYMBOL(FETCH_FUNC_NAME(comm, string));
> +
> +void FETCH_FUNC_NAME(comm, string_size)(struct pt_regs *regs,
> +					       void *data, void *dest)
> +{
> +	*(u32 *)dest = strlen(current->comm) + 1;
> +}
> +NOKPROBE_SYMBOL(FETCH_FUNC_NAME(comm, string_size));
> +
>  static const struct fetch_type *find_fetch_type(const char *type,
>  						const struct fetch_type *ftbl)
>  {
> @@ -348,6 +370,11 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
>  			}
>  		} else
>  			ret = -EINVAL;
> +	} else if (strcmp(arg, "comm") == 0) {
> +		if (strcmp(t->name, "string") != 0 &&
> +		    strcmp(t->name, "string_size") != 0)
> +			return -EINVAL;
> +		f->fn = t->fetch[FETCH_MTD_comm];
>  	} else
>  		ret = -EINVAL;
>  
> @@ -522,6 +549,12 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
>  		arg[t - parg->comm] = '\0';
>  		t++;
>  	}
> +	/*
> +	 * The default type of $comm should be "string", and it can't be
> +	 * dereferenced.
> +	 */
> +	if (!t && strcmp(arg, "$comm") == 0)
> +		t = "string";
>  	parg->type = find_fetch_type(t, ftbl);
>  	if (!parg->type) {
>  		pr_info("Unsupported type: %s\n", t);
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index f6398db09114..45400ca5ded1 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -102,6 +102,7 @@ enum {
>  	FETCH_MTD_reg = 0,
>  	FETCH_MTD_stack,
>  	FETCH_MTD_retval,
> +	FETCH_MTD_comm,
>  	FETCH_MTD_memory,
>  	FETCH_MTD_symbol,
>  	FETCH_MTD_deref,
> @@ -183,6 +184,14 @@ DECLARE_BASIC_FETCH_FUNCS(bitfield);
>  #define fetch_bitfield_string			NULL
>  #define fetch_bitfield_string_size		NULL
>  
> +/* comm only makes sense as a string */
> +#define fetch_comm_u8		NULL
> +#define fetch_comm_u16		NULL
> +#define fetch_comm_u32		NULL
> +#define fetch_comm_u64		NULL
> +DECLARE_FETCH_FUNC(comm, string);
> +DECLARE_FETCH_FUNC(comm, string_size);
> +
>  /*
>   * Define macro for basic types - we don't need to define s* types, because
>   * we have to care only about bitwidth at recording time.
> @@ -213,6 +222,7 @@ DEFINE_FETCH_##method(u64)
>  ASSIGN_FETCH_FUNC(reg, ftype),				\
>  ASSIGN_FETCH_FUNC(stack, ftype),			\
>  ASSIGN_FETCH_FUNC(retval, ftype),			\
> +ASSIGN_FETCH_FUNC(comm, ftype),				\
>  ASSIGN_FETCH_FUNC(memory, ftype),			\
>  ASSIGN_FETCH_FUNC(symbol, ftype),			\
>  ASSIGN_FETCH_FUNC(deref, ftype),			\
> -- 
> 2.8.3
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v3] tracing: expose current->comm to [ku]probe events

[Steven Rostedt]

On Thu, 9 Jun 2016 11:55:34 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Wed,  8 Jun 2016 18:38:02 -0700
> Omar Sandoval <osandov@osandov.com> wrote:
> 
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > ftrace is very quick to give up on saving the task command line (see
> > `trace_save_cmdline()`). The workaround for events which really care
> > about the command line is to explicitly assign it as part of the entry.
> > However, this doesn't work for kprobe events, as there's no
> > straightforward way to get access to current->comm. Add a kprobe/uprobe
> > event variable $comm which provides exactly that.
> > 
> > Signed-off-by: Omar Sandoval <osandov@fb.com>  
> 
> Looks good to me:)
> 
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> 
> > ---
> > Alexei pointed out to me that this same functionality can be acheived
> > with BPF and the bpf_get_current_comm() helper, so strictly speaking,
> > this patch isn't necessary. It's still convenient for ad-hoc stuff,
> > though, so I figured I'd throw this up and see what everyone thought.  
> 
> Anyway, I'd like to pull this patch since this still be useful
> in handy way to get comm on ftrace :)
> 

I pulled it.

Thanks!

-- Steve

Re: [PATCH v3] tracing: expose current->comm to [ku]probe events

[Omar Sandoval]

On Fri, Jun 17, 2016 at 12:19:27PM -0400, Steven Rostedt wrote:
> On Thu, 9 Jun 2016 11:55:34 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > On Wed,  8 Jun 2016 18:38:02 -0700
> > Omar Sandoval <osandov@osandov.com> wrote:
> > 
> > > From: Omar Sandoval <osandov@fb.com>
> > > 
> > > ftrace is very quick to give up on saving the task command line (see
> > > `trace_save_cmdline()`). The workaround for events which really care
> > > about the command line is to explicitly assign it as part of the entry.
> > > However, this doesn't work for kprobe events, as there's no
> > > straightforward way to get access to current->comm. Add a kprobe/uprobe
> > > event variable $comm which provides exactly that.
> > > 
> > > Signed-off-by: Omar Sandoval <osandov@fb.com>  
> > 
> > Looks good to me:)
> > 
> > Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> > 
> > > ---
> > > Alexei pointed out to me that this same functionality can be acheived
> > > with BPF and the bpf_get_current_comm() helper, so strictly speaking,
> > > this patch isn't necessary. It's still convenient for ad-hoc stuff,
> > > though, so I figured I'd throw this up and see what everyone thought.  
> > 
> > Anyway, I'd like to pull this patch since this still be useful
> > in handy way to get comm on ftrace :)
> > 
> 
> I pulled it.
> 
> Thanks!
> 
> -- Steve

Thanks, Steve!

-- 
Omar