* [PATCH 1/3] tracing/probe: Fix to allow user to enable events on unloaded modules
2019-09-18 8:55 [PATCH 0/3] tracing/probe: Fix some issues on multiprobe support Masami Hiramatsu
@ 2019-09-18 8:55 ` Masami Hiramatsu
2019-09-18 8:55 ` [PATCH 2/3] tracing/probe: Reject exactly same probe event Masami Hiramatsu
2019-09-18 8:55 ` [PATCH 3/3] selftests/ftrace: Update kprobe event error testcase Masami Hiramatsu
2 siblings, 0 replies; 4+ messages in thread
From: Masami Hiramatsu @ 2019-09-18 8:55 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-kernel, mhiramat, mingo
Fix to allow user to enable probe events on unloaded modules.
This operations was allowed before commit 60d53e2c3b75 ("tracing/probe:
Split trace_event related data from trace_probe"), because if users
need to probe module init functions, they have to enable those probe
events before loading module.
Fixes: 60d53e2c3b75 ("tracing/probe: Split trace_event related data from trace_probe")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace_kprobe.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 7579c53bb053..0ba3239c0270 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -371,31 +371,24 @@ static int enable_trace_kprobe(struct trace_event_call *call,
if (enabled)
return 0;
- enabled = false;
list_for_each_entry(pos, trace_probe_probe_list(tp), list) {
tk = container_of(pos, struct trace_kprobe, tp);
if (trace_kprobe_has_gone(tk))
continue;
ret = __enable_trace_kprobe(tk);
- if (ret) {
- if (enabled) {
- __disable_trace_kprobe(tp);
- enabled = false;
- }
+ if (ret)
break;
- }
enabled = true;
}
- if (!enabled) {
- /* No probe is enabled. Roll back */
+ if (ret) {
+ /* Failed to enable one of them. Roll back all */
+ if (enabled)
+ __disable_trace_kprobe(tp);
if (file)
trace_probe_remove_file(tp, file);
else
trace_probe_clear_flag(tp, TP_FLAG_PROFILE);
- if (!ret)
- /* Since all probes are gone, this is not available */
- ret = -EADDRNOTAVAIL;
}
return ret;
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/3] tracing/probe: Reject exactly same probe event
2019-09-18 8:55 [PATCH 0/3] tracing/probe: Fix some issues on multiprobe support Masami Hiramatsu
2019-09-18 8:55 ` [PATCH 1/3] tracing/probe: Fix to allow user to enable events on unloaded modules Masami Hiramatsu
@ 2019-09-18 8:55 ` Masami Hiramatsu
2019-09-18 8:55 ` [PATCH 3/3] selftests/ftrace: Update kprobe event error testcase Masami Hiramatsu
2 siblings, 0 replies; 4+ messages in thread
From: Masami Hiramatsu @ 2019-09-18 8:55 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-kernel, mhiramat, mingo
Reject exactly same probe events as existing probes.
Multiprobe allows user to define multiple probes on same
event. If user appends a probe which exactly same definition
(same probe address and same arguments) on existing event,
the event will record same probe information twice.
That can be confusing users, so reject it.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace_kprobe.c | 52 ++++++++++++++++++++++++++++++++++++-------
kernel/trace/trace_probe.h | 3 ++
kernel/trace/trace_uprobe.c | 52 ++++++++++++++++++++++++++++++++++++-------
3 files changed, 90 insertions(+), 17 deletions(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 0ba3239c0270..a6697e28ddda 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -528,10 +528,53 @@ static int unregister_trace_kprobe(struct trace_kprobe *tk)
return 0;
}
+static bool trace_kprobe_has_same_kprobe(struct trace_kprobe *orig,
+ struct trace_kprobe *comp)
+{
+ struct trace_probe_event *tpe = orig->tp.event;
+ struct trace_probe *pos;
+ int i;
+
+ list_for_each_entry(pos, &tpe->probes, list) {
+ orig = container_of(pos, struct trace_kprobe, tp);
+ if (strcmp(trace_kprobe_symbol(orig),
+ trace_kprobe_symbol(comp)) ||
+ trace_kprobe_offset(orig) != trace_kprobe_offset(comp))
+ continue;
+
+ /*
+ * trace_probe_compare_arg_type() ensured that nr_args and
+ * each argument name and type are same. Let's compare comm.
+ */
+ for (i = 0; i < orig->tp.nr_args; i++) {
+ if (strcmp(orig->tp.args[i].comm,
+ comp->tp.args[i].comm))
+ continue;
+ }
+
+ return true;
+ }
+
+ return false;
+}
+
static int append_trace_kprobe(struct trace_kprobe *tk, struct trace_kprobe *to)
{
int ret;
+ ret = trace_probe_compare_arg_type(&tk->tp, &to->tp);
+ if (ret) {
+ /* Note that argument starts index = 2 */
+ trace_probe_log_set_index(ret + 1);
+ trace_probe_log_err(0, DIFF_ARG_TYPE);
+ return -EEXIST;
+ }
+ if (trace_kprobe_has_same_kprobe(to, tk)) {
+ trace_probe_log_set_index(0);
+ trace_probe_log_err(0, SAME_PROBE);
+ return -EEXIST;
+ }
+
/* Append to existing event */
ret = trace_probe_append(&tk->tp, &to->tp);
if (ret)
@@ -568,14 +611,7 @@ static int register_trace_kprobe(struct trace_kprobe *tk)
trace_probe_log_err(0, DIFF_PROBE_TYPE);
ret = -EEXIST;
} else {
- ret = trace_probe_compare_arg_type(&tk->tp, &old_tk->tp);
- if (ret) {
- /* Note that argument starts index = 2 */
- trace_probe_log_set_index(ret + 1);
- trace_probe_log_err(0, DIFF_ARG_TYPE);
- ret = -EEXIST;
- } else
- ret = append_trace_kprobe(tk, old_tk);
+ ret = append_trace_kprobe(tk, old_tk);
}
goto end;
}
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index f805cc4cbe7c..4ee703728aec 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -436,7 +436,8 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
C(BAD_INSN_BNDRY, "Probe point is not an instruction boundary"),\
C(FAIL_REG_PROBE, "Failed to register probe event"),\
C(DIFF_PROBE_TYPE, "Probe type is different from existing probe"),\
- C(DIFF_ARG_TYPE, "Argument type or name is different from existing probe"),
+ C(DIFF_ARG_TYPE, "Argument type or name is different from existing probe"),\
+ C(SAME_PROBE, "There is already the exact same probe event"),
#undef C
#define C(a, b) TP_ERR_##a
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index cbf4da4bf367..34dd6d0016a3 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -410,10 +410,53 @@ static int unregister_trace_uprobe(struct trace_uprobe *tu)
return 0;
}
+static bool trace_uprobe_has_same_uprobe(struct trace_uprobe *orig,
+ struct trace_uprobe *comp)
+{
+ struct trace_probe_event *tpe = orig->tp.event;
+ struct trace_probe *pos;
+ struct inode *comp_inode = d_real_inode(comp->path.dentry);
+ int i;
+
+ list_for_each_entry(pos, &tpe->probes, list) {
+ orig = container_of(pos, struct trace_uprobe, tp);
+ if (comp_inode != d_real_inode(orig->path.dentry) ||
+ comp->offset != orig->offset)
+ continue;
+
+ /*
+ * trace_probe_compare_arg_type() ensured that nr_args and
+ * each argument name and type are same. Let's compare comm.
+ */
+ for (i = 0; i < orig->tp.nr_args; i++) {
+ if (strcmp(orig->tp.args[i].comm,
+ comp->tp.args[i].comm))
+ continue;
+ }
+
+ return true;
+ }
+
+ return false;
+}
+
static int append_trace_uprobe(struct trace_uprobe *tu, struct trace_uprobe *to)
{
int ret;
+ ret = trace_probe_compare_arg_type(&tu->tp, &to->tp);
+ if (ret) {
+ /* Note that argument starts index = 2 */
+ trace_probe_log_set_index(ret + 1);
+ trace_probe_log_err(0, DIFF_ARG_TYPE);
+ return -EEXIST;
+ }
+ if (trace_uprobe_has_same_uprobe(to, tu)) {
+ trace_probe_log_set_index(0);
+ trace_probe_log_err(0, SAME_PROBE);
+ return -EEXIST;
+ }
+
/* Append to existing event */
ret = trace_probe_append(&tu->tp, &to->tp);
if (!ret)
@@ -469,14 +512,7 @@ static int register_trace_uprobe(struct trace_uprobe *tu)
trace_probe_log_err(0, DIFF_PROBE_TYPE);
ret = -EEXIST;
} else {
- ret = trace_probe_compare_arg_type(&tu->tp, &old_tu->tp);
- if (ret) {
- /* Note that argument starts index = 2 */
- trace_probe_log_set_index(ret + 1);
- trace_probe_log_err(0, DIFF_ARG_TYPE);
- ret = -EEXIST;
- } else
- ret = append_trace_uprobe(tu, old_tu);
+ ret = append_trace_uprobe(tu, old_tu);
}
goto end;
}
^ permalink raw reply related [flat|nested] 4+ messages in thread