From 7b28d36056d947e74e2c3347ee9bb04a1f5388c6 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 8 Feb 2020 12:28:34 +0100 Subject: [PATCH] unix: fix size check in uv_get_process_title() It was checking that the destination buffer was big enough to hold the total capacity of the process title (the total storage of argv) when instead it should be checking that it's big enough to hold the _current_ process title, which is normally much shorter. Fixes: https://github.com/libuv/libuv/issues/2666 PR-URL: https://github.com/libuv/libuv/pull/2668 Reviewed-By: Colin Ihrig Reviewed-By: Richard Lau Reviewed-By: Santiago Gimeno --- src/unix/proctitle.c | 51 +++++++++++++++++++++------------ test/run-tests.c | 7 +++++ test/test-list.h | 2 ++ test/test-process-title.c | 59 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 18 deletions(-) diff --git a/src/unix/proctitle.c b/src/unix/proctitle.c index d475bb23..d124d3c7 100644 --- a/src/unix/proctitle.c +++ b/src/unix/proctitle.c @@ -26,7 +26,8 @@ struct uv__process_title { char* str; - size_t len; + size_t len; /* Length of the current process title. */ + size_t cap; /* Maximum capacity. Computed once in uv_setup_args(). */ }; extern void uv__set_process_title(const char* title); @@ -52,20 +53,14 @@ char** uv_setup_args(int argc, char** argv) { if (argc <= 0) return argv; - /* Calculate how much memory we need for the argv strings. */ - size = 0; - for (i = 0; i < argc; i++) - size += strlen(argv[i]) + 1; - -#if defined(__MVS__) - /* argv is not adjacent. So just use argv[0] */ pt.str = argv[0]; pt.len = strlen(argv[0]); -#else - pt.str = argv[0]; - pt.len = argv[argc - 1] + strlen(argv[argc - 1]) - argv[0]; - assert(pt.len + 1 == size); /* argv memory should be adjacent. */ -#endif + pt.cap = pt.len + 1; + + /* Calculate how much memory we need for the argv strings. */ + size = pt.cap; + for (i = 1; i < argc; i++) + size += strlen(argv[i]) + 1; /* Add space for the argv pointers. */ size += (argc + 1) * sizeof(char*); @@ -75,15 +70,25 @@ char** uv_setup_args(int argc, char** argv) { return argv; /* Copy over the strings and set up the pointer table. */ + i = 0; s = (char*) &new_argv[argc + 1]; - for (i = 0; i < argc; i++) { + size = pt.cap; + goto loop; + + for (/* empty */; i < argc; i++) { size = strlen(argv[i]) + 1; + loop: memcpy(s, argv[i], size); new_argv[i] = s; s += size; } new_argv[i] = NULL; + /* argv is not adjacent on z/os, we use just argv[0] on that platform. */ +#ifndef __MVS__ + pt.cap = argv[i - 1] + size - argv[0]; +#endif + args_mem = new_argv; process_title = pt; @@ -92,15 +97,25 @@ char** uv_setup_args(int argc, char** argv) { int uv_set_process_title(const char* title) { + struct uv__process_title* pt; + size_t len; + + pt = &process_title; + len = strlen(title); + uv_once(&process_title_mutex_once, init_process_title_mutex_once); uv_mutex_lock(&process_title_mutex); - if (process_title.len != 0) { - /* No need to terminate, byte after is always '\0'. */ - strncpy(process_title.str, title, process_title.len); - uv__set_process_title(title); + if (len >= pt->cap) { + len = 0; + if (pt->cap > 0) + len = pt->cap - 1; } + memcpy(pt->str, title, len); + memset(pt->str + len, '\0', pt->cap - len); + pt->len = len; + uv_mutex_unlock(&process_title_mutex); return 0; diff --git a/test/run-tests.c b/test/run-tests.c index 2aad6a52..6c92c2b7 100644 --- a/test/run-tests.c +++ b/test/run-tests.c @@ -45,6 +45,7 @@ int ipc_helper_bind_twice(void); int ipc_helper_send_zero(void); int stdio_over_pipes_helper(void); void spawn_stdin_stdout(void); +void process_title_big_argv(void); int spawn_tcp_server_helper(void); static int maybe_run_test(int argc, char **argv); @@ -235,5 +236,11 @@ static int maybe_run_test(int argc, char **argv) { } #endif /* !_WIN32 */ + if (strcmp(argv[1], "process_title_big_argv_helper") == 0) { + notify_parent_process(); + process_title_big_argv(); + return 0; + } + return run_test(argv[1], 0, 1); } diff --git a/test/test-list.h b/test/test-list.h index a6cfc6bb..f18264f0 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -242,6 +242,7 @@ TEST_DECLARE (async_null_cb) TEST_DECLARE (eintr_handling) TEST_DECLARE (get_currentexe) TEST_DECLARE (process_title) +TEST_DECLARE (process_title_big_argv) TEST_DECLARE (process_title_threadsafe) TEST_DECLARE (cwd_and_chdir) TEST_DECLARE (get_memory) @@ -788,6 +789,7 @@ TASK_LIST_START TEST_ENTRY (get_currentexe) TEST_ENTRY (process_title) + TEST_ENTRY (process_title_big_argv) TEST_ENTRY (process_title_threadsafe) TEST_ENTRY (cwd_and_chdir) diff --git a/test/test-process-title.c b/test/test-process-title.c index b49f3bc4..35a14809 100644 --- a/test/test-process-title.c +++ b/test/test-process-title.c @@ -74,3 +74,62 @@ TEST_IMPL(process_title) { return 0; } + + +static void exit_cb(uv_process_t* process, int64_t status, int signo) { + ASSERT(status == 0); + ASSERT(signo == 0); + uv_close((uv_handle_t*) process, NULL); +} + + +TEST_IMPL(process_title_big_argv) { + uv_process_options_t options; + uv_process_t process; + size_t exepath_size; + char exepath[1024]; + char jumbo[512]; + char* args[5]; + +#ifdef _WIN32 + /* Remove once https://github.com/libuv/libuv/issues/2667 is fixed. */ + uv_set_process_title("run-tests"); +#endif + + exepath_size = sizeof(exepath) - 1; + ASSERT(0 == uv_exepath(exepath, &exepath_size)); + exepath[exepath_size] = '\0'; + + memset(jumbo, 'x', sizeof(jumbo) - 1); + jumbo[sizeof(jumbo) - 1] = '\0'; + + /* Note: need to pass three arguments, not two, otherwise + * run-tests.c thinks it's the name of a test to run. + */ + args[0] = exepath; + args[1] = "process_title_big_argv_helper"; + args[2] = jumbo; + args[3] = jumbo; + args[4] = NULL; + + memset(&options, 0, sizeof(options)); + options.file = exepath; + options.args = args; + options.exit_cb = exit_cb; + + ASSERT(0 == uv_spawn(uv_default_loop(), &process, &options)); + ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_DEFAULT)); + + MAKE_VALGRIND_HAPPY(); + return 0; +} + + +/* Called by process_title_big_argv_helper. */ +void process_title_big_argv(void) { + char buf[256] = "fail"; + + /* Return value deliberately ignored. */ + uv_get_process_title(buf, sizeof(buf)); + ASSERT(0 != strcmp(buf, "fail")); +}