diff --git a/src/basic/escape.c b/src/basic/escape.c index 2e605b2ebe..6734fba1f3 100644 --- a/src/basic/escape.c +++ b/src/basic/escape.c @@ -18,6 +18,8 @@ int cescape_char(char c, char *buf) { char *buf_old = buf; + /* Needs space for 4 characters in the buffer */ + switch (c) { case '\a': diff --git a/src/basic/missing.h b/src/basic/missing.h index e714078f31..dbdbd6f260 100644 --- a/src/basic/missing.h +++ b/src/basic/missing.h @@ -1407,4 +1407,11 @@ struct statx { #define AT_STATX_DONT_SYNC 0x4000 #endif +/* The maximum thread/process name length including trailing NUL byte. This mimics the kernel definition of the same + * name, which we need in userspace at various places but is not defined in userspace currently, neither under this + * name nor any other. */ +#ifndef TASK_COMM_LEN +#define TASK_COMM_LEN 16 +#endif + #include "missing_syscall.h" diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 65abaaf888..e3be7b9793 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -76,20 +76,31 @@ int get_process_state(pid_t pid) { return (unsigned char) state; } -int get_process_comm(pid_t pid, char **name) { +int get_process_comm(pid_t pid, char **ret) { + _cleanup_free_ char *escaped = NULL, *comm = NULL; const char *p; int r; - assert(name); + assert(ret); assert(pid >= 0); + escaped = new(char, TASK_COMM_LEN); + if (!escaped) + return -ENOMEM; + p = procfs_file_alloca(pid, "comm"); - r = read_one_line_file(p, name); + r = read_one_line_file(p, &comm); if (r == -ENOENT) return -ESRCH; + if (r < 0) + return r; - return r; + /* Escape unprintable characters, just in case, but don't grow the string beyond the underlying size */ + cellescape(escaped, TASK_COMM_LEN, comm); + + *ret = TAKE_PTR(escaped); + return 0; } int get_process_cmdline(pid_t pid, size_t max_length, bool comm_fallback, char **line) { @@ -296,7 +307,7 @@ int rename_process(const char name[]) { * can use PR_SET_NAME, which sets the thread name for the calling thread. */ if (prctl(PR_SET_NAME, name) < 0) log_debug_errno(errno, "PR_SET_NAME failed: %m"); - if (l > 15) /* Linux process names can be 15 chars at max */ + if (l >= TASK_COMM_LEN) /* Linux process names can be 15 chars at max */ truncated = true; /* Second step, change glibc's ID of the process name. */ @@ -740,14 +751,17 @@ int wait_for_terminate_and_check(const char *name, pid_t pid, WaitFlags flags) { /* * Return values: - * < 0 : wait_for_terminate_with_timeout() failed to get the state of the - * process, the process timed out, the process was terminated by a - * signal, or failed for an unknown reason. + * + * < 0 : wait_for_terminate_with_timeout() failed to get the state of the process, the process timed out, the process + * was terminated by a signal, or failed for an unknown reason. + * * >=0 : The process terminated normally with no failures. * - * Success is indicated by a return value of zero, a timeout is indicated - * by ETIMEDOUT, and all other child failure states are indicated by error - * is indicated by a non-zero value. + * Success is indicated by a return value of zero, a timeout is indicated by ETIMEDOUT, and all other child failure + * states are indicated by error is indicated by a non-zero value. + * + * This call assumes SIGCHLD has been blocked already, in particular before the child to wait for has been forked off + * to remain entirely race-free. */ int wait_for_terminate_with_timeout(pid_t pid, usec_t timeout) { sigset_t mask; diff --git a/src/basic/string-util.c b/src/basic/string-util.c index b06b50397e..4c7ab3c4d3 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -627,25 +627,58 @@ char *cellescape(char *buf, size_t len, const char *s) { * very end. */ - size_t i; - const char *t = s; + size_t i = 0, last_char_width[4] = {}, k = 0, j; - assert(len > 4 + 4 + 1); /* two chars and the terminator */ + assert(len > 0); /* at least a terminating NUL */ - for (i = 0; i < len - 9; t++) { - if (!*t) + for (;;) { + char four[4]; + int w; + + if (*s == 0) /* terminating NUL detected? then we are done! */ goto done; - i += cescape_char(*t, buf + i); + + w = cescape_char(*s, four); + if (i + w + 1 > len) /* This character doesn't fit into the buffer anymore? In that case let's + * ellipsize at the previous location */ + break; + + /* OK, there was space, let's add this escaped character to the buffer */ + memcpy(buf + i, four, w); + i += w; + + /* And remember its width in the ring buffer */ + last_char_width[k] = w; + k = (k + 1) % 4; + + s++; } - /* We have space for one more char and terminating nul at this point */ - if (*t) { - if (*(t+1)) - i += write_ellipsis(buf + i, false); - else - i += cescape_char(*t, buf + i); + /* Ellipsation is necessary. This means we might need to truncate the string again to make space for 4 + * characters ideally, but the buffer is shorter than that in the first place take what we can get */ + for (j = 0; j < ELEMENTSOF(last_char_width); j++) { + + if (i + 4 <= len) /* nice, we reached our space goal */ + break; + + k = k == 0 ? 3 : k - 1; + if (last_char_width[k] == 0) /* bummer, we reached the beginning of the strings */ + break; + + assert(i >= last_char_width[k]); + i -= last_char_width[k]; } + if (i + 4 <= len) /* yay, enough space */ + i += write_ellipsis(buf + i, false); + else if (i + 3 <= len) { /* only space for ".." */ + buf[i++] = '.'; + buf[i++] = '.'; + } else if (i + 2 <= len) /* only space for a single "." */ + buf[i++] = '.'; + else + assert(i + 1 <= len); + done: buf[i] = '\0'; return buf; diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c index fd5b44dc13..ef18cff70d 100644 --- a/src/test/test-process-util.c +++ b/src/test/test-process-util.c @@ -91,6 +91,38 @@ static void test_get_process_comm(pid_t pid) { log_info("PID"PID_FMT" $PATH: '%s'", pid, strna(i)); } +static void test_get_process_comm_escape_one(const char *input, const char *output) { + _cleanup_free_ char *n = NULL; + + log_info("input: <%s> — output: <%s>", input, output); + + assert_se(prctl(PR_SET_NAME, input) >= 0); + assert_se(get_process_comm(0, &n) >= 0); + + log_info("got: <%s>", n); + + assert_se(streq_ptr(n, output)); +} + +static void test_get_process_comm_escape(void) { + _cleanup_free_ char *saved = NULL; + + assert_se(get_process_comm(0, &saved) >= 0); + + test_get_process_comm_escape_one("", ""); + test_get_process_comm_escape_one("foo", "foo"); + test_get_process_comm_escape_one("012345678901234", "012345678901234"); + test_get_process_comm_escape_one("0123456789012345", "012345678901234"); + test_get_process_comm_escape_one("äöüß", "\\303\\244\\303…"); + test_get_process_comm_escape_one("xäöüß", "x\\303\\244…"); + test_get_process_comm_escape_one("xxäöüß", "xx\\303\\244…"); + test_get_process_comm_escape_one("xxxäöüß", "xxx\\303\\244…"); + test_get_process_comm_escape_one("xxxxäöüß", "xxxx\\303\\244…"); + test_get_process_comm_escape_one("xxxxxäöüß", "xxxxx\\303…"); + + assert_se(prctl(PR_SET_NAME, saved) >= 0); +} + static void test_pid_is_unwaited(void) { pid_t pid; @@ -366,7 +398,7 @@ static void test_rename_process_now(const char *p, int ret) { assert_se(get_process_comm(0, &comm) >= 0); log_info("comm = <%s>", comm); - assert_se(strneq(comm, p, 15)); + assert_se(strneq(comm, p, TASK_COMM_LEN-1)); assert_se(get_process_cmdline(0, 0, false, &cmdline) >= 0); /* we cannot expect cmdline to be renamed properly without privileges */ @@ -572,6 +604,7 @@ int main(int argc, char *argv[]) { test_get_process_comm(getpid()); } + test_get_process_comm_escape(); test_pid_is_unwaited(); test_pid_is_alive(); test_personality(); diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 413adfda7d..8b176781de 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -81,21 +81,79 @@ static void test_ascii_strcasecmp_nn(void) { static void test_cellescape(void) { char buf[40]; - assert_se(streq(cellescape(buf, 10, "1"), "1")); - assert_se(streq(cellescape(buf, 10, "12"), "12")); - assert_se(streq(cellescape(buf, 10, "123"), is_locale_utf8() ? "1…" : "1...")); + assert_se(streq(cellescape(buf, 1, ""), "")); + assert_se(streq(cellescape(buf, 1, "1"), "")); + assert_se(streq(cellescape(buf, 1, "12"), "")); - assert_se(streq(cellescape(buf, 10, "1\011"), "1\\t")); - assert_se(streq(cellescape(buf, 10, "1\020"), "1\\020")); - assert_se(streq(cellescape(buf, 10, "1\020x"), is_locale_utf8() ? "1…" : "1...")); + assert_se(streq(cellescape(buf, 2, ""), "")); + assert_se(streq(cellescape(buf, 2, "1"), "1")); + assert_se(streq(cellescape(buf, 2, "12"), ".")); + assert_se(streq(cellescape(buf, 2, "123"), ".")); + + assert_se(streq(cellescape(buf, 3, ""), "")); + assert_se(streq(cellescape(buf, 3, "1"), "1")); + assert_se(streq(cellescape(buf, 3, "12"), "12")); + assert_se(streq(cellescape(buf, 3, "123"), "..")); + assert_se(streq(cellescape(buf, 3, "1234"), "..")); + + assert_se(streq(cellescape(buf, 4, ""), "")); + assert_se(streq(cellescape(buf, 4, "1"), "1")); + assert_se(streq(cellescape(buf, 4, "12"), "12")); + assert_se(streq(cellescape(buf, 4, "123"), "123")); + assert_se(streq(cellescape(buf, 4, "1234"), is_locale_utf8() ? "…" : "...")); + assert_se(streq(cellescape(buf, 4, "12345"), is_locale_utf8() ? "…" : "...")); + + assert_se(streq(cellescape(buf, 5, ""), "")); + assert_se(streq(cellescape(buf, 5, "1"), "1")); + assert_se(streq(cellescape(buf, 5, "12"), "12")); + assert_se(streq(cellescape(buf, 5, "123"), "123")); + assert_se(streq(cellescape(buf, 5, "1234"), "1234")); + assert_se(streq(cellescape(buf, 5, "12345"), is_locale_utf8() ? "1…" : "1...")); + assert_se(streq(cellescape(buf, 5, "123456"), is_locale_utf8() ? "1…" : "1...")); + + assert_se(streq(cellescape(buf, 1, "\020"), "")); + assert_se(streq(cellescape(buf, 2, "\020"), ".")); + assert_se(streq(cellescape(buf, 3, "\020"), "..")); + assert_se(streq(cellescape(buf, 4, "\020"), "…")); + assert_se(streq(cellescape(buf, 5, "\020"), "\\020")); + + assert_se(streq(cellescape(buf, 5, "1234\020"), "1…")); + assert_se(streq(cellescape(buf, 6, "1234\020"), "12…")); + assert_se(streq(cellescape(buf, 7, "1234\020"), "123…")); + assert_se(streq(cellescape(buf, 8, "1234\020"), "1234…")); + assert_se(streq(cellescape(buf, 9, "1234\020"), "1234\\020")); + + assert_se(streq(cellescape(buf, 1, "\t\n"), "")); + assert_se(streq(cellescape(buf, 2, "\t\n"), ".")); + assert_se(streq(cellescape(buf, 3, "\t\n"), "..")); + assert_se(streq(cellescape(buf, 4, "\t\n"), "…")); + assert_se(streq(cellescape(buf, 5, "\t\n"), "\\t\\n")); + + assert_se(streq(cellescape(buf, 5, "1234\t\n"), "1…")); + assert_se(streq(cellescape(buf, 6, "1234\t\n"), "12…")); + assert_se(streq(cellescape(buf, 7, "1234\t\n"), "123…")); + assert_se(streq(cellescape(buf, 8, "1234\t\n"), "1234…")); + assert_se(streq(cellescape(buf, 9, "1234\t\n"), "1234\\t\\n")); + + assert_se(streq(cellescape(buf, 4, "x\t\020\n"), "…")); + assert_se(streq(cellescape(buf, 5, "x\t\020\n"), "x…")); + assert_se(streq(cellescape(buf, 6, "x\t\020\n"), "x…")); + assert_se(streq(cellescape(buf, 7, "x\t\020\n"), "x\\t…")); + assert_se(streq(cellescape(buf, 8, "x\t\020\n"), "x\\t…")); + assert_se(streq(cellescape(buf, 9, "x\t\020\n"), "x\\t…")); + assert_se(streq(cellescape(buf, 10, "x\t\020\n"), "x\\t\\020\\n")); + + assert_se(streq(cellescape(buf, 6, "1\011"), "1\\t")); + assert_se(streq(cellescape(buf, 6, "1\020"), "1\\020")); + assert_se(streq(cellescape(buf, 6, "1\020x"), is_locale_utf8() ? "1…" : "1...")); assert_se(streq(cellescape(buf, 40, "1\020"), "1\\020")); assert_se(streq(cellescape(buf, 40, "1\020x"), "1\\020x")); assert_se(streq(cellescape(buf, 40, "\a\b\f\n\r\t\v\\\"'"), "\\a\\b\\f\\n\\r\\t\\v\\\\\\\"\\'")); - assert_se(streq(cellescape(buf, 10, "\a\b\f\n\r\t\v\\\"'"), is_locale_utf8() ? "\\a…" : "\\a...")); - assert_se(streq(cellescape(buf, 11, "\a\b\f\n\r\t\v\\\"'"), is_locale_utf8() ? "\\a…" : "\\a...")); - assert_se(streq(cellescape(buf, 12, "\a\b\f\n\r\t\v\\\"'"), is_locale_utf8() ? "\\a\\b…" : "\\a\\b...")); + assert_se(streq(cellescape(buf, 6, "\a\b\f\n\r\t\v\\\"'"), is_locale_utf8() ? "\\a…" : "\\a...")); + assert_se(streq(cellescape(buf, 7, "\a\b\f\n\r\t\v\\\"'"), is_locale_utf8() ? "\\a…" : "\\a...")); + assert_se(streq(cellescape(buf, 8, "\a\b\f\n\r\t\v\\\"'"), is_locale_utf8() ? "\\a\\b…" : "\\a\\b...")); assert_se(streq(cellescape(buf, sizeof buf, "1\020"), "1\\020")); assert_se(streq(cellescape(buf, sizeof buf, "1\020x"), "1\\020x"));