Re: [PATCH] libtracefs: Have tracefs_dynevent_get_all() find kprobes and uprobes properly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 15/10/2024 8:11 pm, Steven Rostedt wrote:
From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>

The function tracefs_dynevent_get_all() will only look at the
dynamic_events file if it exists to find matching probes. But because
uprobes and kprobes both use the same prefix "p" as well as uretprobes and
kretprobes (with prefix "r"), it cannot use the dynamic_events file to
differentiate between the two.

Have kprobes and uprobes always use their own files (kprobe_events and
uprobe_events) even if dynamic_events file exists.

Also cleaned up the code to be a bit more efficient.

Link: https://lore.kernel.org/all/20241015123831.347ff0f4@xxxxxxxxxxxxxxxxxx/

Fixes: b04f18b005c6b ("libtracefs: New APIs for dynamic events")
Reported-by: Metin Kaya <metin.kaya@xxxxxxx>
Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
---
  src/tracefs-dynevents.c | 113 +++++++++++++++++++++-------------------
  1 file changed, 60 insertions(+), 53 deletions(-)

diff --git a/src/tracefs-dynevents.c b/src/tracefs-dynevents.c
index 85c1fcd9d5d5..6b9ba539fde3 100644
--- a/src/tracefs-dynevents.c
+++ b/src/tracefs-dynevents.c
@@ -34,23 +34,21 @@ static int dyn_generic_del(struct dyn_events_desc *, struct tracefs_dynevent *);
  static int dyn_synth_del(struct dyn_events_desc *, struct tracefs_dynevent *);
struct dyn_events_desc {
-	enum tracefs_dynevent_type type;
-	const char *file;
-	const char *prefix;
+	enum tracefs_dynevent_type	type;
+	const char			*file;
+	const char			*prefix;
  	int (*del)(struct dyn_events_desc *desc, struct tracefs_dynevent *dyn);
  	int (*parse)(struct dyn_events_desc *desc, const char *group,
  				char *line, struct tracefs_dynevent **ret_dyn);
  } dynevents[] = {
-	{TRACEFS_DYNEVENT_KPROBE, KPROBE_EVENTS, "p", dyn_generic_del, dyn_generic_parse},
+	{TRACEFS_DYNEVENT_KPROBE,    KPROBE_EVENTS, "p", dyn_generic_del, dyn_generic_parse},
  	{TRACEFS_DYNEVENT_KRETPROBE, KPROBE_EVENTS, "r", dyn_generic_del, dyn_generic_parse},
-	{TRACEFS_DYNEVENT_UPROBE, UPROBE_EVENTS, "p", dyn_generic_del, dyn_generic_parse},
+	{TRACEFS_DYNEVENT_UPROBE,    UPROBE_EVENTS, "p", dyn_generic_del, dyn_generic_parse},
  	{TRACEFS_DYNEVENT_URETPROBE, UPROBE_EVENTS, "r", dyn_generic_del, dyn_generic_parse},
-	{TRACEFS_DYNEVENT_EPROBE, "", "e", dyn_generic_del, dyn_generic_parse},
-	{TRACEFS_DYNEVENT_SYNTH, SYNTH_EVENTS, "", dyn_synth_del, dyn_synth_parse},
+	{TRACEFS_DYNEVENT_EPROBE,    "",            "e", dyn_generic_del, dyn_generic_parse},
+	{TRACEFS_DYNEVENT_SYNTH,     SYNTH_EVENTS, "", dyn_synth_del, dyn_synth_parse},
  };
-
-
  static int dyn_generic_del(struct dyn_events_desc *desc, struct tracefs_dynevent *dyn)
  {
  	char *str;
@@ -280,8 +278,13 @@ static void init_devent_desc(void)
  		return;
/* Use ftrace dynamic_events, if available */
-	for (i = 0; i < EVENT_INDEX(TRACEFS_DYNEVENT_MAX); i++)
+	for (i = 0; i < EVENT_INDEX(TRACEFS_DYNEVENT_MAX); i++) {
+		/* kprobes and uprobes do not use default file */
+		if (dynevents[i].prefix[0] == 'p' ||
+		    dynevents[i].prefix[0] == 'r')
+			continue;
  		dynevents[i].file = DYNEVENTS_EVENTS;
+	}
dynevents[EVENT_INDEX(TRACEFS_DYNEVENT_SYNTH)].prefix = "s";
  }
@@ -480,16 +483,15 @@ int tracefs_dynevent_destroy(struct tracefs_dynevent *devent, bool force)
  	return desc->del(desc, devent);
  }
