From d0ad4e88d4e6b5e312c359a6505125f7e088f3e3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 16 Oct 2024 19:27:36 +0900 Subject: [PATCH 1/2] journalctl: erase verify key before free Even optarg is erased, copied string was not erased. Let's erase the copied key for safety. --- src/journal/journalctl.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 8ed5d98675a..7a49ed8db7a 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -96,7 +96,7 @@ static ImagePolicy *arg_image_policy = NULL; STATIC_DESTRUCTOR_REGISTER(arg_file, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_facilities, set_freep); -STATIC_DESTRUCTOR_REGISTER(arg_verify_key, freep); +STATIC_DESTRUCTOR_REGISTER(arg_verify_key, erase_and_freep); STATIC_DESTRUCTOR_REGISTER(arg_syslog_identifier, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_exclude_identifier, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_system_units, strv_freep); @@ -689,9 +689,11 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_VERIFY_KEY: - r = free_and_strdup(&arg_verify_key, optarg); - if (r < 0) - return r; + erase_and_free(arg_verify_key); + arg_verify_key = strdup(optarg); + if (!arg_verify_key) + return log_oom(); + /* Use memset not explicit_bzero() or similar so this doesn't look confusing * in ps or htop output. */ memset(optarg, 'x', strlen(optarg)); From ce2b92e8b016a677ca60b233318127478e7c53cb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 16 Oct 2024 19:34:18 +0900 Subject: [PATCH 2/2] journalctl: do not directly use optarg, but copy optarg before use Otherwise, if the process forks child processes, then the arguments cannot be used from them. To avoid potential issues like the one fixed by 6d3012bab4ce4c1ed260598d05b4e9f2ea471658. --- src/journal/journalctl.c | 70 ++++++++++++++++++++++++++++------------ src/journal/journalctl.h | 16 ++++----- 2 files changed, 57 insertions(+), 29 deletions(-) diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 7a49ed8db7a..084e1b492d3 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -50,11 +50,11 @@ sd_id128_t arg_boot_id = {}; int arg_boot_offset = 0; bool arg_dmesg = false; bool arg_no_hostname = false; -const char *arg_cursor = NULL; -const char *arg_cursor_file = NULL; -const char *arg_after_cursor = NULL; +char *arg_cursor = NULL; +char *arg_cursor_file = NULL; +char *arg_after_cursor = NULL; bool arg_show_cursor = false; -const char *arg_directory = NULL; +char *arg_directory = NULL; char **arg_file = NULL; bool arg_file_stdin = false; int arg_priorities = 0; @@ -75,7 +75,7 @@ char **arg_user_units = NULL; bool arg_invocation = false; sd_id128_t arg_invocation_id = SD_ID128_NULL; int arg_invocation_offset = 0; -const char *arg_field = NULL; +char *arg_field = NULL; bool arg_catalog = false; bool arg_reverse = false; int arg_journal_type = 0; @@ -83,17 +83,21 @@ int arg_journal_additional_open_flags = 0; int arg_namespace_flags = 0; char *arg_root = NULL; char *arg_image = NULL; -const char *arg_machine = NULL; -const char *arg_namespace = NULL; +char *arg_machine = NULL; +char *arg_namespace = NULL; uint64_t arg_vacuum_size = 0; uint64_t arg_vacuum_n_files = 0; usec_t arg_vacuum_time = 0; Set *arg_output_fields = NULL; -const char *arg_pattern = NULL; +char *arg_pattern = NULL; pcre2_code *arg_compiled_pattern = NULL; PatternCompileCase arg_case = PATTERN_COMPILE_CASE_AUTO; static ImagePolicy *arg_image_policy = NULL; +STATIC_DESTRUCTOR_REGISTER(arg_cursor, freep); +STATIC_DESTRUCTOR_REGISTER(arg_cursor_file, freep); +STATIC_DESTRUCTOR_REGISTER(arg_after_cursor, freep); +STATIC_DESTRUCTOR_REGISTER(arg_directory, freep); STATIC_DESTRUCTOR_REGISTER(arg_file, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_facilities, set_freep); STATIC_DESTRUCTOR_REGISTER(arg_verify_key, erase_and_freep); @@ -101,9 +105,13 @@ STATIC_DESTRUCTOR_REGISTER(arg_syslog_identifier, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_exclude_identifier, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_system_units, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_user_units, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_field, freep); STATIC_DESTRUCTOR_REGISTER(arg_root, freep); STATIC_DESTRUCTOR_REGISTER(arg_image, freep); +STATIC_DESTRUCTOR_REGISTER(arg_machine, freep); +STATIC_DESTRUCTOR_REGISTER(arg_namespace, freep); STATIC_DESTRUCTOR_REGISTER(arg_output_fields, set_freep); +STATIC_DESTRUCTOR_REGISTER(arg_pattern, freep); STATIC_DESTRUCTOR_REGISTER(arg_compiled_pattern, pattern_freep); STATIC_DESTRUCTOR_REGISTER(arg_image_policy, image_policy_freep); @@ -567,24 +575,29 @@ static int parse_argv(int argc, char *argv[]) { break; case 'M': - arg_machine = optarg; + r = free_and_strdup_warn(&arg_machine, optarg); + if (r < 0) + return r; break; case ARG_NAMESPACE: if (streq(optarg, "*")) { arg_namespace_flags = SD_JOURNAL_ALL_NAMESPACES; - arg_namespace = NULL; + arg_namespace = mfree(arg_namespace); } else if (startswith(optarg, "+")) { arg_namespace_flags = SD_JOURNAL_INCLUDE_DEFAULT_NAMESPACE; - arg_namespace = optarg + 1; + r = free_and_strdup_warn(&arg_namespace, optarg + 1); + if (r < 0) + return r; } else if (isempty(optarg)) { arg_namespace_flags = 0; - arg_namespace = NULL; + arg_namespace = mfree(arg_namespace); } else { arg_namespace_flags = 0; - arg_namespace = optarg; + r = free_and_strdup_warn(&arg_namespace, optarg); + if (r < 0) + return r; } - break; case ARG_LIST_NAMESPACES: @@ -592,7 +605,9 @@ static int parse_argv(int argc, char *argv[]) { break; case 'D': - arg_directory = optarg; + r = free_and_strdup_warn(&arg_directory, optarg); + if (r < 0) + return r; break; case 'i': @@ -628,15 +643,21 @@ static int parse_argv(int argc, char *argv[]) { break; case 'c': - arg_cursor = optarg; + r = free_and_strdup_warn(&arg_cursor, optarg); + if (r < 0) + return r; break; case ARG_CURSOR_FILE: - arg_cursor_file = optarg; + r = free_and_strdup_warn(&arg_cursor_file, optarg); + if (r < 0) + return r; break; case ARG_AFTER_CURSOR: - arg_after_cursor = optarg; + r = free_and_strdup_warn(&arg_after_cursor, optarg); + if (r < 0) + return r; break; case ARG_SHOW_CURSOR: @@ -793,7 +814,9 @@ static int parse_argv(int argc, char *argv[]) { } case 'g': - arg_pattern = optarg; + r = free_and_strdup_warn(&arg_pattern, optarg); + if (r < 0) + return r; break; case ARG_CASE_SENSITIVE: @@ -863,7 +886,9 @@ static int parse_argv(int argc, char *argv[]) { case 'F': arg_action = ACTION_LIST_FIELDS; - arg_field = optarg; + r = free_and_strdup_warn(&arg_field, optarg); + if (r < 0) + return r; break; case 'N': @@ -1034,6 +1059,7 @@ static int parse_argv(int argc, char *argv[]) { static int run(int argc, char *argv[]) { _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; _cleanup_(umount_and_freep) char *mounted_dir = NULL; + _cleanup_strv_free_ char **args = NULL; int r; setlocale(LC_ALL, ""); @@ -1043,7 +1069,9 @@ static int run(int argc, char *argv[]) { if (r <= 0) return r; - char **args = strv_skip(argv, optind); + r = strv_copy_unless_empty(strv_skip(argv, optind), &args); + if (r < 0) + return log_oom(); if (arg_image) { assert(!arg_root); diff --git a/src/journal/journalctl.h b/src/journal/journalctl.h index 409cfe2bcaa..5e3e73544fb 100644 --- a/src/journal/journalctl.h +++ b/src/journal/journalctl.h @@ -55,11 +55,11 @@ extern sd_id128_t arg_boot_id; extern int arg_boot_offset; extern bool arg_dmesg; extern bool arg_no_hostname; -extern const char *arg_cursor; -extern const char *arg_cursor_file; -extern const char *arg_after_cursor; +extern char *arg_cursor; +extern char *arg_cursor_file; +extern char *arg_after_cursor; extern bool arg_show_cursor; -extern const char *arg_directory; +extern char *arg_directory; extern char **arg_file; extern bool arg_file_stdin; extern int arg_priorities; @@ -80,7 +80,7 @@ extern char **arg_user_units; extern bool arg_invocation; extern sd_id128_t arg_invocation_id; extern int arg_invocation_offset; -extern const char *arg_field; +extern char *arg_field; extern bool arg_catalog; extern bool arg_reverse; extern int arg_journal_type; @@ -88,13 +88,13 @@ extern int arg_journal_additional_open_flags; extern int arg_namespace_flags; extern char *arg_root; extern char *arg_image; -extern const char *arg_machine; -extern const char *arg_namespace; +extern char *arg_machine; +extern char *arg_namespace; extern uint64_t arg_vacuum_size; extern uint64_t arg_vacuum_n_files; extern usec_t arg_vacuum_time; extern Set *arg_output_fields; -extern const char *arg_pattern; +extern char *arg_pattern; extern pcre2_code *arg_compiled_pattern; extern PatternCompileCase arg_case;