On Fri, 19 Aug 2022 21:40:39 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
>
> Currently, if a symbol "@" is attempted to be used with an event probe
> (eprobes), it will cause a NULL pointer dereference crash.
>
> Both kprobes and uprobes can reference data other than the main registers.
> Such as immediate address, symbols and the current task name. Have eprobes
> do the same thing.
>
> For "comm", if "comm" is used and the event being attached to does not
> have the "comm" field, then make it the "$comm" that kprobes has. This is
> consistent to the way histograms and filters work.
Hmm, I think I would better allow user to use $COMM to get comm string
for kprobe/uprobe event users too. (There are many special variables...)
Anyway, this looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
Thank you!
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 7491e2c44278 ("tracing: Add a probe that attaches to trace events")
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
> kernel/trace/trace_eprobe.c | 67 +++++++++++++++++++++++++++++++++----
> 1 file changed, 61 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
> index a1d3423ab74f..63218a541217 100644
> --- a/kernel/trace/trace_eprobe.c
> +++ b/kernel/trace/trace_eprobe.c
> @@ -227,6 +227,7 @@ static int trace_eprobe_tp_arg_update(struct trace_eprobe *ep, int i)
> struct probe_arg *parg = &ep->tp.args[i];
> struct ftrace_event_field *field;
> struct list_head *head;
> + int ret = -ENOENT;
>
> head = trace_get_fields(ep->event);
> list_for_each_entry(field, head, link) {
> @@ -236,9 +237,17 @@ static int trace_eprobe_tp_arg_update(struct trace_eprobe *ep, int i)
> return 0;
> }
> }
> +
> + /* Argument no found on event. But allow for comm and COMM to be used */
> + if (strcmp(parg->code->data, "COMM") == 0 ||
> + strcmp(parg->code->data, "comm") == 0) {
> + parg->code->op = FETCH_OP_COMM;
> + ret = 0;
> + }
> +
> kfree(parg->code->data);
> parg->code->data = NULL;
> - return -ENOENT;
> + return ret;
> }
>
> static int eprobe_event_define_fields(struct trace_event_call *event_call)
> @@ -363,16 +372,38 @@ static unsigned long get_event_field(struct fetch_insn *code, void *rec)
>
> static int get_eprobe_size(struct trace_probe *tp, void *rec)
> {
> + struct fetch_insn *code;
> struct probe_arg *arg;
> int i, len, ret = 0;
>
> for (i = 0; i < tp->nr_args; i++) {
> arg = tp->args + i;
> - if (unlikely(arg->dynamic)) {
> + if (arg->dynamic) {
> unsigned long val;
>
> - val = get_event_field(arg->code, rec);
> - len = process_fetch_insn_bottom(arg->code + 1, val, NULL, NULL);
> + code = arg->code;
> + retry:
> + switch (code->op) {
> + case FETCH_OP_TP_ARG:
> + val = get_event_field(code, rec);
> + break;
> + case FETCH_OP_IMM:
> + val = code->immediate;
> + break;
> + case FETCH_OP_COMM:
> + val = (unsigned long)current->comm;
> + break;
> + case FETCH_OP_DATA:
> + val = (unsigned long)code->data;
> + break;
> + case FETCH_NOP_SYMBOL: /* Ignore a place holder */
> + code++;
> + goto retry;
> + default:
> + continue;
> + }
> + code++;
> + len = process_fetch_insn_bottom(code, val, NULL, NULL);
> if (len > 0)
> ret += len;
> }
> @@ -390,8 +421,28 @@ process_fetch_insn(struct fetch_insn *code, void *rec, void *dest,
> {
> unsigned long val;
>
> - val = get_event_field(code, rec);
> - return process_fetch_insn_bottom(code + 1, val, dest, base);
> + retry:
> + switch (code->op) {
> + case FETCH_OP_TP_ARG:
> + val = get_event_field(code, rec);
> + break;
> + case FETCH_OP_IMM:
> + val = code->immediate;
> + break;
> + case FETCH_OP_COMM:
> + val = (unsigned long)current->comm;
> + break;
> + case FETCH_OP_DATA:
> + val = (unsigned long)code->data;
> + break;
> + case FETCH_NOP_SYMBOL: /* Ignore a place holder */
> + code++;
> + goto retry;
> + default:
> + return -EILSEQ;
> + }
> + code++;
> + return process_fetch_insn_bottom(code, val, dest, base);
> }
> NOKPROBE_SYMBOL(process_fetch_insn)
>
> @@ -866,6 +917,10 @@ static int trace_eprobe_tp_update_arg(struct trace_eprobe *ep, const char *argv[
> trace_probe_log_err(0, BAD_ATTACH_ARG);
> }
>
> + /* Handle symbols "@" */
> + if (!ret)
> + ret = traceprobe_update_arg(&ep->tp.args[i]);
> +
> return ret;
> }
>
> --
> 2.35.1
--
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
