diff mbox series

[v4] trace-cmd: Open code execvp routine to avoid multiple execve syscalls

Message ID Y8yVDI93geYhLQio@mail.google.com (mailing list archive)
State Accepted
Commit edf9424029cc4a05166100a5957fab6a13791880
Headers show
Series [v4] trace-cmd: Open code execvp routine to avoid multiple execve syscalls | expand

Commit Message

Paulo Miguel Almeida Jan. 22, 2023, 1:44 a.m. UTC
In tracecmd/trace-record.c:<run_cmd>, trace-cmd record -F <executable>
is launched via the libc's execvp() routine. The way that execvp() routine
works is by invoking execve syscall for every entry on the $PATH if
command specified is neither absolute nor relative which can come across
as a bit cryptic to untrained eyes.

- absolute path example:

        # trace-cmd record -p function_graph \
                -g __x64_sys_execve -O nofuncgraph-irqs \
                -n __cond_resched --max-graph-depth 1  \
                -F /usr/bin/echo "ftrace" > /dev/null

        # trace-cmd report
        echo-172994 [000] 185539.798539: funcgraph_entry:      ! 803.376 us |  __x64_sys_execve();

- PATH-dependent path example:

        # trace-cmd record -p function_graph \
                -g __x64_sys_execve -O nofuncgraph-irqs \
                -n __cond_resched --max-graph-depth 1  \
                -F echo "ftrace" > /dev/null

        # trace-cmd report
        echo-172656 [002] 185009.671586: funcgraph_entry:      ! 288.732 us |  __x64_sys_execve();
        echo-172656 [002] 185009.671879: funcgraph_entry:      ! 158.337 us |  __x64_sys_execve();
        echo-172656 [002] 185009.672042: funcgraph_entry:      ! 161.843 us |  __x64_sys_execve();
        echo-172656 [002] 185009.672207: funcgraph_entry:      ! 157.656 us |  __x64_sys_execve();
        echo-172656 [002] 185009.672369: funcgraph_entry:      ! 156.343 us |  __x64_sys_execve();
        echo-172656 [002] 185009.672529: funcgraph_entry:      ! 863.629 us |  __x64_sys_execve();

Open code the libc's execvp routine into trace-cmd so ftrace will only
start recording once the command is found when it needs to be found in
PATH.

Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
---
Changelog:

- v4: handle case in which path is relative but not './' is used. 
      (Req. Steven Rostedt)
- v3: simplify path traversal code via strtok_r. (Req. Steven Rostedt)
- v2: open code execvp routine into trace-cmd. (Req. Steven Rostedt)
- v1: https://lore.kernel.org/linux-trace-devel/Y7dUo6woh9Y31cdl@mail.google.com/

Tests made (that's should help during the review):

# Path-dependent
./tracecmd/trace-cmd <args> -F ls

# relative path with '.'
./tracecmd/trace-cmd <args> -F ../../../../usr/bin/ls

# absolute path
./tracecmd/trace-cmd <args> -F /usr/bin/ls

# relative path with no dots
mkdir test_bin 	&& cp /usr/bin/ls test_bin/ \
	&& ./tracecmd/trace-cmd <args> -F test_bin/ls \
	&& rm -rf test_bin
---
 tracecmd/trace-record.c | 48 +++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 7f0cebe..32fbfef 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -1683,6 +1683,46 @@  static int change_user(const char *user)
 	return 0;
 }
 
+static void execute_program(int argc, char **argv)
+{
+	char buf[PATH_MAX + NAME_MAX + 1];
+	char *path;
+	char *entry;
+	char *saveptr;
+
+	/*
+	 * if command specified by user is neither absolute nor
+	 * relative than we search for it in $PATH.
+	 */
+	if (!strchr(argv[0], '/')) {
+		path = getenv("PATH");
+
+		if (!path)
+			die("can't search for '%s' if $PATH is NULL", argv[0]);
+
+		for (entry = strtok_r(path, ":", &saveptr);
+		     entry; entry = strtok_r(NULL, ":", &saveptr)) {
+
+			snprintf(buf, sizeof(buf), "%s/%s", entry, argv[0]);
+
+			/* does it exist and can we execute it? */
+			if (access(buf, X_OK) == 0)
+				break;
+
+		}
+	} else {
+		strncpy(buf, argv[0], sizeof(buf));
+	}
+
+	tracecmd_enable_tracing();
+	if (execve(buf, argv, environ)) {
+		fprintf(stderr, "\n********************\n");
+		fprintf(stderr, " Unable to exec %s\n", argv[0]);
+		fprintf(stderr, "********************\n");
+		die("Failed to exec %s", argv[0]);
+	}
+}
+
 static void run_cmd(enum trace_type type, const char *user, int argc, char **argv)
 {
 	int status;
@@ -1693,7 +1733,6 @@  static void run_cmd(enum trace_type type, const char *user, int argc, char **arg
 	if (!pid) {
 		/* child */
 		update_task_filter();
-		tracecmd_enable_tracing();
 		if (!fork_process)
 			enable_ptrace();
 		/*
@@ -1709,12 +1748,7 @@  static void run_cmd(enum trace_type type, const char *user, int argc, char **arg
 		if (change_user(user) < 0)
 			die("Failed to change user to %s", user);
 
-		if (execvp(argv[0], argv)) {
-			fprintf(stderr, "\n********************\n");
-			fprintf(stderr, " Unable to exec %s\n", argv[0]);
-			fprintf(stderr, "********************\n");
-			die("Failed to exec %s", argv[0]);
-		}
+		execute_program(argc, argv);
 	}
 	if (fork_process)
 		exit(0);