From e09543b87b76e4dc5285e397e30b0252f928aab5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 8 Oct 2025 14:30:25 +0900 Subject: [PATCH] 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 | 43 ++++++++++----- 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, 147 insertions(+), 85 deletions(-) create mode 100644 src/coredump/coredump-gperf.gperf diff --git a/src/coredump/coredump-config.c b/src/coredump/coredump-config.c index e84b2a0cbf4..de2bb68bf31 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 5d9e8e82a29..bb278f311e0 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; - -int coredump_parse_config(void); -uint64_t coredump_storage_size_max(void); +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; +}; + +#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 8b95868cfd3..b5a2d51a5e0 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 00000000000..087c73d5130 --- /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 88f8584ab16..9cac410b032 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 db9987b3ff6..be786be2f1a 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 a4c9c424cf5..56093e2d7b6 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 35ff30f3daa..460b2d14b11 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 b89503a6ced..6e110db630c 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 a2638390a1e..a401ad34fe4 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, -- 2.47.3