From 9ce3440a6fb3fb785e0fa3a6fcf60af7ef652440 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Nov 2023 14:13:33 +0100 Subject: [PATCH] run: fix bad escaping and memory ownership confusion arg_description was either set to arg_unit (i.e. a const char*), or to char *description, the result of allocation in run(). But description was decorated with _cleanup_, so it would be freed when going out of the function. Nothing bad would happen, because the program would exit after exiting from run(), but this is just all too messy. Also, strv_join(" ") + shell_escape() is not a good way to escape command lines. In particular, one the join has happened, we cannot distinguish empty arguments, or arguments with whitespace, etc. We have a helper function to do the escaping properly, so let's use that. Fixup for 2c29813da3421b77eca5e5cdc3b9a863cad473b9. --- src/run/run.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index 5c0f3ad247..88eca0fd6d 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -43,7 +43,7 @@ static bool arg_remain_after_exit = false; static bool arg_no_block = false; static bool arg_wait = false; static const char *arg_unit = NULL; -static const char *arg_description = NULL; +static char *arg_description = NULL; static const char *arg_slice = NULL; static bool arg_slice_inherit = false; static int arg_expand_environment = -1; @@ -74,6 +74,7 @@ static char *arg_working_directory = NULL; static bool arg_shell = false; static char **arg_cmdline = NULL; +STATIC_DESTRUCTOR_REGISTER(arg_description, freep); STATIC_DESTRUCTOR_REGISTER(arg_environment, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_property, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_path_property, strv_freep); @@ -281,7 +282,9 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_DESCRIPTION: - arg_description = optarg; + r = free_and_strdup(&arg_description, optarg); + if (r < 0) + return r; break; case ARG_SLICE: @@ -1911,7 +1914,6 @@ static bool shall_make_executable_absolute(void) { static int run(int argc, char* argv[]) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; - _cleanup_free_ char *description = NULL; int r; log_show_color(true); @@ -1936,19 +1938,16 @@ static int run(int argc, char* argv[]) { } if (!arg_description) { - if (strv_isempty(arg_cmdline)) - arg_description = arg_unit; - else { - _cleanup_free_ char *joined = strv_join(arg_cmdline, " "); - if (!joined) - return log_oom(); + char *t; - description = shell_escape(joined, "\""); - if (!description) - return log_oom(); + if (strv_isempty(arg_cmdline)) + t = strdup(arg_unit); + else + t = quote_command_line(arg_cmdline, SHELL_ESCAPE_EMPTY); + if (!t) + return log_oom(); - arg_description = description; - } + free_and_replace(arg_description, t); } /* For backward compatibility reasons env var expansion is disabled by default for scopes, and -- 2.25.1