From 3746131aac4798cacf67b60cfc4e2e1c80ec4efb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Aug 2023 04:46:00 +0900 Subject: [PATCH 1/2] io-util: introduce iovw_append() helper function --- src/basic/io-util.c | 37 +++++++++++++++++++++++++++++++++++++ src/basic/io-util.h | 1 + 2 files changed, 38 insertions(+) diff --git a/src/basic/io-util.c b/src/basic/io-util.c index 25e551604a..21828d7be8 100644 --- a/src/basic/io-util.c +++ b/src/basic/io-util.c @@ -374,6 +374,43 @@ size_t iovw_size(struct iovec_wrapper *iovw) { return n; } +int iovw_append(struct iovec_wrapper *target, const struct iovec_wrapper *source) { + size_t original_count; + int r; + + assert(target); + + /* This duplicates the source and merges it into the target. */ + + if (!source || source->count == 0) + return 0; + + original_count = target->count; + + FOREACH_ARRAY(iovec, source->iovec, source->count) { + void *dup; + + dup = memdup(iovec->iov_base, iovec->iov_len); + if (!dup) { + r = -ENOMEM; + goto rollback; + } + + r = iovw_consume(target, dup, iovec->iov_len); + if (r < 0) + goto rollback; + } + + return 0; + +rollback: + for (size_t i = original_count; i < target->count; i++) + free(target->iovec[i].iov_base); + + target->count = original_count; + return r; +} + void iovec_array_free(struct iovec *iov, size_t n) { if (!iov) return; diff --git a/src/basic/io-util.h b/src/basic/io-util.h index 4f7989e66d..6ea2eb20bc 100644 --- a/src/basic/io-util.h +++ b/src/basic/io-util.h @@ -111,5 +111,6 @@ int iovw_put_string_field(struct iovec_wrapper *iovw, const char *field, const c int iovw_put_string_field_free(struct iovec_wrapper *iovw, const char *field, char *value); void iovw_rebase(struct iovec_wrapper *iovw, char *old, char *new); size_t iovw_size(struct iovec_wrapper *iovw); +int iovw_append(struct iovec_wrapper *target, const struct iovec_wrapper *source); void iovec_array_free(struct iovec *iov, size_t n); From 3a19fe4637b5c5ed92a57d8038c6f895538e12d9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 11 Aug 2023 04:48:01 +0900 Subject: [PATCH 2/2] coredump: fix various invalid memory access Previously, we did not check error from iovw_put(). If it fails, the target iovw may have no iov or partial iovs from the journal importar. So, the finalization may cause underflow and may access and free invalid memory. Follow-up for 946dc7c635f050129896d1515c08a81504af2421. --- src/coredump/coredump.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 659eae3b3e..bbe0f1387e 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -1421,11 +1421,11 @@ static int process_kernel(int argc, char* argv[]) { } static int process_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; Context context = {}; - struct iovec_wrapper *iovw; char *message; int r; - _cleanup_(journal_importer_cleanup) JournalImporter importer = JOURNAL_IMPORTER_INIT(STDIN_FILENO); log_debug("Processing backtrace on stdin..."); @@ -1440,19 +1440,17 @@ static int process_backtrace(int argc, char *argv[]) { * '--backtrace' option */ r = gather_pid_metadata_from_argv(iovw, &context, argc - 2, argv + 2); if (r < 0) - goto finish; + return r; /* Collect the rest of the process metadata retrieved from the runtime */ r = gather_pid_metadata_from_procfs(iovw, &context); if (r < 0) - goto finish; + return r; for (;;) { r = journal_importer_process_data(&importer); - if (r < 0) { - log_error_errno(r, "Failed to parse journal entry on stdin: %m"); - goto finish; - } + if (r < 0) + return log_error_errno(r, "Failed to parse journal entry on stdin: %m"); if (r == 1 || /* complete entry */ journal_importer_eof(&importer)) /* end of data */ break; @@ -1468,23 +1466,20 @@ static int process_backtrace(int argc, char *argv[]) { r = iovw_put_string_field(iovw, "MESSAGE=", message); if (r < 0) - goto finish; + return r; } else { - /* The imported iovecs are not supposed to be freed by us so let's store - * them at the end of the array so we can skip them while freeing the - * rest. */ - FOREACH_ARRAY(iovec, importer.iovw.iovec, importer.iovw.count) - iovw_put(iovw, iovec->iov_base, iovec->iov_len); + /* 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); + if (r < 0) + return r; } r = sd_journal_sendv(iovw->iovec, iovw->count); if (r < 0) - log_error_errno(r, "Failed to log backtrace: %m"); + return log_error_errno(r, "Failed to log backtrace: %m"); - finish: - iovw->count -= importer.iovw.count; - iovw = iovw_free_free(iovw); - return r; + return 0; } static int run(int argc, char *argv[]) {