* [PATCH 0/4] tracing/eprobes: Fixes for unexpected arguments
@ 2022-08-20 1:40 Steven Rostedt
2022-08-20 1:40 ` [PATCH 1/4] tracing/eprobes: Do not allow eprobes to use $stack, or % for regs Steven Rostedt
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Steven Rostedt @ 2022-08-20 1:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tzvetomir Stoyanov,
Tom Zanussi
While using eprobes, I decided to entertain the thougth of what would
happen if I tried to get the instruction pointer "%rip", knowing full
well that eprobes do not have access to pt_regs. Well, I found out, and
it led me down a rabbit hole of bugs.
This series fixes those bugs, by not allowing register access for eprobes,
and also filling the holes of @symbol and @immediate argument.
Steven Rostedt (Google) (4):
tracing/eprobes: Do not allow eprobes to use $stack, or % for regs
tracing/eprobes: Do not hardcode $comm as a string
tracing/eprobes: Fix reading of string fields
tracing/eprobes: Have event probes be consistent with kprobes and uprobes
----
kernel/trace/trace_eprobe.c | 88 +++++++++++++++++++++++++++++++++++++++++----
kernel/trace/trace_probe.c | 26 ++++++++------
2 files changed, 98 insertions(+), 16 deletions(-)
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/4] tracing/eprobes: Do not allow eprobes to use $stack, or % for regs
2022-08-20 1:40 [PATCH 0/4] tracing/eprobes: Fixes for unexpected arguments Steven Rostedt
@ 2022-08-20 1:40 ` Steven Rostedt
2022-08-20 8:33 ` Masami Hiramatsu
2022-08-20 1:40 ` [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string Steven Rostedt
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Steven Rostedt @ 2022-08-20 1:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tzvetomir Stoyanov,
Tom Zanussi, stable
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
While playing with event probes (eprobes), I tried to see what would
happen if I attempted to retrieve the instruction pointer (%rip) knowing
that event probes do not use pt_regs. The result was:
BUG: kernel NULL pointer dereference, address: 0000000000000024
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0
Oops: 0000 [#1] PREEMPT SMP PTI
CPU: 1 PID: 1847 Comm: trace-cmd Not tainted 5.19.0-rc5-test+ #309
Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01
v03.03 07/14/2016
RIP: 0010:get_event_field.isra.0+0x0/0x50
Code: ff 48 c7 c7 c0 8f 74 a1 e8 3d 8b f5 ff e8 88 09 f6 ff 4c 89 e7 e8
50 6a 13 00 48 89 ef 5b 5d 41 5c 41 5d e9 42 6a 13 00 66 90 <48> 63 47 24
8b 57 2c 48 01 c6 8b 47 28 83 f8 02 74 0e 83 f8 04 74
RSP: 0018:ffff916c394bbaf0 EFLAGS: 00010086
RAX: ffff916c854041d8 RBX: ffff916c8d9fbf50 RCX: ffff916c255d2000
RDX: 0000000000000000 RSI: ffff916c255d2008 RDI: 0000000000000000
RBP: 0000000000000000 R08: ffff916c3a2a0c08 R09: ffff916c394bbda8
R10: 0000000000000000 R11: 0000000000000000 R12: ffff916c854041d8
R13: ffff916c854041b0 R14: 0000000000000000 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff916c9ea40000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000024 CR3: 000000011b60a002 CR4: 00000000001706e0
Call Trace:
<TASK>
get_eprobe_size+0xb4/0x640
? __mod_node_page_state+0x72/0xc0
__eprobe_trace_func+0x59/0x1a0
? __mod_lruvec_page_state+0xaa/0x1b0
? page_remove_file_rmap+0x14/0x230
? page_remove_rmap+0xda/0x170
event_triggers_call+0x52/0xe0
trace_event_buffer_commit+0x18f/0x240
trace_event_raw_event_sched_wakeup_template+0x7a/0xb0
try_to_wake_up+0x260/0x4c0
__wake_up_common+0x80/0x180
__wake_up_common_lock+0x7c/0xc0
do_notify_parent+0x1c9/0x2a0
exit_notify+0x1a9/0x220
do_exit+0x2ba/0x450
do_group_exit+0x2d/0x90
__x64_sys_exit_group+0x14/0x20
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x46/0xb0
Obviously this is not the desired result.
Move the testing for TPARG_FL_TPOINT which is only used for event probes
to the top of the "$" variable check, as all the other variables are not
used for event probes. Also add a check in the register parsing "%" to
fail if an event probe is used.
Cc: stable@vger.kernel.org
Fixes: 7491e2c44278 ("tracing: Add a probe that attaches to trace events")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_probe.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 850a88abd33b..dec657af363c 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -283,7 +283,14 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
int ret = 0;
int len;
- if (strcmp(arg, "retval") == 0) {
+ if (flags & TPARG_FL_TPOINT) {
+ if (code->data)
+ return -EFAULT;
+ code->data = kstrdup(arg, GFP_KERNEL);
+ if (!code->data)
+ return -ENOMEM;
+ code->op = FETCH_OP_TP_ARG;
+ } else if (strcmp(arg, "retval") == 0) {
if (flags & TPARG_FL_RETURN) {
code->op = FETCH_OP_RETVAL;
} else {
@@ -323,13 +330,6 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
code->op = FETCH_OP_ARG;
code->param = (unsigned int)param - 1;
#endif
- } else if (flags & TPARG_FL_TPOINT) {
- if (code->data)
- return -EFAULT;
- code->data = kstrdup(arg, GFP_KERNEL);
- if (!code->data)
- return -ENOMEM;
- code->op = FETCH_OP_TP_ARG;
} else
goto inval_var;
@@ -384,6 +384,11 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
break;
case '%': /* named register */
+ if (flags & TPARG_FL_TPOINT) {
+ /* eprobes do not handle registers */
+ trace_probe_log_err(offs, BAD_VAR);
+ break;
+ }
ret = regs_query_register_offset(arg + 1);
if (ret >= 0) {
code->op = FETCH_OP_REG;
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string
2022-08-20 1:40 [PATCH 0/4] tracing/eprobes: Fixes for unexpected arguments Steven Rostedt
2022-08-20 1:40 ` [PATCH 1/4] tracing/eprobes: Do not allow eprobes to use $stack, or % for regs Steven Rostedt
@ 2022-08-20 1:40 ` Steven Rostedt
2022-08-20 1:57 ` Steven Rostedt
` (2 more replies)
2022-08-20 1:40 ` [PATCH 3/4] tracing/eprobes: Fix reading of string fields Steven Rostedt
2022-08-20 1:40 ` [PATCH 4/4] tracing/eprobes: Have event probes be consistent with kprobes and uprobes Steven Rostedt
3 siblings, 3 replies; 16+ messages in thread
From: Steven Rostedt @ 2022-08-20 1:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tzvetomir Stoyanov,
Tom Zanussi, stable
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
The variable $comm is hard coded as a string, which is true for both
kprobes and uprobes, but for event probes (eprobes) it is a field name. In
most cases the "comm" field would be a string, but there's no guarantee of
that fact.
Do not assume that comm is a string. Not to mention, it currently forces
comm fields to fault, as string processing for event probes is currently
broken.
Cc: stable@vger.kernel.org
Fixes: 7491e2c44278 ("tracing: Add a probe that attaches to trace events")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_probe.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index dec657af363c..23dcd52ad45c 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -622,9 +622,10 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
/*
* Since $comm and immediate string can not be dereferenced,
- * we can find those by strcmp.
+ * we can find those by strcmp. But ignore for eprobes.
*/
- if (strcmp(arg, "$comm") == 0 || strncmp(arg, "\\\"", 2) == 0) {
+ if (!(flags & TPARG_FL_TPOINT) &&
+ strcmp(arg, "$comm") == 0 || strncmp(arg, "\\\"", 2) == 0) {
/* The type of $comm must be "string", and not an array. */
if (parg->count || (t && strcmp(t, "string")))
goto out;
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/4] tracing/eprobes: Fix reading of string fields
2022-08-20 1:40 [PATCH 0/4] tracing/eprobes: Fixes for unexpected arguments Steven Rostedt
2022-08-20 1:40 ` [PATCH 1/4] tracing/eprobes: Do not allow eprobes to use $stack, or % for regs Steven Rostedt
2022-08-20 1:40 ` [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string Steven Rostedt
@ 2022-08-20 1:40 ` Steven Rostedt
2022-08-20 12:27 ` Masami Hiramatsu
2022-08-20 1:40 ` [PATCH 4/4] tracing/eprobes: Have event probes be consistent with kprobes and uprobes Steven Rostedt
3 siblings, 1 reply; 16+ messages in thread
From: Steven Rostedt @ 2022-08-20 1:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tzvetomir Stoyanov,
Tom Zanussi, stable
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
Currently when an event probe (eprobe) hooks to a string field, it does
not display it as a string, but instead as a number. This makes the field
rather useless. Handle the different kinds of strings, dynamic, static,
relational/dynamic etc.
Now when a string field is used, the ":string" type can be used to display
it:
echo "e:sw sched/sched_switch comm=$next_comm:string" > dynamic_events
Cc: stable@vger.kernel.org
Fixes: 7491e2c44278 ("tracing: Add a probe that attaches to trace events")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_eprobe.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index 550671985fd1..a1d3423ab74f 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -311,6 +311,27 @@ static unsigned long get_event_field(struct fetch_insn *code, void *rec)
addr = rec + field->offset;
+ if (is_string_field(field)) {
+ switch (field->filter_type) {
+ case FILTER_DYN_STRING:
+ val = (unsigned long)(rec + (*(unsigned int *)addr & 0xffff));
+ break;
+ case FILTER_RDYN_STRING:
+ val = (unsigned long)(addr + (*(unsigned int *)addr & 0xffff));
+ break;
+ case FILTER_STATIC_STRING:
+ val = (unsigned long)addr;
+ break;
+ case FILTER_PTR_STRING:
+ val = (unsigned long)(*(char *)addr);
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ return 0;
+ }
+ return val;
+ }
+
switch (field->size) {
case 1:
if (field->is_signed)
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/4] tracing/eprobes: Have event probes be consistent with kprobes and uprobes
2022-08-20 1:40 [PATCH 0/4] tracing/eprobes: Fixes for unexpected arguments Steven Rostedt
` (2 preceding siblings ...)
2022-08-20 1:40 ` [PATCH 3/4] tracing/eprobes: Fix reading of string fields Steven Rostedt
@ 2022-08-20 1:40 ` Steven Rostedt
2022-08-20 13:04 ` Masami Hiramatsu
3 siblings, 1 reply; 16+ messages in thread
From: Steven Rostedt @ 2022-08-20 1:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tzvetomir Stoyanov,
Tom Zanussi, stable
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
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.
Cc: stable@vger.kernel.org
Fixes: 7491e2c44278 ("tracing: Add a probe that attaches to trace events")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string
2022-08-20 1:40 ` [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string Steven Rostedt
@ 2022-08-20 1:57 ` Steven Rostedt
2022-08-20 4:28 ` kernel test robot
2022-08-20 11:18 ` Masami Hiramatsu
2 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2022-08-20 1:57 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Masami Hiramatsu, Tzvetomir Stoyanov,
Tom Zanussi, stable
On Fri, 19 Aug 2022 21:40:37 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> +++ b/kernel/trace/trace_probe.c
> @@ -622,9 +622,10 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
>
> /*
> * Since $comm and immediate string can not be dereferenced,
> - * we can find those by strcmp.
> + * we can find those by strcmp. But ignore for eprobes.
> */
> - if (strcmp(arg, "$comm") == 0 || strncmp(arg, "\\\"", 2) == 0) {
> + if (!(flags & TPARG_FL_TPOINT) &&
> + strcmp(arg, "$comm") == 0 || strncmp(arg, "\\\"", 2) == 0) {
And my tests fail shortly after I send this. It complains about a new
warning. The above needs parenthesis around it.
Will send a v2 after my tests pass, in case it finds something else I
missed.
-- Steve
> /* The type of $comm must be "string", and not an array. */
> if (parg->count || (t && strcmp(t, "string")))
> goto out;
> --
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string
2022-08-20 1:40 ` [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string Steven Rostedt
2022-08-20 1:57 ` Steven Rostedt
@ 2022-08-20 4:28 ` kernel test robot
2022-08-20 11:18 ` Masami Hiramatsu
2 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2022-08-20 4:28 UTC (permalink / raw)
To: Steven Rostedt; +Cc: llvm, kbuild-all
Hi Steven,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on rostedt-trace/for-next]
[also build test WARNING on akpm-mm/mm-everything linus/master v6.0-rc1 next-20220819]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Steven-Rostedt/tracing-eprobes-Fixes-for-unexpected-arguments/20220820-095006
base: https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: arm64-buildonly-randconfig-r002-20220820 (https://download.01.org/0day-ci/archive/20220820/202208201225.4wDs2D5d-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project c9a41fe60ab62f7a40049c100adcc8087a47669b)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/8afe7ca161a3e4f90f4851927005e15f8bd5c094
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Steven-Rostedt/tracing-eprobes-Fixes-for-unexpected-arguments/20220820-095006
git checkout 8afe7ca161a3e4f90f4851927005e15f8bd5c094
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash kernel/trace/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> kernel/trace/trace_probe.c:627:33: warning: '&&' within '||' [-Wlogical-op-parentheses]
if (!(flags & TPARG_FL_TPOINT) &&
~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
kernel/trace/trace_probe.c:627:33: note: place parentheses around the '&&' expression to silence this warning
if (!(flags & TPARG_FL_TPOINT) &&
^
(
1 warning generated.
vim +627 kernel/trace/trace_probe.c
562
563 /* String length checking wrapper */
564 static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
565 struct probe_arg *parg, unsigned int flags, int offset)
566 {
567 struct fetch_insn *code, *scode, *tmp = NULL;
568 char *t, *t2, *t3;
569 char *arg;
570 int ret, len;
571
572 arg = kstrdup(argv, GFP_KERNEL);
573 if (!arg)
574 return -ENOMEM;
575
576 ret = -EINVAL;
577 len = strlen(arg);
578 if (len > MAX_ARGSTR_LEN) {
579 trace_probe_log_err(offset, ARG_TOO_LONG);
580 goto out;
581 } else if (len == 0) {
582 trace_probe_log_err(offset, NO_ARG_BODY);
583 goto out;
584 }
585
586 ret = -ENOMEM;
587 parg->comm = kstrdup(arg, GFP_KERNEL);
588 if (!parg->comm)
589 goto out;
590
591 ret = -EINVAL;
592 t = strchr(arg, ':');
593 if (t) {
594 *t = '\0';
595 t2 = strchr(++t, '[');
596 if (t2) {
597 *t2++ = '\0';
598 t3 = strchr(t2, ']');
599 if (!t3) {
600 offset += t2 + strlen(t2) - arg;
601 trace_probe_log_err(offset,
602 ARRAY_NO_CLOSE);
603 goto out;
604 } else if (t3[1] != '\0') {
605 trace_probe_log_err(offset + t3 + 1 - arg,
606 BAD_ARRAY_SUFFIX);
607 goto out;
608 }
609 *t3 = '\0';
610 if (kstrtouint(t2, 0, &parg->count) || !parg->count) {
611 trace_probe_log_err(offset + t2 - arg,
612 BAD_ARRAY_NUM);
613 goto out;
614 }
615 if (parg->count > MAX_ARRAY_LEN) {
616 trace_probe_log_err(offset + t2 - arg,
617 ARRAY_TOO_BIG);
618 goto out;
619 }
620 }
621 }
622
623 /*
624 * Since $comm and immediate string can not be dereferenced,
625 * we can find those by strcmp. But ignore for eprobes.
626 */
> 627 if (!(flags & TPARG_FL_TPOINT) &&
628 strcmp(arg, "$comm") == 0 || strncmp(arg, "\\\"", 2) == 0) {
629 /* The type of $comm must be "string", and not an array. */
630 if (parg->count || (t && strcmp(t, "string")))
631 goto out;
632 parg->type = find_fetch_type("string");
633 } else
634 parg->type = find_fetch_type(t);
635 if (!parg->type) {
636 trace_probe_log_err(offset + (t ? (t - arg) : 0), BAD_TYPE);
637 goto out;
638 }
639 parg->offset = *size;
640 *size += parg->type->size * (parg->count ?: 1);
641
642 ret = -ENOMEM;
643 if (parg->count) {
644 len = strlen(parg->type->fmttype) + 6;
645 parg->fmt = kmalloc(len, GFP_KERNEL);
646 if (!parg->fmt)
647 goto out;
648 snprintf(parg->fmt, len, "%s[%d]", parg->type->fmttype,
649 parg->count);
650 }
651
652 code = tmp = kcalloc(FETCH_INSN_MAX, sizeof(*code), GFP_KERNEL);
653 if (!code)
654 goto out;
655 code[FETCH_INSN_MAX - 1].op = FETCH_OP_END;
656
657 ret = parse_probe_arg(arg, parg->type, &code, &code[FETCH_INSN_MAX - 1],
658 flags, offset);
659 if (ret)
660 goto fail;
661
662 ret = -EINVAL;
663 /* Store operation */
664 if (!strcmp(parg->type->name, "string") ||
665 !strcmp(parg->type->name, "ustring")) {
666 if (code->op != FETCH_OP_DEREF && code->op != FETCH_OP_UDEREF &&
667 code->op != FETCH_OP_IMM && code->op != FETCH_OP_COMM &&
668 code->op != FETCH_OP_DATA && code->op != FETCH_OP_TP_ARG) {
669 trace_probe_log_err(offset + (t ? (t - arg) : 0),
670 BAD_STRING);
671 goto fail;
672 }
673 if ((code->op == FETCH_OP_IMM || code->op == FETCH_OP_COMM ||
674 code->op == FETCH_OP_DATA) || code->op == FETCH_OP_TP_ARG ||
675 parg->count) {
676 /*
677 * IMM, DATA and COMM is pointing actual address, those
678 * must be kept, and if parg->count != 0, this is an
679 * array of string pointers instead of string address
680 * itself.
681 */
682 code++;
683 if (code->op != FETCH_OP_NOP) {
684 trace_probe_log_err(offset, TOO_MANY_OPS);
685 goto fail;
686 }
687 }
688 /* If op == DEREF, replace it with STRING */
689 if (!strcmp(parg->type->name, "ustring") ||
690 code->op == FETCH_OP_UDEREF)
691 code->op = FETCH_OP_ST_USTRING;
692 else
693 code->op = FETCH_OP_ST_STRING;
694 code->size = parg->type->size;
695 parg->dynamic = true;
696 } else if (code->op == FETCH_OP_DEREF) {
697 code->op = FETCH_OP_ST_MEM;
698 code->size = parg->type->size;
699 } else if (code->op == FETCH_OP_UDEREF) {
700 code->op = FETCH_OP_ST_UMEM;
701 code->size = parg->type->size;
702 } else {
703 code++;
704 if (code->op != FETCH_OP_NOP) {
705 trace_probe_log_err(offset, TOO_MANY_OPS);
706 goto fail;
707 }
708 code->op = FETCH_OP_ST_RAW;
709 code->size = parg->type->size;
710 }
711 scode = code;
712 /* Modify operation */
713 if (t != NULL) {
714 ret = __parse_bitfield_probe_arg(t, parg->type, &code);
715 if (ret) {
716 trace_probe_log_err(offset + t - arg, BAD_BITFIELD);
717 goto fail;
718 }
719 }
720 ret = -EINVAL;
721 /* Loop(Array) operation */
722 if (parg->count) {
723 if (scode->op != FETCH_OP_ST_MEM &&
724 scode->op != FETCH_OP_ST_STRING &&
725 scode->op != FETCH_OP_ST_USTRING) {
726 trace_probe_log_err(offset + (t ? (t - arg) : 0),
727 BAD_STRING);
728 goto fail;
729 }
730 code++;
731 if (code->op != FETCH_OP_NOP) {
732 trace_probe_log_err(offset, TOO_MANY_OPS);
733 goto fail;
734 }
735 code->op = FETCH_OP_LP_ARRAY;
736 code->param = parg->count;
737 }
738 code++;
739 code->op = FETCH_OP_END;
740
741 ret = 0;
742 /* Shrink down the code buffer */
743 parg->code = kcalloc(code - tmp + 1, sizeof(*code), GFP_KERNEL);
744 if (!parg->code)
745 ret = -ENOMEM;
746 else
747 memcpy(parg->code, tmp, sizeof(*code) * (code - tmp + 1));
748
749 fail:
750 if (ret) {
751 for (code = tmp; code < tmp + FETCH_INSN_MAX; code++)
752 if (code->op == FETCH_NOP_SYMBOL ||
753 code->op == FETCH_OP_DATA)
754 kfree(code->data);
755 }
756 kfree(tmp);
757 out:
758 kfree(arg);
759
760 return ret;
761 }
762
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] tracing/eprobes: Do not allow eprobes to use $stack, or % for regs
2022-08-20 1:40 ` [PATCH 1/4] tracing/eprobes: Do not allow eprobes to use $stack, or % for regs Steven Rostedt
@ 2022-08-20 8:33 ` Masami Hiramatsu
0 siblings, 0 replies; 16+ messages in thread
From: Masami Hiramatsu @ 2022-08-20 8:33 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Masami Hiramatsu,
Tzvetomir Stoyanov, Tom Zanussi, stable
On Fri, 19 Aug 2022 21:40:36 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>
> While playing with event probes (eprobes), I tried to see what would
> happen if I attempted to retrieve the instruction pointer (%rip) knowing
> that event probes do not use pt_regs. The result was:
>
> BUG: kernel NULL pointer dereference, address: 0000000000000024
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 0 P4D 0
> Oops: 0000 [#1] PREEMPT SMP PTI
> CPU: 1 PID: 1847 Comm: trace-cmd Not tainted 5.19.0-rc5-test+ #309
> Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01
> v03.03 07/14/2016
> RIP: 0010:get_event_field.isra.0+0x0/0x50
> Code: ff 48 c7 c7 c0 8f 74 a1 e8 3d 8b f5 ff e8 88 09 f6 ff 4c 89 e7 e8
> 50 6a 13 00 48 89 ef 5b 5d 41 5c 41 5d e9 42 6a 13 00 66 90 <48> 63 47 24
> 8b 57 2c 48 01 c6 8b 47 28 83 f8 02 74 0e 83 f8 04 74
> RSP: 0018:ffff916c394bbaf0 EFLAGS: 00010086
> RAX: ffff916c854041d8 RBX: ffff916c8d9fbf50 RCX: ffff916c255d2000
> RDX: 0000000000000000 RSI: ffff916c255d2008 RDI: 0000000000000000
> RBP: 0000000000000000 R08: ffff916c3a2a0c08 R09: ffff916c394bbda8
> R10: 0000000000000000 R11: 0000000000000000 R12: ffff916c854041d8
> R13: ffff916c854041b0 R14: 0000000000000000 R15: 0000000000000000
> FS: 0000000000000000(0000) GS:ffff916c9ea40000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000024 CR3: 000000011b60a002 CR4: 00000000001706e0
> Call Trace:
> <TASK>
> get_eprobe_size+0xb4/0x640
> ? __mod_node_page_state+0x72/0xc0
> __eprobe_trace_func+0x59/0x1a0
> ? __mod_lruvec_page_state+0xaa/0x1b0
> ? page_remove_file_rmap+0x14/0x230
> ? page_remove_rmap+0xda/0x170
> event_triggers_call+0x52/0xe0
> trace_event_buffer_commit+0x18f/0x240
> trace_event_raw_event_sched_wakeup_template+0x7a/0xb0
> try_to_wake_up+0x260/0x4c0
> __wake_up_common+0x80/0x180
> __wake_up_common_lock+0x7c/0xc0
> do_notify_parent+0x1c9/0x2a0
> exit_notify+0x1a9/0x220
> do_exit+0x2ba/0x450
> do_group_exit+0x2d/0x90
> __x64_sys_exit_group+0x14/0x20
> do_syscall_64+0x3b/0x90
> entry_SYSCALL_64_after_hwframe+0x46/0xb0
>
> Obviously this is not the desired result.
>
> Move the testing for TPARG_FL_TPOINT which is only used for event probes
> to the top of the "$" variable check, as all the other variables are not
> used for event probes. Also add a check in the register parsing "%" to
> fail if an event probe is used.
This looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks!
>
> Cc: stable@vger.kernel.org
> Fixes: 7491e2c44278 ("tracing: Add a probe that attaches to trace events")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> kernel/trace/trace_probe.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 850a88abd33b..dec657af363c 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -283,7 +283,14 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
> int ret = 0;
> int len;
>
> - if (strcmp(arg, "retval") == 0) {
> + if (flags & TPARG_FL_TPOINT) {
> + if (code->data)
> + return -EFAULT;
> + code->data = kstrdup(arg, GFP_KERNEL);
> + if (!code->data)
> + return -ENOMEM;
> + code->op = FETCH_OP_TP_ARG;
> + } else if (strcmp(arg, "retval") == 0) {
> if (flags & TPARG_FL_RETURN) {
> code->op = FETCH_OP_RETVAL;
> } else {
> @@ -323,13 +330,6 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
> code->op = FETCH_OP_ARG;
> code->param = (unsigned int)param - 1;
> #endif
> - } else if (flags & TPARG_FL_TPOINT) {
> - if (code->data)
> - return -EFAULT;
> - code->data = kstrdup(arg, GFP_KERNEL);
> - if (!code->data)
> - return -ENOMEM;
> - code->op = FETCH_OP_TP_ARG;
> } else
> goto inval_var;
>
> @@ -384,6 +384,11 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
> break;
>
> case '%': /* named register */
> + if (flags & TPARG_FL_TPOINT) {
> + /* eprobes do not handle registers */
> + trace_probe_log_err(offs, BAD_VAR);
> + break;
> + }
> ret = regs_query_register_offset(arg + 1);
> if (ret >= 0) {
> code->op = FETCH_OP_REG;
> --
> 2.35.1
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string
2022-08-20 1:40 ` [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string Steven Rostedt
2022-08-20 1:57 ` Steven Rostedt
2022-08-20 4:28 ` kernel test robot
@ 2022-08-20 11:18 ` Masami Hiramatsu
2022-08-20 12:48 ` Steven Rostedt
2 siblings, 1 reply; 16+ messages in thread
From: Masami Hiramatsu @ 2022-08-20 11:18 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Masami Hiramatsu,
Tzvetomir Stoyanov, Tom Zanussi, stable
On Fri, 19 Aug 2022 21:40:37 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>
> The variable $comm is hard coded as a string, which is true for both
> kprobes and uprobes, but for event probes (eprobes) it is a field name. In
> most cases the "comm" field would be a string, but there's no guarantee of
> that fact.
>
> Do not assume that comm is a string. Not to mention, it currently forces
> comm fields to fault, as string processing for event probes is currently
> broken.
Indeed. There should be an event argument which names "comm".
Eprobe might refer it. BTW, does eprobe use any special common fields?
I originally introduced "$" variable for such special variables.
Thank you,
>
> Cc: stable@vger.kernel.org
> Fixes: 7491e2c44278 ("tracing: Add a probe that attaches to trace events")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> kernel/trace/trace_probe.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index dec657af363c..23dcd52ad45c 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -622,9 +622,10 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
>
> /*
> * Since $comm and immediate string can not be dereferenced,
> - * we can find those by strcmp.
> + * we can find those by strcmp. But ignore for eprobes.
> */
> - if (strcmp(arg, "$comm") == 0 || strncmp(arg, "\\\"", 2) == 0) {
> + if (!(flags & TPARG_FL_TPOINT) &&
> + strcmp(arg, "$comm") == 0 || strncmp(arg, "\\\"", 2) == 0) {
> /* The type of $comm must be "string", and not an array. */
> if (parg->count || (t && strcmp(t, "string")))
> goto out;
> --
> 2.35.1
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] tracing/eprobes: Fix reading of string fields
2022-08-20 1:40 ` [PATCH 3/4] tracing/eprobes: Fix reading of string fields Steven Rostedt
@ 2022-08-20 12:27 ` Masami Hiramatsu
0 siblings, 0 replies; 16+ messages in thread
From: Masami Hiramatsu @ 2022-08-20 12:27 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Masami Hiramatsu,
Tzvetomir Stoyanov, Tom Zanussi, stable
On Fri, 19 Aug 2022 21:40:38 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>
> Currently when an event probe (eprobe) hooks to a string field, it does
> not display it as a string, but instead as a number. This makes the field
> rather useless. Handle the different kinds of strings, dynamic, static,
> relational/dynamic etc.
>
> Now when a string field is used, the ":string" type can be used to display
> it:
>
> echo "e:sw sched/sched_switch comm=$next_comm:string" > dynamic_events
Really nice!
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thank you!
>
> Cc: stable@vger.kernel.org
> Fixes: 7491e2c44278 ("tracing: Add a probe that attaches to trace events")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> kernel/trace/trace_eprobe.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
> index 550671985fd1..a1d3423ab74f 100644
> --- a/kernel/trace/trace_eprobe.c
> +++ b/kernel/trace/trace_eprobe.c
> @@ -311,6 +311,27 @@ static unsigned long get_event_field(struct fetch_insn *code, void *rec)
>
> addr = rec + field->offset;
>
> + if (is_string_field(field)) {
> + switch (field->filter_type) {
> + case FILTER_DYN_STRING:
> + val = (unsigned long)(rec + (*(unsigned int *)addr & 0xffff));
> + break;
> + case FILTER_RDYN_STRING:
> + val = (unsigned long)(addr + (*(unsigned int *)addr & 0xffff));
> + break;
> + case FILTER_STATIC_STRING:
> + val = (unsigned long)addr;
> + break;
> + case FILTER_PTR_STRING:
> + val = (unsigned long)(*(char *)addr);
> + break;
> + default:
> + WARN_ON_ONCE(1);
> + return 0;
> + }
> + return val;
> + }
> +
> switch (field->size) {
> case 1:
> if (field->is_signed)
> --
> 2.35.1
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string
2022-08-20 11:18 ` Masami Hiramatsu
@ 2022-08-20 12:48 ` Steven Rostedt
2022-08-20 13:00 ` Steven Rostedt
0 siblings, 1 reply; 16+ messages in thread
From: Steven Rostedt @ 2022-08-20 12:48 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Tzvetomir Stoyanov,
Tom Zanussi, stable
On Sat, 20 Aug 2022 20:18:24 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > Do not assume that comm is a string. Not to mention, it currently forces
> > comm fields to fault, as string processing for event probes is currently
> > broken.
>
> Indeed. There should be an event argument which names "comm".
> Eprobe might refer it. BTW, does eprobe use any special common fields?
> I originally introduced "$" variable for such special variables.
I used the '$' for denoting the fields, as it was the easiest way to
integrate with trace_probe.c. There's no special variables, but this patch
series now allows '@' as well as if $comm (or $COMM) is not a field, it
acts the same as $comm for kprobes. Filtering and histograms do the same
thing (use 'comm' as the event field, or has the current->comm if the event
does not have 'comm' as a field). I should probably make "$common_comm"
used too.
-- Steve
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string
2022-08-20 12:48 ` Steven Rostedt
@ 2022-08-20 13:00 ` Steven Rostedt
2022-08-20 13:09 ` Masami Hiramatsu
0 siblings, 1 reply; 16+ messages in thread
From: Steven Rostedt @ 2022-08-20 13:00 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Tzvetomir Stoyanov,
Tom Zanussi, stable
On Sat, 20 Aug 2022 08:48:37 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> I should probably make "$common_comm" used too.
My mistake. It was "common_cpu" not "common_comm". The filter and histogram
just use "comm" or "COMM". I'll leave this as it.
-- Steve
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] tracing/eprobes: Have event probes be consistent with kprobes and uprobes
2022-08-20 1:40 ` [PATCH 4/4] tracing/eprobes: Have event probes be consistent with kprobes and uprobes Steven Rostedt
@ 2022-08-20 13:04 ` Masami Hiramatsu
2022-08-20 13:11 ` Steven Rostedt
0 siblings, 1 reply; 16+ messages in thread
From: Masami Hiramatsu @ 2022-08-20 13:04 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Masami Hiramatsu,
Tzvetomir Stoyanov, Tom Zanussi, stable
On Fri, 19 Aug 2022 21:40:39 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>
> 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@kernel.org>
Thank you!
>
> Cc: stable@vger.kernel.org
> Fixes: 7491e2c44278 ("tracing: Add a probe that attaches to trace events")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> 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@kernel.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string
2022-08-20 13:00 ` Steven Rostedt
@ 2022-08-20 13:09 ` Masami Hiramatsu
2022-08-20 13:19 ` Steven Rostedt
0 siblings, 1 reply; 16+ messages in thread
From: Masami Hiramatsu @ 2022-08-20 13:09 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Tzvetomir Stoyanov,
Tom Zanussi, stable
On Sat, 20 Aug 2022 09:00:38 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Sat, 20 Aug 2022 08:48:37 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > I should probably make "$common_comm" used too.
>
>
> My mistake. It was "common_cpu" not "common_comm". The filter and histogram
> just use "comm" or "COMM". I'll leave this as it.
Yeah, this is a bit confusing me. histogram allows common_cpu but filter allows
only CPU. Shouldn't we unify those?
Thanks,
>
> -- Steve
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] tracing/eprobes: Have event probes be consistent with kprobes and uprobes
2022-08-20 13:04 ` Masami Hiramatsu
@ 2022-08-20 13:11 ` Steven Rostedt
0 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2022-08-20 13:11 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Tzvetomir Stoyanov,
Tom Zanussi, stable
On Sat, 20 Aug 2022 22:04:42 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > 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@kernel.org>
I could add this on top of the patch series.
-- Steve
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string
2022-08-20 13:09 ` Masami Hiramatsu
@ 2022-08-20 13:19 ` Steven Rostedt
0 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2022-08-20 13:19 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Tzvetomir Stoyanov,
Tom Zanussi, stable
On Sat, 20 Aug 2022 22:09:20 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > My mistake. It was "common_cpu" not "common_comm". The filter and histogram
> > just use "comm" or "COMM". I'll leave this as it.
>
> Yeah, this is a bit confusing me. histogram allows common_cpu but filter allows
> only CPU. Shouldn't we unify those?
I can do that too.
-- Steve
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2022-08-20 13:19 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-20 1:40 [PATCH 0/4] tracing/eprobes: Fixes for unexpected arguments Steven Rostedt
2022-08-20 1:40 ` [PATCH 1/4] tracing/eprobes: Do not allow eprobes to use $stack, or % for regs Steven Rostedt
2022-08-20 8:33 ` Masami Hiramatsu
2022-08-20 1:40 ` [PATCH 2/4] tracing/eprobes: Do not hardcode $comm as a string Steven Rostedt
2022-08-20 1:57 ` Steven Rostedt
2022-08-20 4:28 ` kernel test robot
2022-08-20 11:18 ` Masami Hiramatsu
2022-08-20 12:48 ` Steven Rostedt
2022-08-20 13:00 ` Steven Rostedt
2022-08-20 13:09 ` Masami Hiramatsu
2022-08-20 13:19 ` Steven Rostedt
2022-08-20 1:40 ` [PATCH 3/4] tracing/eprobes: Fix reading of string fields Steven Rostedt
2022-08-20 12:27 ` Masami Hiramatsu
2022-08-20 1:40 ` [PATCH 4/4] tracing/eprobes: Have event probes be consistent with kprobes and uprobes Steven Rostedt
2022-08-20 13:04 ` Masami Hiramatsu
2022-08-20 13:11 ` Steven Rostedt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.