From: Luca Boccassi Date: Sun, 21 Nov 2021 17:05:28 +0000 (+0000) Subject: coredump: analyze object with libdwelf in forked process X-Git-Tag: v250-rc1~108^2~8 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=61aea456c12c54f49c4a76259af130e576130ce9;p=thirdparty%2Fsystemd.git coredump: analyze object with libdwelf in forked process Parsing objects is risky as data could be malformed or malicious, so avoid doing that from the main systemd-coredump process and instead fork another process, and set it to avoid generating core files itself. --- diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 9b035a5d8ad..c4b393631ab 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -825,7 +825,11 @@ static int submit_coredump( "than %"PRIu64" (the configured maximum)", coredump_size, arg_process_size_max); } else if (coredump_fd >= 0) - coredump_parse_core(coredump_fd, context->meta[META_EXE], &stacktrace, &json_metadata); + (void) parse_elf_object(coredump_fd, + context->meta[META_EXE], + /* fork_disable_dump= */endswith(context->meta[META_EXE], "systemd-coredump"), /* avoid loops */ + &stacktrace, + &json_metadata); #endif log: diff --git a/src/coredump/stacktrace.c b/src/coredump/stacktrace.c index 23161d5ac13..eec1af2dd34 100644 --- a/src/coredump/stacktrace.c +++ b/src/coredump/stacktrace.c @@ -4,15 +4,21 @@ #include #include #include +#include +#include #include #include #include "alloc-util.h" +#include "errno-util.h" #include "fileio.h" #include "fd-util.h" #include "format-util.h" #include "hexdecoct.h" +#include "io-util.h" #include "macro.h" +#include "process-util.h" +#include "rlimit-util.h" #include "stacktrace.h" #include "string-util.h" #include "util.h" @@ -365,30 +371,30 @@ static int parse_core(int fd, const char *executable, char **ret, JsonVariant ** c.elf = elf_begin(fd, ELF_C_READ_MMAP, NULL); if (!c.elf) - return -EINVAL; + return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "Could not parse core file, elf_begin() failed: %s", elf_errmsg(elf_errno())); c.dwfl = dwfl_begin(&callbacks); if (!c.dwfl) - return -EINVAL; + return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "Could not parse core file, dwfl_begin() failed: %s", dwfl_errmsg(dwfl_errno())); if (dwfl_core_file_report(c.dwfl, c.elf, executable) < 0) - return -EINVAL; + return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "Could not parse core file, dwfl_core_file_report() failed: %s", dwfl_errmsg(dwfl_errno())); if (dwfl_report_end(c.dwfl, NULL, NULL) != 0) - return -EINVAL; + return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "Could not parse core file, dwfl_report_end() failed: %s", dwfl_errmsg(dwfl_errno())); if (dwfl_getmodules(c.dwfl, &module_callback, &c, 0) < 0) - return -EINVAL; + return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "Could not parse core file, dwfl_getmodules() failed: %s", dwfl_errmsg(dwfl_errno())); if (dwfl_core_file_attach(c.dwfl, c.elf) < 0) - return -EINVAL; + return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "Could not parse core file, dwfl_core_file_attach() failed: %s", dwfl_errmsg(dwfl_errno())); if (dwfl_getthreads(c.dwfl, thread_callback, &c) < 0) - return -EINVAL; + return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "Could not parse core file, dwfl_getthreads() failed: %s", dwfl_errmsg(dwfl_errno())); r = fflush_and_check(c.f); if (r < 0) - return r; + return log_warning_errno(r, "Could not parse core file, flushing file buffer failed: %m"); c.f = safe_fclose(c.f); *ret = TAKE_PTR(buf); @@ -398,12 +404,122 @@ static int parse_core(int fd, const char *executable, char **ret, JsonVariant ** return 0; } -void coredump_parse_core(int fd, const char *executable, char **ret, JsonVariant **ret_package_metadata) { +int parse_elf_object(int fd, const char *executable, bool fork_disable_dump, char **ret, JsonVariant **ret_package_metadata) { + _cleanup_close_pair_ int error_pipe[2] = { -1, -1 }, return_pipe[2] = { -1, -1 }, json_pipe[2] = { -1, -1 }; + _cleanup_(json_variant_unrefp) JsonVariant *package_metadata = NULL; + _cleanup_free_ char *buf = NULL; int r; - r = parse_core(fd, executable, ret, ret_package_metadata); - if (r == -EINVAL) - log_warning("Failed to generate stack trace: %s", dwfl_errmsg(dwfl_errno())); - else if (r < 0) - log_warning_errno(r, "Failed to generate stack trace: %m"); + r = RET_NERRNO(pipe2(error_pipe, O_CLOEXEC|O_NONBLOCK)); + if (r < 0) + return r; + + if (ret) { + r = RET_NERRNO(pipe2(return_pipe, O_CLOEXEC)); + if (r < 0) + return r; + } + + if (ret_package_metadata) { + r = RET_NERRNO(pipe2(json_pipe, O_CLOEXEC)); + if (r < 0) + return r; + } + + /* Parsing possibly malformed data is crash-happy, so fork. In case we crash, + * the core file will not be lost, and the messages will still be attached to + * the journal. Reading the elf object might be slow, but it still has an upper + * bound since the core files have an upper size limit. It's also not doing any + * system call or interacting with the system in any way, besides reading from + * the file descriptor and writing into these four pipes. */ + r = safe_fork_full("(sd-parse-elf)", + (int[]){ fd, error_pipe[1], return_pipe[1], json_pipe[1] }, + 4, + FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE|FORK_NEW_USERNS|FORK_WAIT|FORK_REOPEN_LOG, + NULL); + if (r < 0) { + if (r == -EPROTO) { /* We should have the errno from the child, but don't clobber original error */ + int e, k; + + k = read(error_pipe[0], &e, sizeof(e)); + if (k < 0 && errno != EAGAIN) /* Pipe is non-blocking, EAGAIN means there's nothing */ + return -errno; + if (k == sizeof(e)) + return e; /* propagate error sent to us from child */ + if (k != 0) + return -EIO; + } + + return r; + } + if (r == 0) { + /* We want to avoid loops, given this can be called from systemd-coredump */ + if (fork_disable_dump) + prctl(PR_SET_DUMPABLE, 0); + + r = parse_core(fd, executable, ret ? &buf : NULL, ret_package_metadata ? &package_metadata : NULL); + if (r < 0) + goto child_fail; + + if (buf) { + r = loop_write(return_pipe[1], buf, strlen(buf), false); + if (r < 0) + goto child_fail; + + return_pipe[1] = safe_close(return_pipe[1]); + } + + if (package_metadata) { + _cleanup_fclose_ FILE *json_out = NULL; + + json_out = take_fdopen(&json_pipe[1], "w"); + if (!json_out) { + r = -errno; + goto child_fail; + } + + json_variant_dump(package_metadata, JSON_FORMAT_FLUSH, json_out, NULL); + } + + _exit(EXIT_SUCCESS); + + child_fail: + (void) write(error_pipe[1], &r, sizeof(r)); + _exit(EXIT_FAILURE); + } + + error_pipe[1] = safe_close(error_pipe[1]); + return_pipe[1] = safe_close(return_pipe[1]); + json_pipe[1] = safe_close(json_pipe[1]); + + if (ret) { + _cleanup_fclose_ FILE *in = NULL; + + in = take_fdopen(&return_pipe[0], "r"); + if (!in) + return -errno; + + r = read_full_stream(in, &buf, NULL); + if (r < 0) + return r; + } + + if (ret_package_metadata) { + _cleanup_fclose_ FILE *json_in = NULL; + + json_in = take_fdopen(&json_pipe[0], "r"); + if (!json_in) + return -errno; + + r = json_parse_file(json_in, NULL, 0, &package_metadata, NULL, NULL); + if (r < 0 && r != -EINVAL) /* EINVAL: json was empty, so we got nothing, but that's ok */ + return r; + } + + if (ret) + *ret = TAKE_PTR(buf); + if (ret_package_metadata) + *ret_package_metadata = TAKE_PTR(package_metadata); + + return 0; } diff --git a/src/coredump/stacktrace.h b/src/coredump/stacktrace.h index 5039b934dda..429fb5c912d 100644 --- a/src/coredump/stacktrace.h +++ b/src/coredump/stacktrace.h @@ -3,4 +3,7 @@ #include "json.h" -void coredump_parse_core(int fd, const char *executable, char **ret, JsonVariant **ret_package_metadata); +/* Parse an ELF object in a forked process, so that errors while iterating over + * untrusted and potentially malicious data do not propagate to the main caller's process. + * If fork_disable_dump, the child process will not dump core if it crashes. */ +int parse_elf_object(int fd, const char *executable, bool fork_disable_dump, char **ret, JsonVariant **ret_package_metadata); diff --git a/units/systemd-coredump@.service.in b/units/systemd-coredump@.service.in index 6bf2817a8ca..15bfb243b41 100644 --- a/units/systemd-coredump@.service.in +++ b/units/systemd-coredump@.service.in @@ -35,11 +35,10 @@ ProtectKernelTunables=yes ProtectKernelLogs=yes ProtectSystem=strict RestrictAddressFamilies=AF_UNIX -RestrictNamespaces=yes RestrictRealtime=yes RestrictSUIDSGID=yes RuntimeMaxSec=5min StateDirectory=systemd/coredump SystemCallArchitectures=native SystemCallErrorNumber=EPERM -SystemCallFilter=@system-service +SystemCallFilter=@system-service @mount