Skip to content

Commit

Permalink
unix: fix size check in uv_get_process_title()
Browse files Browse the repository at this point in the history
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: #2666
PR-URL: #2668
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
  • Loading branch information
bnoordhuis committed Feb 8, 2020
1 parent ea3a531 commit 7b28d36
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 18 deletions.
51 changes: 33 additions & 18 deletions src/unix/proctitle.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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*);
Expand All @@ -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;

Expand All @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions test/run-tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
2 changes: 2 additions & 0 deletions test/test-list.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
59 changes: 59 additions & 0 deletions test/test-process-title.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}

0 comments on commit 7b28d36

Please sign in to comment.