On Tue, 28 Nov 2023 13:24:35 -0600
David Vernet <void@xxxxxxxxxxxxx> wrote:
> execute_program(), in the trace-cmd record subcommand, searches for a
> command in PATH to create an absolute path to pass to execve. The
> implementation uses strtok_r, which mutates the underlying string in
> place by replacing ':' tokens with NULL bytes. This can and does cause
> the PATH that's passed to execve to only contain the first entry to
> PATH, which can cause issues such as the following:
>
> [root@maniforge linus]# trace-cmd record -e sched -v -e sched_stat_runtime make clean
> /bin/sh: line 1: uname: command not found
> /bin/sh: line 1: sed: command not found
> /bin/sh: line 1: head: command not found
> /bin/sh: line 1: grep: command not found
> /bin/sh: line 1: mkdir: command not found
> ...
> /bin/sh: line 1: mkdir: command not found
> /bin/sh: line 1: mkdir: command not found
> Makefile:681: arch//Makefile: No such file or directory
> make: *** No rule to make target 'arch//Makefile'. Stop.
>
> We should be resetting the PATH variable to the string stored in the
> saveptr argument to strtok_r.
Bah, I had this fixed locally, but never made pushed it up.
>
> Fixes: edf9424029cc ("trace-cmd: Open code execvp routine to avoid multiple execve syscalls")
> Signed-off-by: David Vernet <void@xxxxxxxxxxxxx>
> ---
> tracecmd/trace-record.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index bced80406816..63af11ecaa80 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -1708,6 +1708,14 @@ static void execute_program(int argc, char **argv)
> break;
>
> }
> +
> + /*
> + * reset PATH to saveptr, as strtok_r overwrites the string
> + * returned by getenv() which backs the PATH environment
> + * variable.
> + */
> + if (setenv("PATH", saveptr, 1))
> + die("Failed to reset PATH to %s (%s)", saveptr, strerror(errno));
> } else {
> strncpy(buf, argv[0], sizeof(buf));
> }
The fix I had was this:
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index bced8040..1a461631 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -1698,6 +1698,9 @@ static void execute_program(int argc, char **argv)
if (!path)
die("can't search for '%s' if $PATH is NULL", argv[0]);
+ /* Do not modify the actual environment variable */
+ path = strdup(path);
+
for (entry = strtok_r(path, ":", &saveptr);
entry; entry = strtok_r(NULL, ":", &saveptr)) {
@@ -1712,6 +1715,7 @@ static void execute_program(int argc, char **argv)
strncpy(buf, argv[0], sizeof(buf));
}
+ free(path);
tracecmd_enable_tracing();
if (execve(buf, argv, environ)) {
fprintf(stderr, "\n********************\n");
Does that work for you?
Although, I still need to test the result of strdup().
-- Steve
|