From e09543b87b76e4dc5285e397e30b0252f928aab5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 8 Oct 2025 14:30:25 +0900 Subject: [PATCH 1/7] coredump-config: several modernizations - introduce CoredumpConfig object that stores all configuration parameters parsed from coredump.conf, - use gperf to generate conf parser table, - parse coredump.conf only when necessary: when systemd-coredump is invoked as kernel helper or by socket actiavation. --- src/coredump/coredump-config.c | 75 ++++++++++++--------------- src/coredump/coredump-config.h | 41 +++++++++++---- src/coredump/coredump-forward.h | 1 + src/coredump/coredump-gperf.gperf | 28 ++++++++++ src/coredump/coredump-kernel-helper.c | 9 +++- src/coredump/coredump-receive.c | 8 ++- src/coredump/coredump-submit.c | 48 ++++++++++------- src/coredump/coredump-submit.h | 6 ++- src/coredump/coredump.c | 5 -- src/coredump/meson.build | 9 ++++ 10 files changed, 146 insertions(+), 84 deletions(-) create mode 100644 src/coredump/coredump-gperf.gperf diff --git a/src/coredump/coredump-config.c b/src/coredump/coredump-config.c index e84b2a0cbf..de2bb68bf3 100644 --- a/src/coredump/coredump-config.c +++ b/src/coredump/coredump-config.c @@ -12,15 +12,6 @@ * size. See DATA_SIZE_MAX in journal-importer.h. */ assert_cc(JOURNAL_SIZE_MAX <= DATA_SIZE_MAX); -CoredumpStorage arg_storage = COREDUMP_STORAGE_EXTERNAL; -bool arg_compress = true; -uint64_t arg_process_size_max = PROCESS_SIZE_MAX; -uint64_t arg_external_size_max = EXTERNAL_SIZE_MAX; -uint64_t arg_journal_size_max = JOURNAL_SIZE_MAX; -uint64_t arg_keep_free = UINT64_MAX; -uint64_t arg_max_use = UINT64_MAX; -bool arg_enter_namespace = false; - static const char* const coredump_storage_table[_COREDUMP_STORAGE_MAX] = { [COREDUMP_STORAGE_NONE] = "none", [COREDUMP_STORAGE_EXTERNAL] = "external", @@ -28,56 +19,54 @@ static const char* const coredump_storage_table[_COREDUMP_STORAGE_MAX] = { }; DEFINE_PRIVATE_STRING_TABLE_LOOKUP(coredump_storage, CoredumpStorage); -static DEFINE_CONFIG_PARSE_ENUM(config_parse_coredump_storage, coredump_storage, CoredumpStorage); - -int coredump_parse_config(void) { - static const ConfigTableItem items[] = { - { "Coredump", "Storage", config_parse_coredump_storage, 0, &arg_storage }, - { "Coredump", "Compress", config_parse_bool, 0, &arg_compress }, - { "Coredump", "ProcessSizeMax", config_parse_iec_uint64, 0, &arg_process_size_max }, - { "Coredump", "ExternalSizeMax", config_parse_iec_uint64_infinity, 0, &arg_external_size_max }, - { "Coredump", "JournalSizeMax", config_parse_iec_size, 0, &arg_journal_size_max }, - { "Coredump", "KeepFree", config_parse_iec_uint64, 0, &arg_keep_free }, - { "Coredump", "MaxUse", config_parse_iec_uint64, 0, &arg_max_use }, -#if HAVE_DWFL_SET_SYSROOT - { "Coredump", "EnterNamespace", config_parse_bool, 0, &arg_enter_namespace }, -#else - { "Coredump", "EnterNamespace", config_parse_warn_compat, DISABLED_CONFIGURATION, NULL }, -#endif - {} - }; +DEFINE_CONFIG_PARSE_ENUM(config_parse_coredump_storage, coredump_storage, CoredumpStorage); +int coredump_parse_config(CoredumpConfig *config) { int r; + assert(config); + r = config_parse_standard_file_with_dropins( "systemd/coredump.conf", "Coredump\0", - config_item_table_lookup, - items, + config_item_perf_lookup, + coredump_gperf_lookup, CONFIG_PARSE_WARN, - /* userdata= */ NULL); + config); if (r < 0) return r; /* Let's make sure we fix up the maximum size we send to the journal here on the client side, for * efficiency reasons. journald wouldn't accept anything larger anyway. */ - if (arg_journal_size_max > JOURNAL_SIZE_MAX) { - log_warning("JournalSizeMax= set to larger value (%s) than journald would accept (%s), lowering automatically.", - FORMAT_BYTES(arg_journal_size_max), FORMAT_BYTES(JOURNAL_SIZE_MAX)); - arg_journal_size_max = JOURNAL_SIZE_MAX; + if (config->journal_size_max > JOURNAL_SIZE_MAX) { + log_full(config->storage == COREDUMP_STORAGE_JOURNAL ? LOG_WARNING : LOG_DEBUG, + "JournalSizeMax= set to larger value (%s) than journald would accept (%s), lowering automatically.", + FORMAT_BYTES(config->journal_size_max), FORMAT_BYTES(JOURNAL_SIZE_MAX)); + config->journal_size_max = JOURNAL_SIZE_MAX; } - log_debug("Selected storage '%s'.", coredump_storage_to_string(arg_storage)); - log_debug("Selected compression %s.", yes_no(arg_compress)); +#if !HAVE_DWFL_SET_SYSROOT + if (config->enter_namespace) { + log_warning("EnterNamespace= is enabled but libdw does not support dwfl_set_sysroot(), disabling."); + config->enter_namespace = false; + } +#endif + + log_debug("Selected storage '%s'.", coredump_storage_to_string(config->storage)); + log_debug("Selected compression %s.", yes_no(config->compress)); return 0; } -uint64_t coredump_storage_size_max(void) { - if (arg_storage == COREDUMP_STORAGE_EXTERNAL) - return arg_external_size_max; - if (arg_storage == COREDUMP_STORAGE_JOURNAL) - return arg_journal_size_max; - assert(arg_storage == COREDUMP_STORAGE_NONE); - return 0; +uint64_t coredump_storage_size_max(const CoredumpConfig *config) { + switch (config->storage) { + case COREDUMP_STORAGE_NONE: + return 0; + case COREDUMP_STORAGE_EXTERNAL: + return config->external_size_max; + case COREDUMP_STORAGE_JOURNAL: + return config->journal_size_max; + default: + assert_not_reached(); + } } diff --git a/src/coredump/coredump-config.h b/src/coredump/coredump-config.h index 5d9e8e82a2..bb278f311e 100644 --- a/src/coredump/coredump-config.h +++ b/src/coredump/coredump-config.h @@ -1,7 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once -#include "basic-forward.h" +#include "conf-parser-forward.h" +#include "coredump-forward.h" /* The maximum size up to which we process coredumps. We use 1G on 32-bit systems, and 32G on 64-bit systems */ #if __SIZEOF_POINTER__ == 4 @@ -31,14 +32,32 @@ typedef enum CoredumpStorage { _COREDUMP_STORAGE_INVALID = -EINVAL, } CoredumpStorage; -extern CoredumpStorage arg_storage; -extern bool arg_compress; -extern uint64_t arg_process_size_max; -extern uint64_t arg_external_size_max; -extern uint64_t arg_journal_size_max; -extern uint64_t arg_keep_free; -extern uint64_t arg_max_use; -extern bool arg_enter_namespace; +struct CoredumpConfig { + CoredumpStorage storage; + bool compress; + uint64_t process_size_max; + uint64_t external_size_max; + uint64_t journal_size_max; + uint64_t keep_free; + uint64_t max_use; + bool enter_namespace; +}; -int coredump_parse_config(void); -uint64_t coredump_storage_size_max(void); +#define COREDUMP_CONFIG_NULL \ + (CoredumpConfig) { \ + .storage = COREDUMP_STORAGE_EXTERNAL, \ + .compress = true, \ + .process_size_max = PROCESS_SIZE_MAX, \ + .external_size_max = EXTERNAL_SIZE_MAX, \ + .journal_size_max = JOURNAL_SIZE_MAX, \ + .keep_free = UINT64_MAX, \ + .max_use = UINT64_MAX, \ + } + +int coredump_parse_config(CoredumpConfig *config); +uint64_t coredump_storage_size_max(const CoredumpConfig *config); + +/* Defined in generated coredump-gperf.c */ +const struct ConfigPerfItem* coredump_gperf_lookup(const char *key, GPERF_LEN_TYPE length); + +CONFIG_PARSER_PROTOTYPE(config_parse_coredump_storage); diff --git a/src/coredump/coredump-forward.h b/src/coredump/coredump-forward.h index 8b95868cfd..b5a2d51a5e 100644 --- a/src/coredump/coredump-forward.h +++ b/src/coredump/coredump-forward.h @@ -3,4 +3,5 @@ #include "basic-forward.h" +typedef struct CoredumpConfig CoredumpConfig; typedef struct Context Context; diff --git a/src/coredump/coredump-gperf.gperf b/src/coredump/coredump-gperf.gperf new file mode 100644 index 0000000000..087c73d513 --- /dev/null +++ b/src/coredump/coredump-gperf.gperf @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +%{ +_Pragma("GCC diagnostic ignored \"-Wimplicit-fallthrough\"") +#if __GNUC__ >= 15 +_Pragma("GCC diagnostic ignored \"-Wzero-as-null-pointer-constant\"") +#endif +#include "conf-parser.h" +#include "coredump-config.h" +%} +struct ConfigPerfItem; +%null_strings +%language=ANSI-C +%define slot-name section_and_lvalue +%define hash-function-name coredump_gperf_hash +%define lookup-function-name coredump_gperf_lookup +%readonly-tables +%omit-struct-type +%struct-type +%includes +%% +Coredump.Storage, config_parse_coredump_storage, 0, offsetof(CoredumpConfig, storage) +Coredump.Compress, config_parse_bool, 0, offsetof(CoredumpConfig, compress) +Coredump.ProcessSizeMax, config_parse_iec_uint64, 0, offsetof(CoredumpConfig, process_size_max) +Coredump.ExternalSizeMax, config_parse_iec_uint64_infinity, 0, offsetof(CoredumpConfig, external_size_max) +Coredump.JournalSizeMax, config_parse_iec_size, 0, offsetof(CoredumpConfig, journal_size_max) +Coredump.KeepFree, config_parse_iec_uint64, 0, offsetof(CoredumpConfig, keep_free) +Coredump.MaxUse, config_parse_iec_uint64, 0, offsetof(CoredumpConfig, max_use) +Coredump.EnterNamespace, config_parse_bool, 0, offsetof(CoredumpConfig, enter_namespace) diff --git a/src/coredump/coredump-kernel-helper.c b/src/coredump/coredump-kernel-helper.c index 88f8584ab1..9cac410b03 100644 --- a/src/coredump/coredump-kernel-helper.c +++ b/src/coredump/coredump-kernel-helper.c @@ -2,6 +2,7 @@ #include "sd-messages.h" +#include "coredump-config.h" #include "coredump-context.h" #include "coredump-kernel-helper.h" #include "coredump-send.h" @@ -25,6 +26,10 @@ int coredump_kernel_helper(int argc, char *argv[]) { if (r < 0) return log_error_errno(r, "Failed to connect stdout/stderr to /dev/null: %m"); + /* Ignore all parse errors */ + CoredumpConfig config = COREDUMP_CONFIG_NULL; + (void) coredump_parse_config(&config); + log_debug("Processing coredump received from the kernel..."); iovw = iovw_new(); @@ -61,7 +66,7 @@ int coredump_kernel_helper(int argc, char *argv[]) { if (r >= 0) return 0; - r = acquire_pid_mount_tree_fd(&context, &context.mount_tree_fd); + r = acquire_pid_mount_tree_fd(&config, &context, &context.mount_tree_fd); if (r < 0) log_warning_errno(r, "Failed to access the mount tree of a container, ignoring: %m"); } @@ -81,7 +86,7 @@ int coredump_kernel_helper(int argc, char *argv[]) { (void) iovw_put_string_field(iovw, "PRIORITY=", STRINGIFY(LOG_CRIT)); if (context.is_journald || context.is_pid1) - return coredump_submit(&context, iovw, STDIN_FILENO); + return coredump_submit(&config, &context, iovw, STDIN_FILENO); return coredump_send(iovw, STDIN_FILENO, &context.pidref, context.mount_tree_fd); } diff --git a/src/coredump/coredump-receive.c b/src/coredump/coredump-receive.c index db9987b3ff..be786be2f1 100644 --- a/src/coredump/coredump-receive.c +++ b/src/coredump/coredump-receive.c @@ -2,6 +2,7 @@ #include +#include "coredump-config.h" #include "coredump-context.h" #include "coredump-receive.h" #include "coredump-submit.h" @@ -25,9 +26,12 @@ int coredump_receive(int fd) { assert(fd >= 0); log_setup(); - log_debug("Processing coredump received via socket..."); + /* Ignore all parse errors */ + CoredumpConfig config = COREDUMP_CONFIG_NULL; + (void) coredump_parse_config(&config); + for (;;) { CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int))) control; struct msghdr mh = { @@ -158,5 +162,5 @@ int coredump_receive(int fd) { "Mandatory argument %s not received on socket, aborting.", meta_field_names[i]); - return coredump_submit(&context, &iovw, input_fd); + return coredump_submit(&config, &context, &iovw, input_fd); } diff --git a/src/coredump/coredump-submit.c b/src/coredump/coredump-submit.c index a4c9c424cf..56093e2d7b 100644 --- a/src/coredump/coredump-submit.c +++ b/src/coredump/coredump-submit.c @@ -232,6 +232,7 @@ static int fix_permissions_and_link( } static int save_external_coredump( + const CoredumpConfig *config, const Context *context, int input_fd, char **ret_filename, @@ -249,6 +250,7 @@ static int save_external_coredump( struct stat st; int r; + assert(config); assert(context); assert(ret_filename); assert(ret_node_fd); @@ -266,7 +268,7 @@ static int save_external_coredump( "Resource limits disable core dumping for process %s (%s).", context->meta[META_ARGV_PID], context->meta[META_COMM]); - process_limit = MAX(arg_process_size_max, coredump_storage_size_max()); + process_limit = MAX(config->process_size_max, coredump_storage_size_max(config)); if (process_limit == 0) return log_debug_errno(SYNTHETIC_ERRNO(EBADSLT), "Limits for coredump processing and storage are both 0, not dumping core."); @@ -297,7 +299,7 @@ static int save_external_coredump( * should be able to at least store the full compressed core file. */ storage_on_tmpfs = fd_is_temporary_fs(fd) > 0; - if (storage_on_tmpfs && arg_compress) { + if (storage_on_tmpfs && config->compress) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; uint64_t cgroup_limit = UINT64_MAX; struct statvfs sv; @@ -351,7 +353,7 @@ static int save_external_coredump( bool allow_user = grant_user_access(fd, context) > 0; #if HAVE_COMPRESSION - if (arg_compress) { + if (config->compress) { _cleanup_(unlink_and_freep) char *tmp_compressed = NULL; _cleanup_free_ char *fn_compressed = NULL; _cleanup_close_ int fd_compressed = -EBADF; @@ -434,20 +436,22 @@ static int save_external_coredump( } static int maybe_remove_external_coredump( - const Context *c, + const CoredumpConfig *config, + const Context *context, const char *filename, uint64_t size) { - assert(c); + assert(config); + assert(context); /* Returns true if might remove, false if will not remove, < 0 on error. */ /* Always keep around in case of journald/pid1, since we cannot rely on the journal to accept them. */ - if (arg_storage != COREDUMP_STORAGE_NONE && (c->is_pid1 || c->is_journald)) + if (config->storage != COREDUMP_STORAGE_NONE && (context->is_pid1 || context->is_journald)) return false; - if (arg_storage == COREDUMP_STORAGE_EXTERNAL && - size <= arg_external_size_max) + if (config->storage == COREDUMP_STORAGE_EXTERNAL && + size <= config->external_size_max) return false; if (!filename) @@ -459,7 +463,7 @@ static int maybe_remove_external_coredump( return true; } -int acquire_pid_mount_tree_fd(const Context *context, int *ret_fd) { +int acquire_pid_mount_tree_fd(const CoredumpConfig *config, const Context *context, int *ret_fd) { /* Don't bother preparing environment if we can't pass it to libdwfl. */ #if !HAVE_DWFL_SET_SYSROOT *ret_fd = -EOPNOTSUPP; @@ -469,10 +473,11 @@ int acquire_pid_mount_tree_fd(const Context *context, int *ret_fd) { _cleanup_close_pair_ int pair[2] = EBADF_PAIR; int r; + assert(config); assert(context); assert(ret_fd); - if (!arg_enter_namespace) { + if (!config->enter_namespace) { *ret_fd = -EHOSTDOWN; log_debug("EnterNamespace=no so we won't use mount tree of the crashed process for generating backtrace."); return 0; @@ -618,6 +623,7 @@ static int allocate_journal_field(int fd, size_t size, char **ret, size_t *ret_s } int coredump_submit( + const CoredumpConfig *config, const Context *context, struct iovec_wrapper *iovw, int input_fd) { @@ -631,16 +637,17 @@ int coredump_submit( sd_json_variant *module_json; int r; + assert(config); assert(context); assert(iovw); assert(input_fd >= 0); /* Vacuum before we write anything again */ - (void) coredump_vacuum(-1, arg_keep_free, arg_max_use); + (void) coredump_vacuum(-1, config->keep_free, config->max_use); /* Always stream the coredump to disk, if that's possible */ written = save_external_coredump( - context, input_fd, + config, context, input_fd, &filename, &coredump_node_fd, &coredump_fd, &coredump_size, &coredump_compressed_size, &truncated) >= 0; if (written) { @@ -649,6 +656,7 @@ int coredump_submit( * will lack the privileges for it. However, we keep the fd to it, so that we can * still process it and log it. */ r = maybe_remove_external_coredump( + config, context, filename, coredump_node_fd >= 0 ? coredump_compressed_size : coredump_size); @@ -656,12 +664,12 @@ int coredump_submit( return r; if (r == 0) (void) iovw_put_string_field(iovw, "COREDUMP_FILENAME=", filename); - else if (arg_storage == COREDUMP_STORAGE_EXTERNAL) + else if (config->storage == COREDUMP_STORAGE_EXTERNAL) log_info("The core will not be stored: size %"PRIu64" is greater than %"PRIu64" (the configured maximum)", - coredump_node_fd >= 0 ? coredump_compressed_size : coredump_size, arg_external_size_max); + coredump_node_fd >= 0 ? coredump_compressed_size : coredump_size, config->external_size_max); /* Vacuum again, but exclude the coredump we just created */ - (void) coredump_vacuum(coredump_node_fd >= 0 ? coredump_node_fd : coredump_fd, arg_keep_free, arg_max_use); + (void) coredump_vacuum(coredump_node_fd >= 0 ? coredump_node_fd : coredump_fd, config->keep_free, config->max_use); } if (context->mount_tree_fd >= 0 && attach_mount_tree(context->mount_tree_fd) >= 0) @@ -677,10 +685,10 @@ int coredump_submit( if (written) { /* Try to get a stack trace if we can */ - if (coredump_size > arg_process_size_max) + if (coredump_size > config->process_size_max) log_debug("Not generating stack trace: core size %"PRIu64" is greater " "than %"PRIu64" (the configured maximum)", - coredump_size, arg_process_size_max); + coredump_size, config->process_size_max); else if (coredump_fd >= 0) { bool skip = startswith(context->meta[META_COMM], "systemd-coredum"); /* COMM is 16 bytes usually */ @@ -752,8 +760,8 @@ int coredump_submit( } /* Optionally store the entire coredump in the journal */ - if (arg_storage == COREDUMP_STORAGE_JOURNAL && coredump_fd >= 0) { - if (coredump_size <= arg_journal_size_max) { + if (config->storage == COREDUMP_STORAGE_JOURNAL && coredump_fd >= 0) { + if (coredump_size <= config->journal_size_max) { size_t sz = 0; /* Store the coredump itself in the journal */ @@ -766,7 +774,7 @@ int coredump_submit( log_warning_errno(r, "Failed to attach the core to the journal entry: %m"); } else log_info("The core will not be stored: size %"PRIu64" is greater than %"PRIu64" (the configured maximum)", - coredump_size, arg_journal_size_max); + coredump_size, config->journal_size_max); } /* If journald is coredumping, we have to be careful that we don't deadlock when trying to write the diff --git a/src/coredump/coredump-submit.h b/src/coredump/coredump-submit.h index 35ff30f3da..460b2d14b1 100644 --- a/src/coredump/coredump-submit.h +++ b/src/coredump/coredump-submit.h @@ -3,8 +3,12 @@ #include "coredump-forward.h" -int acquire_pid_mount_tree_fd(const Context *context, int *ret_fd); +int acquire_pid_mount_tree_fd( + const CoredumpConfig *config, + const Context *context, + int *ret_fd); int coredump_submit( + const CoredumpConfig *config, const Context *context, struct iovec_wrapper *iovw, int input_fd); diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index b89503a6ce..6e110db630 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -3,8 +3,6 @@ #include "sd-daemon.h" #include "coredump-backtrace.h" -#include "coredump-config.h" -#include "coredump-context.h" #include "coredump-kernel-helper.h" #include "coredump-receive.h" #include "coredump-util.h" @@ -23,9 +21,6 @@ static int run(int argc, char *argv[]) { /* Make sure we never enter a loop */ (void) set_dumpable(SUID_DUMP_DISABLE); - /* Ignore all parse errors */ - (void) coredump_parse_config(); - r = sd_listen_fds(false); if (r < 0) return log_error_errno(r, "Failed to determine the number of file descriptors: %m"); diff --git a/src/coredump/meson.build b/src/coredump/meson.build index a2638390a1..a401ad34fe 100644 --- a/src/coredump/meson.build +++ b/src/coredump/meson.build @@ -18,6 +18,14 @@ systemd_coredump_extract_sources = files( 'coredump-vacuum.c', ) +coredump_gperf_c = custom_target( + input : 'coredump-gperf.gperf', + output : 'coredump-gperf.c', + command : [gperf, '@INPUT@', '--output-file', '@OUTPUT@']) + +generated_sources += coredump_gperf_c +systemd_coredump_sources += coredump_gperf_c + common_dependencies = [ liblz4_cflags, libxz_cflags, @@ -29,6 +37,7 @@ executables += [ libexec_template + { 'name' : 'systemd-coredump', 'sources' : systemd_coredump_sources + systemd_coredump_extract_sources, + 'include_directories' : [libexec_template['include_directories'], include_directories('.')], 'extract' : systemd_coredump_extract_sources, 'link_with' : [libshared], 'dependencies' : common_dependencies, From 4d73712ef744adeade9377bb268867521b048655 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 9 Oct 2025 00:39:11 +0900 Subject: [PATCH 2/7] coredump: shortcut to start backtrace mode When running as backtrace mode, it is not necessary to use kmsg, not necessary to disable coredump from the command, and unexpectedly passed file descriptors can be silently ignored. --- src/coredump/coredump-backtrace.c | 1 + src/coredump/coredump.c | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/coredump/coredump-backtrace.c b/src/coredump/coredump-backtrace.c index 4e223e5220..4d80cae24c 100644 --- a/src/coredump/coredump-backtrace.c +++ b/src/coredump/coredump-backtrace.c @@ -18,6 +18,7 @@ int coredump_backtrace(int argc, char *argv[]) { assert(argc >= 2); + log_setup(); log_debug("Processing backtrace on stdin..."); iovw = iovw_new(); diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 6e110db630..c29976e281 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -13,9 +13,13 @@ static int run(int argc, char *argv[]) { int r; + /* When running as backtrace mode, it is not necessary to use kmsg, not necessary to disable coredump + * from the command, and unexpectedly passed file descriptors can be silently ignored. */ + if (streq_ptr(argv[1], "--backtrace")) + return coredump_backtrace(argc, argv); + /* First, log to a safe place, since we don't know what crashed and it might * be journald which we'd rather not log to then. */ - log_set_target_and_open(LOG_TARGET_KMSG); /* Make sure we never enter a loop */ @@ -25,14 +29,11 @@ static int run(int argc, char *argv[]) { if (r < 0) return log_error_errno(r, "Failed to determine the number of file descriptors: %m"); - /* If we got an fd passed, we are running in coredumpd mode. Otherwise we - * are invoked from the kernel as coredump handler. */ - if (r == 0) { - if (streq_ptr(argv[1], "--backtrace")) - return coredump_backtrace(argc, argv); - else - return coredump_kernel_helper(argc, argv); - } else if (r == 1) + /* If we got an fd passed, we are running in coredumpd mode. Otherwise we are invoked from the + * kernel as coredump handler. */ + if (r == 0) + return coredump_kernel_helper(argc, argv); + if (r == 1) return coredump_receive(SD_LISTEN_FDS_START); return log_error_errno(SYNTHETIC_ERRNO(EINVAL), From d7755c9caf55da2df3969709840022619fe1d20b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 13 Oct 2025 10:35:22 +0900 Subject: [PATCH 3/7] coredump: allow to control log level and so on via command line --- src/coredump/coredump.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index c29976e281..6a758ac277 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -18,11 +18,12 @@ static int run(int argc, char *argv[]) { if (streq_ptr(argv[1], "--backtrace")) return coredump_backtrace(argc, argv); - /* First, log to a safe place, since we don't know what crashed and it might - * be journald which we'd rather not log to then. */ + /* First, log to a safe place, since we don't know what crashed and it might be journald which we'd + * rather not log to then. */ + log_parse_environment(); log_set_target_and_open(LOG_TARGET_KMSG); - /* Make sure we never enter a loop */ + /* Make sure we never enter a loop. */ (void) set_dumpable(SUID_DUMP_DISABLE); r = sd_listen_fds(false); From 7df4841e9465a565d9a6b8260f86be59f89c8b4d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 17 Oct 2025 09:30:33 +0900 Subject: [PATCH 4/7] coredump-context: several cleanups - Renames Context to CoredumpContext. - Renames enum meta_argv_t to MetadataField, and use the correct type at several pleaces, especially in loop. - Parses argv (when invoked as kernel helper or with '--backtrace') or iov (when invoked through socket) and store values in the context. Previously we use the passed string as is, but let's make them in a type safe way. - Stores received or built iovw in the context. Also store input fd and mountfs fd in the context. - Introduces coredump_context_is_pid1() and _is_journald() helper functions. - Adds COREDUMP_PIDFDID= field when the kernel passed pidfd of the crashed process. No effective funtional change. Just refactoring. --- src/coredump/coredump-backtrace.c | 33 +- src/coredump/coredump-context.c | 528 ++++++++++++++++---------- src/coredump/coredump-context.h | 57 +-- src/coredump/coredump-forward.h | 2 +- src/coredump/coredump-kernel-helper.c | 53 ++- src/coredump/coredump-receive.c | 36 +- src/coredump/coredump-send.c | 95 ++--- src/coredump/coredump-send.h | 4 +- src/coredump/coredump-submit.c | 219 ++++++----- src/coredump/coredump-submit.h | 11 +- 10 files changed, 532 insertions(+), 506 deletions(-) diff --git a/src/coredump/coredump-backtrace.c b/src/coredump/coredump-backtrace.c index 4d80cae24c..f18b5ff800 100644 --- a/src/coredump/coredump-backtrace.c +++ b/src/coredump/coredump-backtrace.c @@ -5,15 +5,15 @@ #include "coredump-backtrace.h" #include "coredump-context.h" -#include "iovec-util.h" +#include "format-util.h" #include "journal-importer.h" #include "log.h" #include "string-util.h" +#include "user-util.h" int coredump_backtrace(int argc, char *argv[]) { _cleanup_(journal_importer_cleanup) JournalImporter importer = JOURNAL_IMPORTER_INIT(STDIN_FILENO); - _cleanup_(iovw_free_freep) struct iovec_wrapper *iovw = NULL; - _cleanup_(context_done) Context context = CONTEXT_NULL; + _cleanup_(coredump_context_done) CoredumpContext context = COREDUMP_CONTEXT_NULL; int r; assert(argc >= 2); @@ -21,23 +21,17 @@ int coredump_backtrace(int argc, char *argv[]) { log_setup(); log_debug("Processing backtrace on stdin..."); - iovw = iovw_new(); - if (!iovw) - return log_oom(); - - (void) iovw_put_string_field(iovw, "MESSAGE_ID=", SD_MESSAGE_BACKTRACE_STR); - (void) iovw_put_string_field(iovw, "PRIORITY=", STRINGIFY(LOG_CRIT)); - /* Collect all process metadata from argv[] by making sure to skip the '--backtrace' option. */ - r = gather_pid_metadata_from_argv(iovw, &context, argc - 2, argv + 2); + r = coredump_context_parse_from_argv(&context, argc - 2, argv + 2); if (r < 0) return r; - /* Collect the rest of the process metadata retrieved from the runtime */ - r = gather_pid_metadata_from_procfs(iovw, &context); + r = coredump_context_build_iovw(&context); if (r < 0) return r; + (void) iovw_replace_string_field(&context.iovw, "MESSAGE_ID=", SD_MESSAGE_BACKTRACE_STR); + for (;;) { r = journal_importer_process_data(&importer); if (r < 0) @@ -49,24 +43,19 @@ int coredump_backtrace(int argc, char *argv[]) { if (journal_importer_eof(&importer)) { log_warning("Did not receive a full journal entry on stdin, ignoring message sent by reporter."); - - const char *message = strjoina("Process ", context.meta[META_ARGV_PID], - " (", context.meta[META_COMM], ")" - " of user ", context.meta[META_ARGV_UID], - " failed with ", context.meta[META_ARGV_SIGNAL]); - - r = iovw_put_string_field(iovw, "MESSAGE=", message); + r = iovw_put_string_fieldf(&context.iovw, "MESSAGE=", "Process "PID_FMT" (%s) of user "UID_FMT" failed with %i.", + context.pidref.pid, context.comm, context.uid, context.signo); if (r < 0) return r; } else { /* The imported iovecs are not supposed to be freed by us so let's copy and merge them at the * end of the array. */ - r = iovw_append(iovw, &importer.iovw); + r = iovw_append(&context.iovw, &importer.iovw); if (r < 0) return r; } - r = sd_journal_sendv(iovw->iovec, iovw->count); + r = sd_journal_sendv(context.iovw.iovec, context.iovw.count); if (r < 0) return log_error_errno(r, "Failed to log backtrace: %m"); diff --git a/src/coredump/coredump-context.c b/src/coredump/coredump-context.c index 6e62c4615f..db5dd2e141 100644 --- a/src/coredump/coredump-context.c +++ b/src/coredump/coredump-context.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include "sd-login.h" +#include "sd-messages.h" #include "coredump-config.h" #include "coredump-context.h" @@ -8,6 +9,7 @@ #include "dirent-util.h" #include "fd-util.h" #include "fs-util.h" +#include "hostname-util.h" #include "iovec-wrapper.h" #include "log.h" #include "memstream-util.h" @@ -16,10 +18,12 @@ #include "process-util.h" #include "signal-util.h" #include "special.h" +#include "string-table.h" #include "string-util.h" +#include "time-util.h" #include "user-util.h" -const char * const meta_field_names[_META_MAX] = { +static const char * const metadata_field_table[_META_MAX] = { [META_ARGV_PID] = "COREDUMP_PID=", [META_ARGV_UID] = "COREDUMP_UID=", [META_ARGV_GID] = "COREDUMP_GID=", @@ -35,11 +39,30 @@ const char * const meta_field_names[_META_MAX] = { [META_PROC_AUXV] = "COREDUMP_PROC_AUXV=", }; -void context_done(Context *c) { - assert(c); +DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(metadata_field, MetadataField); - pidref_done(&c->pidref); - c->mount_tree_fd = safe_close(c->mount_tree_fd); +void coredump_context_done(CoredumpContext *context) { + assert(context); + + pidref_done(&context->pidref); + free(context->hostname); + free(context->comm); + free(context->exe); + free(context->unit); + free(context->auxv); + safe_close(context->mount_tree_fd); + iovw_done_free(&context->iovw); + safe_close(context->input_fd); +} + +bool coredump_context_is_pid1(CoredumpContext *context) { + assert(context); + return context->pidref.pid == 1 || streq_ptr(context->unit, SPECIAL_INIT_SCOPE); +} + +bool coredump_context_is_journald(CoredumpContext *context) { + assert(context); + return streq_ptr(context->unit, SPECIAL_JOURNALD_SERVICE); } /* Joins /proc/[pid]/fd/ and /proc/[pid]/fdinfo/ into the following lines: @@ -150,215 +173,370 @@ static int get_process_container_parent_cmdline(PidRef *pid, char** ret_cmdline) return 1; } -int gather_pid_metadata_from_procfs(struct iovec_wrapper *iovw, Context *context) { +int coredump_context_build_iovw(CoredumpContext *context) { char *t; - size_t size; int r; - assert(iovw); assert(context); + assert(pidref_is_set(&context->pidref)); - /* Note that if we fail on oom later on, we do not roll-back changes to the iovec - * structure. (It remains valid, with the first iovec fields initialized.) */ + if (!iovw_isempty(&context->iovw)) + return 0; pid_t pid = context->pidref.pid; - /* The following is mandatory */ - r = pidref_get_comm(&context->pidref, &t); + r = iovw_put_string_fieldf(&context->iovw, "COREDUMP_PID=", PID_FMT, context->pidref.pid); if (r < 0) - return log_error_errno(r, "Failed to get COMM: %m"); + return log_error_errno(r, "Failed to add COREDUMP_PID= field: %m"); - r = iovw_put_string_field_free(iovw, "COREDUMP_COMM=", t); - if (r < 0) - return r; + if (context->got_pidfd) { + (void) iovw_put_string_field(&context->iovw, "COREDUMP_BY_PIDFD=", "1"); - /* The following are optional, but we use them if present. */ - r = get_process_exe(pid, &t); - if (r >= 0) - r = iovw_put_string_field_free(iovw, "COREDUMP_EXE=", t); - if (r < 0) - log_warning_errno(r, "Failed to get EXE, ignoring: %m"); - - if (cg_pidref_get_unit(&context->pidref, &t) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_UNIT=", t); - - if (cg_pidref_get_user_unit(&context->pidref, &t) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_USER_UNIT=", t); - - if (cg_pidref_get_session(&context->pidref, &t) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_SESSION=", t); - - uid_t owner_uid; - if (cg_pidref_get_owner_uid(&context->pidref, &owner_uid) >= 0) { - r = asprintf(&t, UID_FMT, owner_uid); - if (r > 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_OWNER_UID=", t); + if (pidref_acquire_pidfd_id(&context->pidref) >= 0) + (void) iovw_put_string_fieldf(&context->iovw, "COREDUMP_PIDFDID=", "%"PRIu64, context->pidref.fd_id); } + r = iovw_put_string_fieldf(&context->iovw, "COREDUMP_UID=", UID_FMT, context->uid); + if (r < 0) + return log_error_errno(r, "Failed to add COREDUMP_UID= field: %m"); + + r = iovw_put_string_fieldf(&context->iovw, "COREDUMP_GID=", UID_FMT, context->gid); + if (r < 0) + return log_error_errno(r, "Failed to add COREDUMP_GID= field: %m"); + + if (SIGNAL_VALID(context->signo)) { + r = iovw_put_string_fieldf(&context->iovw, "COREDUMP_SIGNAL=", "%i", context->signo); + if (r < 0) + return log_error_errno(r, "Failed to add COREDUMP_SIGNAL= field: %m"); + + (void) iovw_put_string_field(&context->iovw, "COREDUMP_SIGNAL_NAME=SIG", signal_to_string(context->signo)); + } + + r = iovw_put_string_fieldf(&context->iovw, "COREDUMP_TIMESTAMP=", USEC_FMT, context->timestamp); + if (r < 0) + return log_error_errno(r, "Failed to add COREDUMP_TIMESTAMP= field: %m"); + + r = iovw_put_string_fieldf(&context->iovw, "COREDUMP_RLIMIT=", "%"PRIu64, context->rlimit); + if (r < 0) + return log_error_errno(r, "Failed to add COREDUMP_RLIMIT= field: %m"); + + if (context->hostname) + (void) iovw_put_string_field(&context->iovw, "COREDUMP_HOSTNAME=", context->hostname); + + (void) iovw_put_string_fieldf(&context->iovw, "COREDUMP_DUMPABLE=", "%u", context->dumpable); + + r = iovw_put_string_field(&context->iovw, "COREDUMP_COMM=", context->comm); + if (r < 0) + return log_error_errno(r, "Failed to add COREDUMP_COMM= field: %m"); + + if (context->exe) + (void) iovw_put_string_field(&context->iovw, "COREDUMP_EXE=", context->exe); + + (void) iovw_put_string_field(&context->iovw, "COREDUMP_UNIT=", context->unit); + + if (cg_pidref_get_user_unit(&context->pidref, &t) >= 0) + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_USER_UNIT=", t); + + if (cg_pidref_get_session(&context->pidref, &t) >= 0) + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_SESSION=", t); + + uid_t owner_uid; + if (cg_pidref_get_owner_uid(&context->pidref, &owner_uid) >= 0) + (void) iovw_put_string_fieldf(&context->iovw, "COREDUMP_OWNER_UID=", UID_FMT, owner_uid); + if (sd_pid_get_slice(pid, &t) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_SLICE=", t); + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_SLICE=", t); if (pidref_get_cmdline(&context->pidref, SIZE_MAX, PROCESS_CMDLINE_QUOTE_POSIX, &t) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_CMDLINE=", t); + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_CMDLINE=", t); if (cg_pid_get_path_shifted(pid, NULL, &t) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_CGROUP=", t); + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_CGROUP=", t); if (compose_open_fds(pid, &t) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_OPEN_FDS=", t); + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_OPEN_FDS=", t); if (read_full_file(procfs_file_alloca(pid, "status"), &t, /* ret_size= */ NULL) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_STATUS=", t); + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_PROC_STATUS=", t); if (read_full_file(procfs_file_alloca(pid, "maps"), &t, /* ret_size= */ NULL) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_MAPS=", t); + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_PROC_MAPS=", t); if (read_full_file(procfs_file_alloca(pid, "limits"), &t, /* ret_size= */ NULL) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_LIMITS=", t); + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_PROC_LIMITS=", t); if (read_full_file(procfs_file_alloca(pid, "cgroup"), &t, /* ret_size= */ NULL) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_CGROUP=", t); + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_PROC_CGROUP=", t); if (read_full_file(procfs_file_alloca(pid, "mountinfo"), &t, /* ret_size= */ NULL) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_MOUNTINFO=", t); + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_PROC_MOUNTINFO=", t); /* We attach /proc/auxv here. ELF coredumps also contain a note for this (NT_AUXV), see elf(5). */ - if (read_full_file(procfs_file_alloca(pid, "auxv"), &t, &size) >= 0) { - char *buf = malloc(strlen("COREDUMP_PROC_AUXV=") + size + 1); + if (context->auxv) { + size_t sz = STRLEN("COREDUMP_PROC_AUXV=") + context->auxv_size; + char *buf = malloc(sz + 1); if (buf) { - /* Add a dummy terminator to make context_parse_iovw() happy. */ - *mempcpy_typesafe(stpcpy(buf, "COREDUMP_PROC_AUXV="), t, size) = '\0'; - (void) iovw_consume(iovw, buf, size + strlen("COREDUMP_PROC_AUXV=")); + /* Add a dummy terminator to make coredump_context_parse_iovw() happy. */ + *mempcpy_typesafe(stpcpy(buf, "COREDUMP_PROC_AUXV="), context->auxv, context->auxv_size) = '\0'; + (void) iovw_consume(&context->iovw, buf, sz); } - - free(t); } if (get_process_cwd(pid, &t) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_CWD=", t); + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_CWD=", t); if (get_process_root(pid, &t) >= 0) { bool proc_self_root_is_slash; proc_self_root_is_slash = strcmp(t, "/") == 0; - (void) iovw_put_string_field_free(iovw, "COREDUMP_ROOT=", t); + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_ROOT=", t); /* If the process' root is "/", then there is a chance it has * mounted own root and hence being containerized. */ if (proc_self_root_is_slash && get_process_container_parent_cmdline(&context->pidref, &t) > 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_CONTAINER_CMDLINE=", t); + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_CONTAINER_CMDLINE=", t); } if (get_process_environ(pid, &t) >= 0) - (void) iovw_put_string_field_free(iovw, "COREDUMP_ENVIRON=", t); + (void) iovw_put_string_field_free(&context->iovw, "COREDUMP_ENVIRON=", t); + + if (context->forwarded) + (void) iovw_put_string_field(&context->iovw, "COREDUMP_FORWARDED=", "1"); + + (void) iovw_put_string_field(&context->iovw, "PRIORITY=", STRINGIFY(LOG_CRIT)); + (void) iovw_put_string_field(&context->iovw, "MESSAGE_ID=", SD_MESSAGE_COREDUMP_STR); /* Now that we have parsed info from /proc/ ensure the pidfd is still valid before continuing. */ r = pidref_verify(&context->pidref); if (r < 0) return log_error_errno(r, "PIDFD validation failed: %m"); - /* We successfully acquired all metadata. */ - return context_parse_iovw(context, iovw); + return 0; } -int context_parse_iovw(Context *context, struct iovec_wrapper *iovw) { - const char *unit; +static int coredump_context_parse_from_procfs(CoredumpContext *context) { int r; assert(context); - assert(iovw); + assert(pidref_is_set(&context->pidref)); - /* Converts the data in the iovec array iovw into separate fields. Fills in context->meta[] (for - * which no memory is allocated, it just contains direct pointers into the iovec array memory). */ + pid_t pid = context->pidref.pid; - bool have_signal_name = false; - FOREACH_ARRAY(iovec, iovw->iovec, iovw->count) { - /* Note that these strings are NUL-terminated, because we made sure that a trailing NUL byte - * is in the buffer, though not included in the iov_len count. See coredump_receive() and - * gather_pid_metadata_*(). */ - assert(((char*) iovec->iov_base)[iovec->iov_len] == 0); - - for (size_t i = 0; i < ELEMENTSOF(meta_field_names); i++) { - const char *p = memory_startswith(iovec->iov_base, iovec->iov_len, meta_field_names[i]); - if (p) { - context->meta[i] = p; - context->meta_size[i] = iovec->iov_len - strlen(meta_field_names[i]); - break; - } - } - - have_signal_name = have_signal_name || - memory_startswith(iovec->iov_base, iovec->iov_len, "COREDUMP_SIGNAL_NAME="); - } - - /* The basic fields from argv[] should always be there, refuse early if not. */ - for (int i = 0; i < _META_ARGV_REQUIRED; i++) - if (!context->meta[i]) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "A required (%s) has not been sent, aborting.", meta_field_names[i]); - - pid_t parsed_pid; - r = parse_pid(context->meta[META_ARGV_PID], &parsed_pid); + r = pidref_get_comm(&context->pidref, &context->comm); if (r < 0) - return log_error_errno(r, "Failed to parse PID \"%s\": %m", context->meta[META_ARGV_PID]); - if (pidref_is_set(&context->pidref)) { - if (context->pidref.pid != parsed_pid) - return log_error_errno(r, "Passed PID " PID_FMT " does not match passed " PID_FMT ": %m", - parsed_pid, context->pidref.pid); - } else { - r = pidref_set_pid(&context->pidref, parsed_pid); - if (r < 0) - return log_error_errno(r, "Failed to initialize pidref from pid " PID_FMT ": %m", parsed_pid); - } + return log_error_errno(r, "Failed to get COMM: %m"); - r = parse_uid(context->meta[META_ARGV_UID], &context->uid); + r = get_process_exe(pid, &context->exe); if (r < 0) - return log_error_errno(r, "Failed to parse UID \"%s\": %m", context->meta[META_ARGV_UID]); + log_warning_errno(r, "Failed to get EXE, ignoring: %m"); - r = parse_gid(context->meta[META_ARGV_GID], &context->gid); + r = cg_pidref_get_unit(&context->pidref, &context->unit); if (r < 0) - return log_error_errno(r, "Failed to parse GID \"%s\": %m", context->meta[META_ARGV_GID]); + log_warning_errno(r, "Failed to get unit, ignoring: %m"); - r = parse_signo(context->meta[META_ARGV_SIGNAL], &context->signo); + r = read_full_file(procfs_file_alloca(pid, "auxv"), &context->auxv, &context->auxv_size); if (r < 0) - log_warning_errno(r, "Failed to parse signal number \"%s\", ignoring: %m", context->meta[META_ARGV_SIGNAL]); + log_warning_errno(r, "Failed to get auxv, ignoring: %m"); - r = safe_atou64(context->meta[META_ARGV_RLIMIT], &context->rlimit); + r = pidref_verify(&context->pidref); if (r < 0) - log_warning_errno(r, "Failed to parse resource limit \"%s\", ignoring: %m", context->meta[META_ARGV_RLIMIT]); - - /* The value is set to contents of /proc/sys/fs/suid_dumpable, which we set to SUID_DUMP_SAFE (2), - * if the process is marked as not dumpable, see PR_SET_DUMPABLE(2const). */ - if (context->meta[META_ARGV_DUMPABLE]) { - r = safe_atou(context->meta[META_ARGV_DUMPABLE], &context->dumpable); - if (r < 0) - return log_error_errno(r, "Failed to parse dumpable field \"%s\": %m", context->meta[META_ARGV_DUMPABLE]); - if (context->dumpable > SUID_DUMP_SAFE) - log_notice("Got unexpected %%d/dumpable value %u.", context->dumpable); - } - - unit = context->meta[META_UNIT]; - context->is_pid1 = streq(context->meta[META_ARGV_PID], "1") || streq_ptr(unit, SPECIAL_INIT_SCOPE); - context->is_journald = streq_ptr(unit, SPECIAL_JOURNALD_SERVICE); - - /* After parsing everything, let's also synthesize a new iovw field for the textual signal name if it - * isn't already set. */ - if (SIGNAL_VALID(context->signo) && !have_signal_name) - (void) iovw_put_string_field(iovw, "COREDUMP_SIGNAL_NAME=SIG", signal_to_string(context->signo)); + return log_error_errno(r, "PIDFD validation failed: %m"); return 0; } -int gather_pid_metadata_from_argv( - struct iovec_wrapper *iovw, - Context *context, - int argc, char **argv) { +static int context_parse_one(CoredumpContext *context, MetadataField meta, bool from_argv, const char *s, size_t size) { + int r; - _cleanup_(pidref_done) PidRef local_pidref = PIDREF_NULL; - int r, kernel_fd = -EBADF; + assert(context); + assert(s); + + switch (meta) { + case META_ARGV_PID: { + /* Store this so that we can check whether the core will be forwarded to a container + * even when the kernel doesn't provide a pidfd. Can be dropped once baseline is + * >= v6.16. */ + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + r = pidref_set_pidstr(&pidref, s); + if (r < 0) + return log_error_errno(r, "Failed to initialize pidref from pid %s: %m", s); + + if (pidref_is_set(&context->pidref)) { + if (!pidref_equal(&context->pidref, &pidref)) + return log_error_errno(SYNTHETIC_ERRNO(ESTALE), "Received conflicting pid: %s", s); + } else + context->pidref = TAKE_PIDREF(pidref); + return 0; + } + case META_ARGV_UID: + r = parse_uid(s, &context->uid); + if (r < 0) + return log_error_errno(r, "Failed to parse UID \"%s\": %m", s); + return 0; + + case META_ARGV_GID: + r = parse_gid(s, &context->gid); + if (r < 0) + return log_error_errno(r, "Failed to parse GID \"%s\": %m", s); + return 0; + + case META_ARGV_SIGNAL: + r = parse_signo(s, &context->signo); + if (r < 0) + log_warning_errno(r, "Failed to parse signal number \"%s\", ignoring: %m", s); + return 0; + + case META_ARGV_TIMESTAMP: + /* The kernel provides 1 sec granularity timestamps, while we forward it with 1 μsec granularity. */ + r = parse_time(s, &context->timestamp, from_argv ? USEC_PER_SEC : 1); + if (r < 0) + log_warning_errno(r, "Failed to parse timestamp \"%s\", ignoring: %m", s); + return 0; + + case META_ARGV_RLIMIT: + r = safe_atou64(s, &context->rlimit); + if (r < 0) + log_warning_errno(r, "Failed to parse resource limit \"%s\", ignoring: %m", s); + return 0; + + case META_ARGV_HOSTNAME: + if (!hostname_is_valid(s, /* flags= */ 0)) { + log_warning("Received coredump with an invalid hostname, ignoring: %s", s); + return 0; + } + + return free_and_strdup_warn(&context->hostname, s); + + case META_ARGV_DUMPABLE: + /* The value is set to contents of /proc/sys/fs/suid_dumpable, which we set to SUID_DUMP_SAFE (2), + * if the process is marked as not dumpable, see PR_SET_DUMPABLE(2const). */ + r = safe_atou(s, &context->dumpable); + if (r < 0) + return log_error_errno(r, "Failed to parse dumpable field \"%s\": %m", s); + if (context->dumpable > SUID_DUMP_SAFE) + log_notice("Got unexpected %%d/dumpable value %u.", context->dumpable); + return 0; + + case META_ARGV_PIDFD: { + /* We do not forward the index of the file descriptor, as it is meaningless, and always set to 1. */ + if (!from_argv) { + if (!streq(s, "1")) + return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "Received unexpected pidfd field: %s", s); + if (!pidref_is_set(&context->pidref) || !context->got_pidfd) + return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "Received unexpected pidfd field without pidfd."); + return 0; + } + + /* If the current kernel doesn't support the %F specifier (which resolves to a pidfd), but we + * included it in the core_pattern expression, we'll receive an empty string here. Deal with + * that gracefully. */ + if (isempty(s)) + return 0; + + r = parse_fd(s); + if (r < 0) + return log_error_errno(r, "Failed to parse pidfd \"%s\": %m", s); + + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + r = pidref_set_pidfd_consume(&pidref, r); + if (r < 0) + return log_error_errno(r, "Failed to initialize pidref from pidfd \"%s\": %m", s); + + if (pidref_is_set(&context->pidref) && !pidref_equal(&context->pidref, &pidref)) + return log_error_errno(SYNTHETIC_ERRNO(ESTALE), "Received conflicting pidfd: %s", s); + + /* pidref by pidfd has higher preference over one by pid. */ + pidref_done(&context->pidref); + context->pidref = TAKE_PIDREF(pidref); + + context->got_pidfd = 1; + return 0; + } + case META_COMM: + return free_and_strdup_warn(&context->comm, s); + + case META_EXE: + return free_and_strdup_warn(&context->exe, s); + + case META_UNIT: + return free_and_strdup_warn(&context->unit, s); + + case META_PROC_AUXV: { + char *t = memdup_suffix0(s, size); + if (!t) + return log_oom(); + + context->auxv_size = size; + return free_and_replace(context->auxv, t); + } + + default: + assert_not_reached(); + } +} + +int coredump_context_parse_iovw(CoredumpContext *context) { + int r; + + assert(context); + + /* Parse the data in the iovec array iovw into separate fields. */ + + bool have[_META_MAX] = {}; + FOREACH_ARRAY(iovec, context->iovw.iovec, context->iovw.count) { + /* Note that these strings are NUL-terminated, because we made sure that a trailing NUL byte + * is in the buffer, though not included in the iov_len count. See coredump_receive() and + * coredump_context_parse_from_*(). */ + assert(((char*) iovec->iov_base)[iovec->iov_len] == 0); + + for (MetadataField i = 0; i < _META_MAX; i++) { + const char *s = metadata_field_to_string(i); + const char *p = memory_startswith(iovec->iov_base, iovec->iov_len, s); + if (!p) + continue; + + size_t size = iovec->iov_len - strlen(s); + if (i != META_PROC_AUXV && strlen(p) != size) + return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "%s= field contains NUL character.", s); + + if (have[i]) + return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "Message contains duplicated field: %s", s); + + have[i] = true; + + r = context_parse_one(context, i, /* from_argv= */ false, p, size); + if (r < 0) + return r; + + break; + } + } + + /* Make sure we received all the expected fields. We support being called by an *older* systemd-coredump + * from the outside, so we require only the basic set of fields that was being sent when the support for + * sending to containers over a socket was added in a108c43e36d3ceb6e34efe37c014fc2cda856000. */ + MetadataField i; + FOREACH_ARGUMENT(i, + META_ARGV_PID, + META_ARGV_UID, + META_ARGV_GID, + META_ARGV_SIGNAL, + META_ARGV_TIMESTAMP, + META_ARGV_RLIMIT, + META_ARGV_HOSTNAME, + META_COMM) + if (!have[i]) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Mandatory argument %s not received on socket.", + metadata_field_to_string(i)); + + return 0; +} + +int coredump_context_parse_from_argv(CoredumpContext *context, int argc, char **argv) { + int r; - assert(iovw); assert(context); /* We gather all metadata that were passed via argv[] into an array of iovecs that @@ -372,83 +550,11 @@ int gather_pid_metadata_from_argv( "Not enough arguments passed by the kernel (%i, expected between %i and %i).", argc, _META_ARGV_REQUIRED, _META_ARGV_MAX); - for (int i = 0; i < MIN(argc, _META_ARGV_MAX); i++) { - _cleanup_free_ char *buf = NULL; - const char *t = argv[i]; - - if (i == META_ARGV_TIMESTAMP) { - /* The journal fields contain the timestamp padded with six - * zeroes, so that the kernel-supplied 1s granularity timestamps - * becomes 1μs granularity, i.e. the granularity systemd usually - * operates in. */ - buf = strjoin(argv[i], "000000"); - if (!buf) - return log_oom(); - - t = buf; - } - - if (i == META_ARGV_PID) { - /* Store this so that we can check whether the core will be forwarded to a container - * even when the kernel doesn't provide a pidfd. Can be dropped once baseline is - * >= v6.16. */ - r = pidref_set_pidstr(&local_pidref, t); - if (r < 0) - return log_error_errno(r, "Failed to initialize pidref from pid %s: %m", t); - } - - if (i == META_ARGV_PIDFD) { - /* If the current kernel doesn't support the %F specifier (which resolves to a - * pidfd), but we included it in the core_pattern expression, we'll receive an empty - * string here. Deal with that gracefully. */ - if (isempty(t)) - continue; - - assert(!pidref_is_set(&context->pidref)); - assert(kernel_fd < 0); - - kernel_fd = parse_fd(t); - if (kernel_fd < 0) - return log_error_errno(kernel_fd, "Failed to parse pidfd \"%s\": %m", t); - - r = pidref_set_pidfd(&context->pidref, kernel_fd); - if (r < 0) - return log_error_errno(r, "Failed to initialize pidref from pidfd %d: %m", kernel_fd); - - context->got_pidfd = 1; - - /* If there are containers involved with different versions of the code they might - * not be using pidfds, so it would be wrong to set the metadata, skip it. */ - r = pidref_in_same_namespace(/* pid1 = */ NULL, &context->pidref, NAMESPACE_PID); - if (r < 0) - log_debug_errno(r, "Failed to check pidns of crashing process, ignoring: %m"); - if (r <= 0) - continue; - - /* We don't print the fd number in the journal as it's meaningless, but we still - * record that the parsing was done with a kernel-provided fd as it means it's safe - * from races, which is valuable information to provide in the journal record. */ - t = "1"; - } - - r = iovw_put_string_field(iovw, meta_field_names[i], t); + for (MetadataField i = 0; i < MIN(argc, _META_ARGV_MAX); i++) { + r = context_parse_one(context, i, /* from_argv= */ true, argv[i], SIZE_MAX); if (r < 0) return r; } - /* Cache some of the process metadata we collected so far and that we'll need to - * access soon. */ - r = context_parse_iovw(context, iovw); - if (r < 0) - return r; - - /* If the kernel didn't give us a PIDFD, then use the one derived from the - * PID immediately, given we have it. */ - if (!pidref_is_set(&context->pidref)) - context->pidref = TAKE_PIDREF(local_pidref); - - /* Close the kernel-provided FD as the last thing after everything else succeeded. */ - kernel_fd = safe_close(kernel_fd); - - return 0; + return coredump_context_parse_from_procfs(context); } diff --git a/src/coredump/coredump-context.h b/src/coredump/coredump-context.h index 31c236ebc4..32557348f9 100644 --- a/src/coredump/coredump-context.h +++ b/src/coredump/coredump-context.h @@ -2,9 +2,10 @@ #pragma once #include "coredump-forward.h" +#include "iovec-wrapper.h" #include "pidref.h" -typedef enum { +typedef enum MetadataField { /* We use these as array indexes for our process metadata cache. * * The first indices of the cache stores the same metadata as the ones passed by the kernel via @@ -39,37 +40,43 @@ typedef enum { META_EXE, META_UNIT, META_PROC_AUXV, - _META_MAX -} meta_argv_t; + _META_MAX, + _META_INVALID = -EINVAL, +} MetadataField; -extern const char * const meta_field_names[_META_MAX]; - -struct Context { - PidRef pidref; - uid_t uid; - gid_t gid; - unsigned dumpable; - int signo; - uint64_t rlimit; - bool is_pid1; - bool is_journald; - bool got_pidfd; +struct CoredumpContext { + PidRef pidref; /* META_ARGV_PID and META_ARGV_PIDFD */ + uid_t uid; /* META_ARGV_UID */ + gid_t gid; /* META_ARGV_GID */ + int signo; /* META_ARGV_SIGNAL */ + usec_t timestamp; /* META_ARGV_TIMESTAMP */ + uint64_t rlimit; /* META_ARGV_RLIMIT */ + char *hostname; /* META_ARGV_HOSTNAME */ + unsigned dumpable; /* META_ARGV_DUMPABLE */ + char *comm; /* META_COMM */ + char *exe; /* META_EXE */ + char *unit; /* META_UNIT */ + char *auxv; /* META_PROC_AUXV */ + size_t auxv_size; /* META_PROC_AUXV */ + bool got_pidfd; /* META_ARGV_PIDFD */ + bool forwarded; + int input_fd; int mount_tree_fd; - - /* These point into external memory, are not owned by this object */ - const char *meta[_META_MAX]; - size_t meta_size[_META_MAX]; + struct iovec_wrapper iovw; }; -#define CONTEXT_NULL \ - (Context) { \ +#define COREDUMP_CONTEXT_NULL \ + (CoredumpContext) { \ .pidref = PIDREF_NULL, \ .uid = UID_INVALID, \ .gid = GID_INVALID, \ .mount_tree_fd = -EBADF, \ + .input_fd = -EBADF, \ } -void context_done(Context *c); -int context_parse_iovw(Context *context, struct iovec_wrapper *iovw); -int gather_pid_metadata_from_argv(struct iovec_wrapper *iovw, Context *context, int argc, char **argv); -int gather_pid_metadata_from_procfs(struct iovec_wrapper *iovw, Context *context); +void coredump_context_done(CoredumpContext *context); +bool coredump_context_is_pid1(CoredumpContext *context); +bool coredump_context_is_journald(CoredumpContext *context); +int coredump_context_build_iovw(CoredumpContext *context); +int coredump_context_parse_iovw(CoredumpContext *context); +int coredump_context_parse_from_argv(CoredumpContext *context, int argc, char **argv); diff --git a/src/coredump/coredump-forward.h b/src/coredump/coredump-forward.h index b5a2d51a5e..0fa18a4a12 100644 --- a/src/coredump/coredump-forward.h +++ b/src/coredump/coredump-forward.h @@ -4,4 +4,4 @@ #include "basic-forward.h" typedef struct CoredumpConfig CoredumpConfig; -typedef struct Context Context; +typedef struct CoredumpContext CoredumpContext; diff --git a/src/coredump/coredump-kernel-helper.c b/src/coredump/coredump-kernel-helper.c index 9cac410b03..8b75382aba 100644 --- a/src/coredump/coredump-kernel-helper.c +++ b/src/coredump/coredump-kernel-helper.c @@ -1,7 +1,5 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include "sd-messages.h" - #include "coredump-config.h" #include "coredump-context.h" #include "coredump-kernel-helper.h" @@ -9,22 +7,28 @@ #include "coredump-submit.h" #include "coredump-util.h" #include "fd-util.h" +#include "format-util.h" #include "iovec-wrapper.h" #include "log.h" #include "namespace-util.h" #include "signal-util.h" int coredump_kernel_helper(int argc, char *argv[]) { - _cleanup_(iovw_free_freep) struct iovec_wrapper *iovw = NULL; - _cleanup_(context_done) Context context = CONTEXT_NULL; + _cleanup_(coredump_context_done) CoredumpContext context = COREDUMP_CONTEXT_NULL; int r; /* When we're invoked by the kernel, stdout/stderr are closed which is dangerous because the fds * could get reallocated. To avoid hard to debug issues, let's instead bind stdout/stderr to - * /dev/null. */ - r = rearrange_stdio(STDIN_FILENO, -EBADF, -EBADF); + * /dev/null. Also, move stdin to above stdio and then also bind stdin to /dev/null. */ + + r = fd_move_above_stdio(STDIN_FILENO); if (r < 0) - return log_error_errno(r, "Failed to connect stdout/stderr to /dev/null: %m"); + return log_error_errno(r, "Failed to move stdin above stdio: %m"); + context.input_fd = r; + + r = make_null_stdio(); + if (r < 0) + return log_error_errno(r, "Failed to connect stdin/stdout/stderr to /dev/null: %m"); /* Ignore all parse errors */ CoredumpConfig config = COREDUMP_CONFIG_NULL; @@ -32,28 +36,18 @@ int coredump_kernel_helper(int argc, char *argv[]) { log_debug("Processing coredump received from the kernel..."); - iovw = iovw_new(); - if (!iovw) - return log_oom(); - /* Collect all process metadata passed by the kernel through argv[] */ - r = gather_pid_metadata_from_argv(iovw, &context, argc - 1, argv + 1); + r = coredump_context_parse_from_argv(&context, argc - 1, argv + 1); if (r < 0) return r; - /* Collect the rest of the process metadata retrieved from the runtime */ - r = gather_pid_metadata_from_procfs(iovw, &context); - if (r < 0) - return r; - - if (!context.is_journald) + if (!coredump_context_is_journald(&context)) /* OK, now we know it's not the journal, hence we can make use of it now. */ log_set_target_and_open(LOG_TARGET_JOURNAL_OR_KMSG); /* Log minimal metadata now, so it is not lost if the system is about to shut down. */ - log_info("Process %s (%s) of user %s terminated abnormally with signal %s/%s, processing...", - context.meta[META_ARGV_PID], context.meta[META_COMM], - context.meta[META_ARGV_UID], context.meta[META_ARGV_SIGNAL], + log_info("Process "PID_FMT" (%s) of user "UID_FMT" terminated abnormally with signal %i/%s, processing...", + context.pidref.pid, context.comm, context.uid, context.signo, signal_to_string(context.signo)); r = pidref_in_same_namespace(/* pid1 = */ NULL, &context.pidref, NAMESPACE_PID); @@ -66,9 +60,7 @@ int coredump_kernel_helper(int argc, char *argv[]) { if (r >= 0) return 0; - r = acquire_pid_mount_tree_fd(&config, &context, &context.mount_tree_fd); - if (r < 0) - log_warning_errno(r, "Failed to access the mount tree of a container, ignoring: %m"); + (void) acquire_pid_mount_tree_fd(&config, &context); } /* If this is PID 1, disable coredump collection, we'll unlikely be able to process @@ -77,16 +69,17 @@ int coredump_kernel_helper(int argc, char *argv[]) { * FIXME: maybe we should disable coredumps generation from the beginning and * re-enable it only when we know it's either safe (i.e. we're not running OOM) or * it's not PID 1 ? */ - if (context.is_pid1) { + if (coredump_context_is_pid1(&context)) { log_notice("Due to PID 1 having crashed coredump collection will now be turned off."); disable_coredumps(); } - (void) iovw_put_string_field(iovw, "MESSAGE_ID=", SD_MESSAGE_COREDUMP_STR); - (void) iovw_put_string_field(iovw, "PRIORITY=", STRINGIFY(LOG_CRIT)); + if (coredump_context_is_journald(&context) || coredump_context_is_pid1(&context)) + return coredump_submit(&config, &context); - if (context.is_journald || context.is_pid1) - return coredump_submit(&config, &context, iovw, STDIN_FILENO); + r = coredump_context_build_iovw(&context); + if (r < 0) + return r; - return coredump_send(iovw, STDIN_FILENO, &context.pidref, context.mount_tree_fd); + return coredump_send(&context); } diff --git a/src/coredump/coredump-receive.c b/src/coredump/coredump-receive.c index be786be2f1..c1d2df4a7d 100644 --- a/src/coredump/coredump-receive.c +++ b/src/coredump/coredump-receive.c @@ -13,9 +13,7 @@ #include "socket-util.h" int coredump_receive(int fd) { - _cleanup_(iovw_done_free) struct iovec_wrapper iovw = {}; - _cleanup_(context_done) Context context = CONTEXT_NULL; - _cleanup_close_ int input_fd = -EBADF; + _cleanup_(coredump_context_done) CoredumpContext context = COREDUMP_CONTEXT_NULL; enum { STATE_PAYLOAD, STATE_INPUT_FD_DONE, @@ -90,8 +88,8 @@ int coredump_receive(int fd) { switch (state) { case STATE_PAYLOAD: - assert(input_fd < 0); - input_fd = *CMSG_TYPED_DATA(found, int); + assert(context.input_fd < 0); + context.input_fd = *CMSG_TYPED_DATA(found, int); state = STATE_INPUT_FD_DONE; continue; @@ -102,6 +100,7 @@ int coredump_receive(int fd) { if (r < 0) return log_error_errno(r, "Failed to initialize pidref: %m"); + context.got_pidfd = true; state = STATE_PID_FD_DONE; continue; @@ -130,37 +129,18 @@ int coredump_receive(int fd) { ((char*) iovec.iov_base)[n] = 0; iovec.iov_len = (size_t) n; - if (iovw_put(&iovw, iovec.iov_base, iovec.iov_len) < 0) + if (iovw_put(&context.iovw, iovec.iov_base, iovec.iov_len) < 0) return log_oom(); TAKE_STRUCT(iovec); } /* Make sure we got all data we really need */ - assert(input_fd >= 0); + assert(context.input_fd >= 0); - r = context_parse_iovw(&context, &iovw); + r = coredump_context_parse_iovw(&context); if (r < 0) return r; - /* Make sure we received all the expected fields. We support being called by an *older* - * systemd-coredump from the outside, so we require only the basic set of fields that - * was being sent when the support for sending to containers over a socket was added - * in a108c43e36d3ceb6e34efe37c014fc2cda856000. */ - meta_argv_t i; - FOREACH_ARGUMENT(i, - META_ARGV_PID, - META_ARGV_UID, - META_ARGV_GID, - META_ARGV_SIGNAL, - META_ARGV_TIMESTAMP, - META_ARGV_RLIMIT, - META_ARGV_HOSTNAME, - META_COMM) - if (!context.meta[i]) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Mandatory argument %s not received on socket, aborting.", - meta_field_names[i]); - - return coredump_submit(&config, &context, &iovw, input_fd); + return coredump_submit(&config, &context); } diff --git a/src/coredump/coredump-send.c b/src/coredump/coredump-send.c index ef2fc02ceb..f178a3bf8c 100644 --- a/src/coredump/coredump-send.c +++ b/src/coredump/coredump-send.c @@ -2,8 +2,6 @@ #include -#include "sd-messages.h" - #include "coredump-context.h" #include "coredump-send.h" #include "coredump-util.h" @@ -19,12 +17,12 @@ #include "process-util.h" #include "socket-util.h" -int coredump_send(const struct iovec_wrapper *iovw, int input_fd, PidRef *pidref, int mount_tree_fd) { +int coredump_send(CoredumpContext *context) { _cleanup_close_ int fd = -EBADF; int r; - assert(iovw); - assert(input_fd >= 0); + assert(context); + assert(context->input_fd >= 0); fd = socket(AF_UNIX, SOCK_SEQPACKET|SOCK_CLOEXEC, 0); if (fd < 0) @@ -34,9 +32,9 @@ int coredump_send(const struct iovec_wrapper *iovw, int input_fd, PidRef *pidref if (r < 0) return log_error_errno(r, "Failed to connect to coredump service: %m"); - for (size_t i = 0; i < iovw->count; i++) { + FOREACH_ARRAY(iovec, context->iovw.iovec, context->iovw.count) { struct msghdr mh = { - .msg_iov = iovw->iovec + i, + .msg_iov = iovec, .msg_iovlen = 1, }; struct iovec copy[2]; @@ -57,7 +55,7 @@ int coredump_send(const struct iovec_wrapper *iovw, int input_fd, PidRef *pidref * iovecs, where the first is a (truncated) copy of * what we want to send, and the second one contains * the trailing dots. */ - copy[0] = iovw->iovec[i]; + copy[0] = *iovec; copy[1] = IOVEC_MAKE(((const char[]){'.', '.', '.'}), 3); mh.msg_iov = copy; @@ -73,30 +71,30 @@ int coredump_send(const struct iovec_wrapper *iovw, int input_fd, PidRef *pidref } /* First sentinel: the coredump fd */ - r = send_one_fd(fd, input_fd, 0); + r = send_one_fd(fd, context->input_fd, 0); if (r < 0) return log_error_errno(r, "Failed to send coredump fd: %m"); /* The optional second sentinel: the pidfd */ - if (!pidref_is_set(pidref) || pidref->fd < 0) /* If we have no pidfd, stop now */ + if (!pidref_is_set(&context->pidref) || context->pidref.fd < 0) /* If we have no pidfd, stop now */ return 0; - r = send_one_fd(fd, pidref->fd, 0); + r = send_one_fd(fd, context->pidref.fd, 0); if (r < 0) return log_error_errno(r, "Failed to send pidfd: %m"); /* The optional third sentinel: the mount tree fd */ - if (mount_tree_fd < 0) /* If we have no mount tree, stop now */ + if (context->mount_tree_fd < 0) /* If we have no mount tree, stop now */ return 0; - r = send_one_fd(fd, mount_tree_fd, 0); + r = send_one_fd(fd, context->mount_tree_fd, 0); if (r < 0) return log_error_errno(r, "Failed to send mount tree fd: %m"); return 0; } -static int can_forward_coredump(Context *context, const PidRef *pid) { +static int can_forward_coredump(CoredumpContext *context, const PidRef *pid) { _cleanup_free_ char *cgroup = NULL, *path = NULL, *unit = NULL; int r; @@ -192,7 +190,7 @@ static int receive_ucred(int transport_fd, struct ucred *ret_ucred) { return 0; } -int coredump_send_to_container(Context *context) { +int coredump_send_to_container(CoredumpContext *context) { _cleanup_close_ int pidnsfd = -EBADF, mntnsfd = -EBADF, netnsfd = -EBADF, usernsfd = -EBADF, rootfd = -EBADF; _cleanup_close_pair_ int pair[2] = EBADF_PAIR; pid_t child; @@ -249,63 +247,26 @@ int coredump_send_to_container(Context *context) { _exit(EXIT_FAILURE); } - _cleanup_(iovw_free_freep) struct iovec_wrapper *iovw = iovw_new(); - if (!iovw) { - log_oom(); - _exit(EXIT_FAILURE); - } - - (void) iovw_put_string_field(iovw, "MESSAGE_ID=", SD_MESSAGE_COREDUMP_STR); - (void) iovw_put_string_field(iovw, "PRIORITY=", STRINGIFY(LOG_CRIT)); - (void) iovw_put_string_field(iovw, "COREDUMP_FORWARDED=", "1"); - - for (int i = 0; i < _META_ARGV_MAX; i++) { - char buf[DECIMAL_STR_MAX(pid_t)]; - const char *t = context->meta[i]; - - /* Patch some of the fields with the translated ucred data */ - switch (i) { - - case META_ARGV_PID: - xsprintf(buf, PID_FMT, ucred.pid); - t = buf; - break; - - case META_ARGV_UID: - xsprintf(buf, UID_FMT, ucred.uid); - t = buf; - break; - - case META_ARGV_GID: - xsprintf(buf, GID_FMT, ucred.gid); - t = buf; - break; - - default: - ; - } - - r = iovw_put_string_field(iovw, meta_field_names[i], t); - if (r < 0) { - log_debug_errno(r, "Failed to construct iovec: %m"); - _exit(EXIT_FAILURE); - } - } - - _cleanup_(context_done) Context child_context = CONTEXT_NULL; - r = context_parse_iovw(&child_context, iovw); + PidRef pidref; + r = pidref_set_pid(&pidref, ucred.pid); if (r < 0) { - log_debug_errno(r, "Failed to save context: %m"); + log_error_errno(r, "Failed to set pid to pidref: %m"); _exit(EXIT_FAILURE); } - r = gather_pid_metadata_from_procfs(iovw, &child_context); - if (r < 0) { - log_debug_errno(r, "Failed to gather metadata from procfs: %m"); - _exit(EXIT_FAILURE); - } + pidref_done(&context->pidref); + context->pidref = TAKE_PIDREF(pidref); - r = coredump_send(iovw, STDIN_FILENO, &context->pidref, /* mount_tree_fd= */ -EBADF); + context->uid = ucred.uid; + context->gid = ucred.gid; + + r = coredump_context_build_iovw(context); + if (r < 0) + _exit(EXIT_FAILURE); + + (void) iovw_put_string_field(&context->iovw, "COREDUMP_FORWARDED=", "1"); + + r = coredump_send(context); if (r < 0) { log_debug_errno(r, "Failed to send iovec to coredump socket: %m"); _exit(EXIT_FAILURE); diff --git a/src/coredump/coredump-send.h b/src/coredump/coredump-send.h index 97ddec26c8..a68be8e83a 100644 --- a/src/coredump/coredump-send.h +++ b/src/coredump/coredump-send.h @@ -3,5 +3,5 @@ #include "coredump-forward.h" -int coredump_send(const struct iovec_wrapper *iovw, int input_fd, PidRef *pidref, int mount_tree_fd); -int coredump_send_to_container(Context *context); +int coredump_send(CoredumpContext *context); +int coredump_send_to_container(CoredumpContext *context); diff --git a/src/coredump/coredump-submit.c b/src/coredump/coredump-submit.c index 56093e2d7b..ba11e9001a 100644 --- a/src/coredump/coredump-submit.c +++ b/src/coredump/coredump-submit.c @@ -40,6 +40,7 @@ #include "socket-util.h" #include "stat-util.h" #include "string-util.h" +#include "time-util.h" #include "tmpfile-util.h" #include "uid-classification.h" #include "user-util.h" @@ -50,52 +51,34 @@ #define MOUNT_TREE_ROOT "/run/systemd/mount-rootfs" -#define filename_escape(s) xescape((s), "./ ") - static const char* coredump_tmpfile_name(const char *s) { return s ?: "(unnamed temporary file)"; } -static int make_filename(const Context *context, char **ret) { - _cleanup_free_ char *c = NULL, *u = NULL, *p = NULL, *t = NULL; - sd_id128_t boot = {}; +static int make_filename(const CoredumpContext *context, char **ret) { + _cleanup_free_ char *c = NULL; + sd_id128_t boot; int r; assert(context); - c = filename_escape(context->meta[META_COMM]); + c = xescape(context->comm, "./ "); if (!c) return -ENOMEM; - u = filename_escape(context->meta[META_ARGV_UID]); - if (!u) - return -ENOMEM; - r = sd_id128_get_boot(&boot); if (r < 0) return r; - p = filename_escape(context->meta[META_ARGV_PID]); - if (!p) - return -ENOMEM; - - t = filename_escape(context->meta[META_ARGV_TIMESTAMP]); - if (!t) - return -ENOMEM; - if (asprintf(ret, - "/var/lib/systemd/coredump/core.%s.%s." SD_ID128_FORMAT_STR ".%s.%s", - c, - u, - SD_ID128_FORMAT_VAL(boot), - p, - t) < 0) + "/var/lib/systemd/coredump/core.%s."UID_FMT"." SD_ID128_FORMAT_STR "."PID_FMT"."USEC_FMT, + c, context->uid, SD_ID128_FORMAT_VAL(boot), context->pidref.pid, context->timestamp) < 0) return -ENOMEM; return 0; } -static int grant_user_access(int core_fd, const Context *context) { +static int grant_user_access(int core_fd, const CoredumpContext *context) { int at_secure = -1; uid_t uid = UID_INVALID, euid = UID_INVALID; uid_t gid = GID_INVALID, egid = GID_INVALID; @@ -104,7 +87,7 @@ static int grant_user_access(int core_fd, const Context *context) { assert(core_fd >= 0); assert(context); - if (!context->meta[META_PROC_AUXV]) + if (!context->auxv) return log_warning_errno(SYNTHETIC_ERRNO(ENODATA), "No auxv data, not adjusting permissions."); uint8_t elf[EI_NIDENT]; @@ -131,8 +114,8 @@ static int grant_user_access(int core_fd, const Context *context) { r = parse_auxv(LOG_WARNING, /* elf_class= */ elf[EI_CLASS], - context->meta[META_PROC_AUXV], - context->meta_size[META_PROC_AUXV], + context->auxv, + context->auxv_size, &at_secure, &uid, &euid, &gid, &egid); if (r < 0) return r; @@ -174,34 +157,50 @@ static int fix_acl(int fd, uid_t uid, bool allow_user) { return 0; } -static int fix_xattr(int fd, const Context *context) { - static const char * const xattrs[_META_MAX] = { - [META_ARGV_PID] = "user.coredump.pid", - [META_ARGV_UID] = "user.coredump.uid", - [META_ARGV_GID] = "user.coredump.gid", - [META_ARGV_SIGNAL] = "user.coredump.signal", - [META_ARGV_TIMESTAMP] = "user.coredump.timestamp", - [META_ARGV_RLIMIT] = "user.coredump.rlimit", - [META_ARGV_HOSTNAME] = "user.coredump.hostname", - [META_COMM] = "user.coredump.comm", - [META_EXE] = "user.coredump.exe", - }; +static int fix_xattr_one(int fd, const char *xattr, const char *val) { + assert(fd >= 0); + assert(xattr); - int r = 0; + if (isempty(val)) + return 0; + + return RET_NERRNO(fsetxattr(fd, xattr, val, strlen(val), XATTR_CREATE)); +} + +_printf_(3, 4) +static int fix_xattr_format(int fd, const char *xattr, const char *format, ...) { + _cleanup_free_ char *value = NULL; + va_list ap; + int r; + + assert(format); + + va_start(ap, format); + r = vasprintf(&value, format, ap); + va_end(ap); + if (r < 0) + return -ENOMEM; + + return fix_xattr_one(fd, xattr, value); +} + +static int fix_xattr(int fd, const CoredumpContext *context) { + int r; assert(fd >= 0); + assert(context); /* Attach some metadata to coredumps via extended attributes. Just because we can. */ - for (unsigned i = 0; i < _META_MAX; i++) { - int k; - - if (isempty(context->meta[i]) || !xattrs[i]) - continue; - - k = RET_NERRNO(fsetxattr(fd, xattrs[i], context->meta[i], strlen(context->meta[i]), XATTR_CREATE)); - RET_GATHER(r, k); - } + r = fix_xattr_format(fd, "user.coredump.pid", PID_FMT, context->pidref.pid); + RET_GATHER(r, fix_xattr_format(fd, "user.coredump.uid", UID_FMT, context->uid)); + RET_GATHER(r, fix_xattr_format(fd, "user.coredump.gid", GID_FMT, context->gid)); + RET_GATHER(r, fix_xattr_format(fd, "user.coredump.signal", "%i", context->signo)); + RET_GATHER(r, fix_xattr_format(fd, "user.coredump.timestamp", USEC_FMT, context->timestamp)); + RET_GATHER(r, fix_xattr_format(fd, "user.coredump.rlimit", "%"PRIu64, context->rlimit)); + RET_GATHER(r, fix_xattr_one(fd, "user.coredump.hostname", context->hostname)); + RET_GATHER(r, fix_xattr_one(fd, "user.coredump.comm", context->comm)); + RET_GATHER(r, fix_xattr_one(fd, "user.coredump.exe", context->exe)); return r; } @@ -210,7 +209,7 @@ static int fix_permissions_and_link( int fd, const char *filename, const char *target, - const Context *context, + const CoredumpContext *context, bool allow_user) { int r; @@ -233,8 +232,7 @@ static int fix_permissions_and_link( static int save_external_coredump( const CoredumpConfig *config, - const Context *context, - int input_fd, + const CoredumpContext *context, char **ret_filename, int *ret_node_fd, int *ret_data_fd, @@ -252,6 +250,7 @@ static int save_external_coredump( assert(config); assert(context); + assert(context->input_fd >= 0); assert(ret_filename); assert(ret_node_fd); assert(ret_data_fd); @@ -265,8 +264,8 @@ static int save_external_coredump( * (the kernel uses ELF_EXEC_PAGESIZE which is not easily accessible, but * is usually the same as PAGE_SIZE. */ return log_info_errno(SYNTHETIC_ERRNO(EBADSLT), - "Resource limits disable core dumping for process %s (%s).", - context->meta[META_ARGV_PID], context->meta[META_COMM]); + "Resource limits disable core dumping for process "PID_FMT" (%s).", + context->pidref.pid, context->comm); process_limit = MAX(config->process_size_max, coredump_storage_size_max(config)); if (process_limit == 0) @@ -344,10 +343,10 @@ static int save_external_coredump( log_debug("Limiting core file size to %" PRIu64 " bytes due to cgroup and/or filesystem limits.", max_size); } - r = copy_bytes(input_fd, fd, max_size, 0); + r = copy_bytes(context->input_fd, fd, max_size, 0); if (r < 0) - return log_error_errno(r, "Cannot store coredump of %s (%s): %m", - context->meta[META_ARGV_PID], context->meta[META_COMM]); + return log_error_errno(r, "Cannot store coredump of "PID_FMT" (%s): %m", + context->pidref.pid, context->comm); truncated = r == 1; bool allow_user = grant_user_access(fd, context) > 0; @@ -383,7 +382,7 @@ static int save_external_coredump( tmp = unlink_and_free(tmp); fd = safe_close(fd); - r = compress_stream(input_fd, fd_compressed, max_size, &partial_uncompressed_size); + r = compress_stream(context->input_fd, fd_compressed, max_size, &partial_uncompressed_size); if (r < 0) return log_error_errno(r, "Failed to compress %s: %m", coredump_tmpfile_name(tmp_compressed)); uncompressed_size += partial_uncompressed_size; @@ -437,7 +436,7 @@ static int save_external_coredump( static int maybe_remove_external_coredump( const CoredumpConfig *config, - const Context *context, + CoredumpContext *context, const char *filename, uint64_t size) { @@ -447,7 +446,8 @@ static int maybe_remove_external_coredump( /* Returns true if might remove, false if will not remove, < 0 on error. */ /* Always keep around in case of journald/pid1, since we cannot rely on the journal to accept them. */ - if (config->storage != COREDUMP_STORAGE_NONE && (context->is_pid1 || context->is_journald)) + if (config->storage != COREDUMP_STORAGE_NONE && + (coredump_context_is_pid1(context) || coredump_context_is_journald(context))) return false; if (config->storage == COREDUMP_STORAGE_EXTERNAL && @@ -463,25 +463,21 @@ static int maybe_remove_external_coredump( return true; } -int acquire_pid_mount_tree_fd(const CoredumpConfig *config, const Context *context, int *ret_fd) { - /* Don't bother preparing environment if we can't pass it to libdwfl. */ -#if !HAVE_DWFL_SET_SYSROOT - *ret_fd = -EOPNOTSUPP; - log_debug("dwfl_set_sysroot() is not supported."); -#else +int acquire_pid_mount_tree_fd(const CoredumpConfig *config, CoredumpContext *context) { +#if HAVE_DWFL_SET_SYSROOT _cleanup_close_ int mntns_fd = -EBADF, root_fd = -EBADF, fd = -EBADF; _cleanup_close_pair_ int pair[2] = EBADF_PAIR; int r; assert(config); assert(context); - assert(ret_fd); - if (!config->enter_namespace) { - *ret_fd = -EHOSTDOWN; - log_debug("EnterNamespace=no so we won't use mount tree of the crashed process for generating backtrace."); + if (context->mount_tree_fd >= 0) return 0; - } + + if (!config->enter_namespace) + return log_debug_errno(SYNTHETIC_ERRNO(EHOSTDOWN), + "EnterNamespace=no so we won't use mount tree of the crashed process for generating backtrace."); if (socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, pair) < 0) return log_error_errno(errno, "Failed to create socket pair: %m"); @@ -533,9 +529,12 @@ int acquire_pid_mount_tree_fd(const CoredumpConfig *config, const Context *conte if (fd < 0) return log_error_errno(fd, "Failed to receive mount tree: %m"); - *ret_fd = TAKE_FD(fd); -#endif + context->mount_tree_fd = TAKE_FD(fd); return 0; +#else + /* Don't bother preparing environment if we can't pass it to libdwfl. */ + return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "dwfl_set_sysroot() is not supported."); +#endif } static int attach_mount_tree(int mount_tree_fd) { @@ -569,7 +568,7 @@ static int attach_mount_tree(int mount_tree_fd) { return 0; } -static int change_uid_gid(const Context *context) { +static int change_uid_gid(const CoredumpContext *context) { int r; assert(context); @@ -622,12 +621,7 @@ static int allocate_journal_field(int fd, size_t size, char **ret, size_t *ret_s return 0; } -int coredump_submit( - const CoredumpConfig *config, - const Context *context, - struct iovec_wrapper *iovw, - int input_fd) { - +int coredump_submit(const CoredumpConfig *config, CoredumpContext *context) { _cleanup_(sd_json_variant_unrefp) sd_json_variant *json_metadata = NULL; _cleanup_close_ int coredump_fd = -EBADF, coredump_node_fd = -EBADF; _cleanup_free_ char *filename = NULL, *coredump_data = NULL, *stacktrace = NULL; @@ -639,15 +633,13 @@ int coredump_submit( assert(config); assert(context); - assert(iovw); - assert(input_fd >= 0); /* Vacuum before we write anything again */ (void) coredump_vacuum(-1, config->keep_free, config->max_use); /* Always stream the coredump to disk, if that's possible */ written = save_external_coredump( - config, context, input_fd, + config, context, &filename, &coredump_node_fd, &coredump_fd, &coredump_size, &coredump_compressed_size, &truncated) >= 0; if (written) { @@ -662,11 +654,13 @@ int coredump_submit( coredump_node_fd >= 0 ? coredump_compressed_size : coredump_size); if (r < 0) return r; - if (r == 0) - (void) iovw_put_string_field(iovw, "COREDUMP_FILENAME=", filename); - else if (config->storage == COREDUMP_STORAGE_EXTERNAL) - log_info("The core will not be stored: size %"PRIu64" is greater than %"PRIu64" (the configured maximum)", - coredump_node_fd >= 0 ? coredump_compressed_size : coredump_size, config->external_size_max); + if (r > 0) { + filename = mfree(filename); + + if (config->storage == COREDUMP_STORAGE_EXTERNAL) + log_info("The core will not be stored: size %"PRIu64" is greater than %"PRIu64" (the configured maximum)", + coredump_node_fd >= 0 ? coredump_compressed_size : coredump_size, config->external_size_max); + } /* Vacuum again, but exclude the coredump we just created */ (void) coredump_vacuum(coredump_node_fd >= 0 ? coredump_node_fd : coredump_fd, config->keep_free, config->max_use); @@ -690,10 +684,10 @@ int coredump_submit( "than %"PRIu64" (the configured maximum)", coredump_size, config->process_size_max); else if (coredump_fd >= 0) { - bool skip = startswith(context->meta[META_COMM], "systemd-coredum"); /* COMM is 16 bytes usually */ + bool skip = startswith(context->comm, "systemd-coredum"); /* COMM is 16 bytes usually */ (void) parse_elf_object(coredump_fd, - context->meta[META_EXE], + context->exe, root, /* fork_disable_dump= */ skip, /* avoid loops */ &stacktrace, @@ -701,16 +695,17 @@ int coredump_submit( } } + r = coredump_context_build_iovw(context); + if (r < 0) + return r; + _cleanup_free_ char *core_message = NULL; - core_message = strjoin( - "Process ", context->meta[META_ARGV_PID], - " (", context->meta[META_COMM], - ") of user ", context->meta[META_ARGV_UID], - written ? " dumped core." : " terminated abnormally without generating a coredump."); - if (!core_message) + if (asprintf(&core_message, "Process "PID_FMT" (%s) of user "UID_FMT" %s", + context->pidref.pid, context->comm, context->uid, + written ? "dumped core." : "terminated abnormally without generating a coredump.") < 0) return log_oom(); - if (context->is_journald && filename) + if (coredump_context_is_journald(context) && filename) if (!strextend(&core_message, "\nCoredump diverted to ", filename)) return log_oom(); @@ -718,15 +713,17 @@ int coredump_submit( if (!strextend(&core_message, "\n\n", stacktrace)) return log_oom(); - if (context->is_journald) + if (coredump_context_is_journald(context)) /* We might not be able to log to the journal, so let's always print the message to another * log target. The target was set previously to something safe. */ log_dispatch(LOG_ERR, 0, core_message); - (void) iovw_put_string_field(iovw, "MESSAGE=", core_message); + (void) iovw_put_string_field(&context->iovw, "MESSAGE=", core_message); + if (filename) + (void) iovw_put_string_field(&context->iovw, "COREDUMP_FILENAME=", filename); if (truncated) - (void) iovw_put_string_field(iovw, "COREDUMP_TRUNCATED=", "1"); + (void) iovw_put_string_field(&context->iovw, "COREDUMP_TRUNCATED=", "1"); /* If we managed to parse any ELF metadata (build-id, ELF package meta), * attach it as journal metadata. */ @@ -737,26 +734,26 @@ int coredump_submit( if (r < 0) return log_error_errno(r, "Failed to format JSON package metadata: %m"); - (void) iovw_put_string_field(iovw, "COREDUMP_PACKAGE_JSON=", formatted_json); + (void) iovw_put_string_field(&context->iovw, "COREDUMP_PACKAGE_JSON=", formatted_json); } /* In the unlikely scenario that context->meta[META_EXE] is not available, * let's avoid guessing the module name and skip the loop. */ - if (context->meta[META_EXE]) + if (context->exe) JSON_VARIANT_OBJECT_FOREACH(module_name, module_json, json_metadata) { sd_json_variant *t; /* We only add structured fields for the 'main' ELF module, and only if we can identify it. */ - if (!path_equal_filename(module_name, context->meta[META_EXE])) + if (!path_equal_filename(module_name, context->exe)) continue; t = sd_json_variant_by_key(module_json, "name"); if (t) - (void) iovw_put_string_field(iovw, "COREDUMP_PACKAGE_NAME=", sd_json_variant_string(t)); + (void) iovw_put_string_field(&context->iovw, "COREDUMP_PACKAGE_NAME=", sd_json_variant_string(t)); t = sd_json_variant_by_key(module_json, "version"); if (t) - (void) iovw_put_string_field(iovw, "COREDUMP_PACKAGE_VERSION=", sd_json_variant_string(t)); + (void) iovw_put_string_field(&context->iovw, "COREDUMP_PACKAGE_VERSION=", sd_json_variant_string(t)); } /* Optionally store the entire coredump in the journal */ @@ -768,7 +765,7 @@ int coredump_submit( r = allocate_journal_field(coredump_fd, (size_t) coredump_size, &coredump_data, &sz); if (r >= 0) { - if (iovw_put(iovw, coredump_data, sz) >= 0) + if (iovw_put(&context->iovw, coredump_data, sz) >= 0) TAKE_PTR(coredump_data); } else log_warning_errno(r, "Failed to attach the core to the journal entry: %m"); @@ -781,15 +778,15 @@ int coredump_submit( * coredump to the journal, so we put the journal socket in nonblocking mode before trying to write * the coredump to the socket. */ - if (context->is_journald) { + if (coredump_context_is_journald(context)) { r = journal_fd_nonblock(true); if (r < 0) return log_error_errno(r, "Failed to make journal socket non-blocking: %m"); } - r = sd_journal_sendv(iovw->iovec, iovw->count); + r = sd_journal_sendv(context->iovw.iovec, context->iovw.count); - if (context->is_journald) { + if (coredump_context_is_journald(context)) { int k; k = journal_fd_nonblock(false); @@ -797,7 +794,7 @@ int coredump_submit( return log_error_errno(k, "Failed to make journal socket blocking: %m"); } - if (r == -EAGAIN && context->is_journald) + if (r == -EAGAIN && coredump_context_is_journald(context)) log_warning_errno(r, "Failed to log journal coredump, ignoring: %m"); else if (r < 0) return log_error_errno(r, "Failed to log coredump: %m"); diff --git a/src/coredump/coredump-submit.h b/src/coredump/coredump-submit.h index 460b2d14b1..b36790f916 100644 --- a/src/coredump/coredump-submit.h +++ b/src/coredump/coredump-submit.h @@ -3,12 +3,5 @@ #include "coredump-forward.h" -int acquire_pid_mount_tree_fd( - const CoredumpConfig *config, - const Context *context, - int *ret_fd); -int coredump_submit( - const CoredumpConfig *config, - const Context *context, - struct iovec_wrapper *iovw, - int input_fd); +int acquire_pid_mount_tree_fd(const CoredumpConfig *config, CoredumpContext *context); +int coredump_submit(const CoredumpConfig *config, CoredumpContext *context); From 91840f324e581be1aac77c457a19d0e77855a4ba Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 17 Oct 2025 17:25:23 +0900 Subject: [PATCH 5/7] coredump-context: cache if the crashed process in the same PID namespace --- src/coredump/coredump-context.c | 15 +++++++++++++++ src/coredump/coredump-context.h | 1 + src/coredump/coredump-kernel-helper.c | 6 +----- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/coredump/coredump-context.c b/src/coredump/coredump-context.c index db5dd2e141..3b13289e42 100644 --- a/src/coredump/coredump-context.c +++ b/src/coredump/coredump-context.c @@ -315,6 +315,19 @@ int coredump_context_build_iovw(CoredumpContext *context) { return 0; } +static void coredump_context_check_pidns(CoredumpContext *context) { + int r; + + assert(context); + assert(pidref_is_set(&context->pidref)); + + r = pidref_in_same_namespace(/* pid1 = */ NULL, &context->pidref, NAMESPACE_PID); + if (r < 0) + log_debug_errno(r, "Failed to check pidns of crashing process, ignoring: %m"); + + context->same_pidns = r != 0; +} + static int coredump_context_parse_from_procfs(CoredumpContext *context) { int r; @@ -531,6 +544,7 @@ int coredump_context_parse_iovw(CoredumpContext *context) { "Mandatory argument %s not received on socket.", metadata_field_to_string(i)); + coredump_context_check_pidns(context); return 0; } @@ -556,5 +570,6 @@ int coredump_context_parse_from_argv(CoredumpContext *context, int argc, char ** return r; } + coredump_context_check_pidns(context); return coredump_context_parse_from_procfs(context); } diff --git a/src/coredump/coredump-context.h b/src/coredump/coredump-context.h index 32557348f9..f2d44c141c 100644 --- a/src/coredump/coredump-context.h +++ b/src/coredump/coredump-context.h @@ -59,6 +59,7 @@ struct CoredumpContext { char *auxv; /* META_PROC_AUXV */ size_t auxv_size; /* META_PROC_AUXV */ bool got_pidfd; /* META_ARGV_PIDFD */ + bool same_pidns; bool forwarded; int input_fd; int mount_tree_fd; diff --git a/src/coredump/coredump-kernel-helper.c b/src/coredump/coredump-kernel-helper.c index 8b75382aba..be3b756a23 100644 --- a/src/coredump/coredump-kernel-helper.c +++ b/src/coredump/coredump-kernel-helper.c @@ -10,7 +10,6 @@ #include "format-util.h" #include "iovec-wrapper.h" #include "log.h" -#include "namespace-util.h" #include "signal-util.h" int coredump_kernel_helper(int argc, char *argv[]) { @@ -50,10 +49,7 @@ int coredump_kernel_helper(int argc, char *argv[]) { context.pidref.pid, context.comm, context.uid, context.signo, signal_to_string(context.signo)); - r = pidref_in_same_namespace(/* pid1 = */ NULL, &context.pidref, NAMESPACE_PID); - if (r < 0) - log_debug_errno(r, "Failed to check pidns of crashing process, ignoring: %m"); - if (r == 0) { + if (!context.same_pidns) { /* If this fails, fallback to the old behavior so that * there is still some record of the crash. */ r = coredump_send_to_container(&context); From ab999c9280750bd3ffa2339811299655cdbb892d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 17 Oct 2025 17:49:41 +0900 Subject: [PATCH 6/7] coredump: enter mount namespace even when the crashed process is in the same PID namespace Otherwise, we may not get stacktrace of the crashed process when it is in running a mount namespace, especially when it is in a portable service or service that uses RootImage=/RootDirectory=. --- src/coredump/coredump-kernel-helper.c | 2 -- src/coredump/coredump-submit.c | 19 +++++++++++++------ src/coredump/coredump-submit.h | 1 - 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/coredump/coredump-kernel-helper.c b/src/coredump/coredump-kernel-helper.c index be3b756a23..bdadba10b6 100644 --- a/src/coredump/coredump-kernel-helper.c +++ b/src/coredump/coredump-kernel-helper.c @@ -55,8 +55,6 @@ int coredump_kernel_helper(int argc, char *argv[]) { r = coredump_send_to_container(&context); if (r >= 0) return 0; - - (void) acquire_pid_mount_tree_fd(&config, &context); } /* If this is PID 1, disable coredump collection, we'll unlikely be able to process diff --git a/src/coredump/coredump-submit.c b/src/coredump/coredump-submit.c index ba11e9001a..78d46c28c9 100644 --- a/src/coredump/coredump-submit.c +++ b/src/coredump/coredump-submit.c @@ -463,7 +463,7 @@ static int maybe_remove_external_coredump( return true; } -int acquire_pid_mount_tree_fd(const CoredumpConfig *config, CoredumpContext *context) { +static int acquire_pid_mount_tree_fd(const CoredumpConfig *config, CoredumpContext *context) { #if HAVE_DWFL_SET_SYSROOT _cleanup_close_ int mntns_fd = -EBADF, root_fd = -EBADF, fd = -EBADF; _cleanup_close_pair_ int pair[2] = EBADF_PAIR; @@ -537,10 +537,17 @@ int acquire_pid_mount_tree_fd(const CoredumpConfig *config, CoredumpContext *con #endif } -static int attach_mount_tree(int mount_tree_fd) { +static int attach_mount_tree(const CoredumpConfig *config, CoredumpContext *context) { int r; - assert(mount_tree_fd >= 0); + assert(config); + assert(context); + + r = acquire_pid_mount_tree_fd(config, context); + if (r < 0) + return r; + + assert(context->mount_tree_fd >= 0); r = detach_mount_namespace(); if (r < 0) @@ -550,7 +557,7 @@ static int attach_mount_tree(int mount_tree_fd) { if (r < 0) return log_warning_errno(r, "Failed to create directory: %m"); - r = mount_setattr(mount_tree_fd, "", AT_EMPTY_PATH, + r = mount_setattr(context->mount_tree_fd, "", AT_EMPTY_PATH, &(struct mount_attr) { /* MOUNT_ATTR_NOSYMFOLLOW is left out on purpose to allow libdwfl to resolve symlinks. * libdwfl will use openat2() with RESOLVE_IN_ROOT so there is no risk of symlink escape. @@ -561,7 +568,7 @@ static int attach_mount_tree(int mount_tree_fd) { if (r < 0) return log_warning_errno(errno, "Failed to change properties of mount tree: %m"); - r = move_mount(mount_tree_fd, "", -EBADF, MOUNT_TREE_ROOT, MOVE_MOUNT_F_EMPTY_PATH); + r = move_mount(context->mount_tree_fd, "", -EBADF, MOUNT_TREE_ROOT, MOVE_MOUNT_F_EMPTY_PATH); if (r < 0) return log_warning_errno(errno, "Failed to attach mount tree: %m"); @@ -666,7 +673,7 @@ int coredump_submit(const CoredumpConfig *config, CoredumpContext *context) { (void) coredump_vacuum(coredump_node_fd >= 0 ? coredump_node_fd : coredump_fd, config->keep_free, config->max_use); } - if (context->mount_tree_fd >= 0 && attach_mount_tree(context->mount_tree_fd) >= 0) + if (attach_mount_tree(config, context) >= 0) root = MOUNT_TREE_ROOT; /* Now, let's drop privileges to become the user who owns the segfaulted process and allocate the diff --git a/src/coredump/coredump-submit.h b/src/coredump/coredump-submit.h index b36790f916..89b07cec9a 100644 --- a/src/coredump/coredump-submit.h +++ b/src/coredump/coredump-submit.h @@ -3,5 +3,4 @@ #include "coredump-forward.h" -int acquire_pid_mount_tree_fd(const CoredumpConfig *config, CoredumpContext *context); int coredump_submit(const CoredumpConfig *config, CoredumpContext *context); From de438df275c4a175509ce5ee85d0e62a7bda2db2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 17 Oct 2025 18:19:38 +0900 Subject: [PATCH 7/7] coredump-send: do not forward when the service manager in container crashed In that case, even if we forward coredump to the container namespace, the socket unit will never triggered. --- src/coredump/coredump-kernel-helper.c | 9 +-- src/coredump/coredump-send.c | 90 +++++++++++++++++++-------- 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/src/coredump/coredump-kernel-helper.c b/src/coredump/coredump-kernel-helper.c index bdadba10b6..b97a96706b 100644 --- a/src/coredump/coredump-kernel-helper.c +++ b/src/coredump/coredump-kernel-helper.c @@ -49,13 +49,8 @@ int coredump_kernel_helper(int argc, char *argv[]) { context.pidref.pid, context.comm, context.uid, context.signo, signal_to_string(context.signo)); - if (!context.same_pidns) { - /* If this fails, fallback to the old behavior so that - * there is still some record of the crash. */ - r = coredump_send_to_container(&context); - if (r >= 0) - return 0; - } + if (coredump_send_to_container(&context) > 0) + return 0; /* If this is PID 1, disable coredump collection, we'll unlikely be able to process * it later on. diff --git a/src/coredump/coredump-send.c b/src/coredump/coredump-send.c index f178a3bf8c..6316511bad 100644 --- a/src/coredump/coredump-send.c +++ b/src/coredump/coredump-send.c @@ -13,6 +13,7 @@ #include "log.h" #include "namespace-util.h" #include "path-util.h" +#include "pidfd-util.h" #include "pidref.h" #include "process-util.h" #include "socket-util.h" @@ -94,45 +95,77 @@ int coredump_send(CoredumpContext *context) { return 0; } -static int can_forward_coredump(CoredumpContext *context, const PidRef *pid) { - _cleanup_free_ char *cgroup = NULL, *path = NULL, *unit = NULL; +static int can_forward_coredump(PidRef *pidref, PidRef *leader) { int r; - assert(context); - assert(pidref_is_set(pid)); - assert(!pidref_is_remote(pid)); + assert(pidref_is_set(pidref)); + assert(pidref_is_set(leader)); - /* We need to avoid a situation where the attacker crashes a SUID process or a root daemon and - * quickly replaces it with a namespaced process and we forward the coredump to the attacker, into - * the namespace. With %F/pidfd we can reliably check the namespace of the original process, hence we - * can allow forwarding. */ - if (!context->got_pidfd && context->dumpable != SUID_DUMP_USER) + if (pidref_equal(pidref, leader)) { + log_debug("The system service manager crashed."); return false; + } - r = cg_pidref_get_path(SYSTEMD_CGROUP_CONTROLLER, pid, &cgroup); + /* Check if the PID1 in the namespace is still running. */ + r = pidref_kill(leader, 0); if (r < 0) - return r; + return log_debug_errno(r, "Failed to send kill(0) to the service manager, maybe it is crashed, ignoring: %m"); + if (leader->fd >= 0) { + struct pidfd_info info = { + .mask = PIDFD_INFO_EXIT | PIDFD_INFO_COREDUMP, + }; + + r = pidfd_get_info(leader->fd, &info); + if (r >= 0) { + if (FLAGS_SET(info.mask, PIDFD_INFO_EXIT)) { + log_debug("PID1 has already exited."); + return false; + } + + if (FLAGS_SET(info.mask, PIDFD_INFO_COREDUMP) && FLAGS_SET(info.coredump_mask, PIDFD_COREDUMPED)) { + log_debug("PID1 has already dumped core."); + return false; + } + } else if (r != -EOPNOTSUPP) + return log_debug_errno(r, "ioctl(PIDFD_GET_INFO) for the service manager failed, maybe crashed, ignoring: %m"); + } + + _cleanup_free_ char *cgroup = NULL; + r = cg_pidref_get_path(SYSTEMD_CGROUP_CONTROLLER, leader, &cgroup); + if (r < 0) + return log_debug_errno(r, "Failed to get cgroup of the leader process, ignoring: %m"); + + _cleanup_free_ char *path = NULL; r = path_extract_directory(cgroup, &path); if (r < 0) - return r; + return log_debug_errno(r, "Failed to get the parent directory of \"%s\", ignoring: %m", cgroup); + _cleanup_free_ char *unit = NULL; r = cg_path_get_unit_path(path, &unit); if (r == -ENOMEM) - return log_oom(); + return log_oom_debug(); if (r == -ENXIO) /* No valid units in this path. */ return false; if (r < 0) - return r; + return log_debug_errno(r, "Failed to get unit path from cgroup \"%s\", ignoring: %m", path); /* We require that this process belongs to a delegated cgroup * (i.e. Delegate=yes), with CoredumpReceive=yes also. */ r = cg_is_delegated(unit); - if (r <= 0) - return r; + if (r < 0) + return log_debug_errno(r, "Failed to determine if cgroup \"%s\" is delegated, ignoring: %m", unit); + if (r == 0) + return false; - return cg_has_coredump_receive(unit); + r = cg_has_coredump_receive(unit); + if (r < 0) + return log_debug_errno(r, "Failed to determine if cgroup \"%s\" can receive coredump, ignoring: %m", unit); + if (r == 0) + return false; + + return true; } static int send_ucred(int transport_fd, const struct ucred *ucred) { @@ -203,17 +236,24 @@ int coredump_send_to_container(CoredumpContext *context) { assert(context); + if (context->same_pidns) + return 0; + + /* We need to avoid a situation where the attacker crashes a SUID process or a root daemon and + * quickly replaces it with a namespaced process and we forward the coredump to the attacker, into + * the namespace. With %F/pidfd we can reliably check the namespace of the original process, hence we + * can allow forwarding. */ + if (!context->got_pidfd && context->dumpable != SUID_DUMP_USER) + return 0; + _cleanup_(pidref_done) PidRef leader_pid = PIDREF_NULL; r = namespace_get_leader(&context->pidref, NAMESPACE_PID, &leader_pid); if (r < 0) return log_debug_errno(r, "Failed to get namespace leader: %m"); - r = can_forward_coredump(context, &leader_pid); - if (r < 0) - return log_debug_errno(r, "Failed to check if coredump can be forwarded: %m"); - if (r == 0) - return log_debug_errno(SYNTHETIC_ERRNO(ENOENT), - "Coredump will not be forwarded because no target cgroup was found."); + r = can_forward_coredump(&context->pidref, &leader_pid); + if (r <= 0) + return r; r = RET_NERRNO(socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, pair)); if (r < 0) @@ -291,5 +331,5 @@ int coredump_send_to_container(CoredumpContext *context) { if (r != EXIT_SUCCESS) return log_debug_errno(SYNTHETIC_ERRNO(EPROTO), "Failed to process coredump in container."); - return 0; + return 1; /* sent */ }