| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CADWwUUacWn60oqddG8kPjGpJsg0GJWAy0VMhaG25QpfzjxCBBA@mail.gmail.com>
Date: Fri, 3 Jun 2016 00:40:02 +0900
From: Namhyung Kim <namhyung@...il.com>
To: Omar Sandoval <osandov@...ndov.com>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Ingo Molnar <mingo@...hat.com>,
Masami Hiramatsu <mhiramat@...nel.org>,
LKML <linux-kernel@...r.kernel.org>, kernel-team@...com,
Omar Sandoval <osandov@...com>
Subject: Re: [PATCH] tracing: expose current->comm to kprobe events
Hello,
On Wed, Jun 1, 2016 at 3:17 PM, Omar Sandoval <osandov@...ndov.com> wrote:
> From: Omar Sandoval <osandov@...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 event
> variable $comm which provides exactly that.
>
> Signed-off-by: Omar Sandoval <osandov@...com>
> ---
> This is a problem I ran into while trying to debug an unrelated issue. I
> was considering trying to make ftrace try harder to save the command
> line, but that would most likely be at the expense at most events which
> don't care about the task comm. This is much less work.
>
> Documentation/trace/kprobetrace.txt | 2 ++
> kernel/trace/trace_kprobe.c | 13 +++++++++++++
> kernel/trace/trace_probe.c | 5 +++++
> kernel/trace/trace_probe.h | 2 ++
> kernel/trace/trace_uprobe.c | 7 +++++++
> 5 files changed, 29 insertions(+)
>
> diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
> index d68ea5fc812b..90914cbc1e51 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
> @@ -48,6 +49,7 @@ Synopsis of kprobe_events
>
> (*) only for return probe.
> (**) this is useful for fetching a field of data structures.
> + (***) you probably want this as a string, i.e., $comm:string
>
> Types
> -----
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5546eec0505f..e3a087552d49 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -214,6 +214,18 @@ static void FETCH_FUNC_NAME(memory, string_size)(struct pt_regs *regs,
> }
> NOKPROBE_SYMBOL(FETCH_FUNC_NAME(memory, string_size));
>
> +#define DEFINE_FETCH_comm(type) \
> +void FETCH_FUNC_NAME(comm, type)(struct pt_regs *regs, \
> + void *data, void *dest) \
> +{ \
> + fetch_memory_##type(regs, current->comm, dest); \
> +} \
> +NOKPROBE_SYMBOL(FETCH_FUNC_NAME(comm, type));
> +
> +DEFINE_BASIC_FETCH_FUNCS(comm)
> +DEFINE_FETCH_comm(string)
> +DEFINE_FETCH_comm(string_size)
I think you can define default type fetch functions (such as u8, ...)
to NULL to make sure it's used as string type only.
> +
> #define DEFINE_FETCH_symbol(type) \
> void FETCH_FUNC_NAME(symbol, type)(struct pt_regs *regs, void *data, void *dest)\
> { \
> @@ -587,6 +599,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..918775fcacd3 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -348,6 +348,11 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
> }
> } else
> ret = -EINVAL;
> + } else if (strcmp(arg, "comm") == 0) {
> + if (is_kprobe)
> + f->fn = t->fetch[FETCH_MTD_comm];
> + else
> + ret = -EINVAL;
I think it should work with uprobes too. Why not enabling it to uprobes?
Thanks,
Namhyung
> } else
> ret = -EINVAL;
>
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index f6398db09114..3f1a987e6dc1 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,
> @@ -213,6 +214,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), \
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index c53485441c88..f7eedb191d1f 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -172,6 +172,13 @@ static void FETCH_FUNC_NAME(memory, string_size)(struct pt_regs *regs,
> *(u32 *)dest = len;
> }
>
> +#define fetch_comm_u8 NULL
> +#define fetch_comm_u16 NULL
> +#define fetch_comm_u32 NULL
> +#define fetch_comm_u64 NULL
> +#define fetch_comm_string NULL
> +#define fetch_comm_string_size NULL
> +
> static unsigned long translate_user_vaddr(void *file_offset)
> {
> unsigned long base_addr;
> --
> 2.8.3
>
Powered by blists - more mailing lists