-static int get_all_dynevents(enum tracefs_dynevent_type type, const char *system,
-			     struct tracefs_dynevent ***ret_all)
+static int get_dynevent(enum tracefs_dynevent_type type, const char *system,
+			struct tracefs_dynevent ***ret_all, int count)
  {
  	struct dyn_events_desc *desc;
-	struct tracefs_dynevent *devent, **tmp, **all = NULL;
+	struct tracefs_dynevent *devent, **tmp, **all;
  	char *content;
-	int count = 0;
  	char *line;
  	char *next;
-	int ret;
+	int ret = -1;
desc = get_devent_desc(type);
  	if (!desc)
@@ -499,6 +501,9 @@ static int get_all_dynevents(enum tracefs_dynevent_type type, const char *system
  	if (!content)
  		return -1;
+ if (ret_all)
+		all = *ret_all;
+
  	line = content;
  	do {
  		next = strchr(line, '\n');
@@ -507,11 +512,12 @@ static int get_all_dynevents(enum tracefs_dynevent_type type, const char *system
  		ret = desc->parse(desc, system, line, ret_all ? &devent : NULL);
  		if (!ret) {
  			if (ret_all) {
-				tmp = realloc(all, (count + 1) * sizeof(*tmp));
+				tmp = realloc(all, (count + 2) * sizeof(*tmp));
  				if (!tmp)
-					goto error;
+					break;
  				all = tmp;
  				all[count] = devent;
+				all[count + 1] = NULL;
  			}
  			count++;
  		}
@@ -521,12 +527,38 @@ static int get_all_dynevents(enum tracefs_dynevent_type type, const char *system
  	free(content);
  	if (ret_all)
  		*ret_all = all;
+
  	return count;
+}
-error:
-	free(content);
-	free(all);
-	return -1;
+static int get_all_dynevents(enum tracefs_dynevent_type type, const char *system,
+			     struct tracefs_dynevent ***ret_all)
+{
+	int count = 0;
+	int i;
+
+	if (ret_all)
+		*ret_all = NULL;
+
+	for (i = 0; i < EVENT_INDEX(TRACEFS_DYNEVENT_MAX); i++) {
+		if (!((1 << i) & type))
+			continue;
+
+		count = get_dynevent((1 << i), system, ret_all, count);
+		if (count < 0) {
+			count = 0;
+			break;
+		}
+	}
+
+	if (!count) {
+		if (ret_all) {
+			free(*ret_all);
+			*ret_all = NULL;
+		}
+		count = -1;
+	}
+	return count;
  }
/**
@@ -561,41 +593,16 @@ void tracefs_dynevent_list_free(struct tracefs_dynevent **events)
  struct tracefs_dynevent **
  tracefs_dynevent_get_all(unsigned int types, const char *system)
  {
-	struct tracefs_dynevent **events, **tmp, **all_events = NULL;
-	int count, all = 0;
-	int i;
+	struct tracefs_dynevent **events;
+	int count;
- for (i = 1; i < TRACEFS_DYNEVENT_MAX; i <<= 1) {
-		if (types) {
-			if (i > types)
-				break;
-			if (!(types & i))
-				continue;
-		}
-		count = get_all_dynevents(i, system, &events);
-		if (count > 0) {
-			tmp = realloc(all_events, (all + count + 1) * sizeof(*tmp));
-			if (!tmp)
-				goto error;
-			all_events = tmp;
-			memcpy(all_events + all, events, count * sizeof(*events));
-			all += count;
-			/* Add a NULL pointer at the end */
-			all_events[all] = NULL;
-			free(events);
-		}
+	count = get_all_dynevents(types, system, &events);
+	if (count <= 0) {
+		free(events);
+		return NULL;
  	}
- return all_events;
-
-error:
-	free(events);
-	if (all_events) {
-		for (i = 0; i < all; i++)
-			free(all_events[i]);
-		free(all_events);
-	}
-	return NULL;
+	return events;
  }
/**


I could successfully apply the patch on 4cbebed79b1f ("libtracefs: Documentation: Add missing documentation to meson.build").
The unit tests at [1] passes with this patch.

Just for the records, there are some unit test failures in libtracefs (they were there even before the patch):

$ sudo ./utest/trace-utest
     CUnit - A unit testing framework for C - Version 2.1-3
     http://cunit.sourceforge.net/

Memory mapped buffers not supported

Suite: tracefs library
  Test: Test tracefs/debugfs mounting ...FAILED
    1. tracefs-utest.c:1806  - ret == 0
  Test: trace cpu read ...passed
  Test: trace cpu read_buf_percent ...passed
  Test: trace cpu pipe ...passed
  Test: trace pid events filter ...passed
  Test: trace pid function filter ...passed
  Test: trace sql ...passed
  Test: trace sql trace onmax ...passed
  Test: trace sql trace onchange ...passed
  Test: trace sql snapshot onmax ...passed
  Test: trace sql snapshot onchange ...passed
  Test: trace sql save onmax ...passed
  Test: trace sql save onchange ...passed
  Test: trace sql trace and snapshot onmax ...passed
  Test: trace sql trace and snapshot onchange ...passed
  Test: tracing file / directory APIs ...passed
  Test: instance file / directory APIs ...passed
  Test: instance file descriptor ...passed
  Test: instance reset ...passed
  Test: systems and events APIs ...passed
  Test: tracefs_iterate_snapshot_events API ...passed
  Test: tracefs_iterate_raw_events API ...FAILED
    1. tracefs-utest.c:235  - ret == sizeof(struct test_sample)
    2. tracefs-utest.c:235  - ret == sizeof(struct test_sample)
  Test: Follow events ...passed
  Test: Follow events clear ...passed
  Test: tracefs_tracers API ...passed
  Test: tracefs_local events API ...passed
  Test: tracefs_instances_walk API ...passed
  Test: tracefs_get_clock API ...passed
  Test: tracing on / off ...passed
  Test: tracing options ...passed
  Test: custom system directory ...passed
  Test: ftrace marker ...passed
  Test: kprobes ...passed
  Test: synthetic events ...passed
  Test: eprobes ...passed
  Test: uprobes ...FAILED
    1. tracefs-utest.c:2222  - ret == 0
    2. tracefs-utest.c:2222  - ret == 0

Run Summary:    Type    Total      Ran   Passed Failed Inactive
              suites        1        1      n/a      0        0
               tests       36       36       33      3        0
             asserts 30341119 30341119 30341114      5      n/a

Elapsed time =  124.937 seconds

With that,

Reviewed-by: Metin Kaya <metin.kaya@xxxxxxx>


Thanks a lot for fixing it promptly!

[1] https://lore.kernel.org/all/20241015140840.4183007-1-metin.kaya@xxxxxxx/





[Index of Archives]     [Linux USB Development]     [Linux USB Development]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux