Merge pull request systemd#4225 from keszybz/coredump

coredump: remove Storage=both support, various fixes for sd-coredump and coredumpctl

Author: martinpitt

TODO

@@ -670,6 +670,7 @@ Features:
 
 * coredump:
   - save coredump in Windows/Mozilla minidump format
+  - when truncating coredumps, also log the full size that the process had, and make a metadata field so we can report truncated coredumps
 
 * support crash reporting operation modes (https://live.gnome.org/GnomeOS/Design/Whiteboards/ProblemReporting)
 

catalog/systemd.catalog.in

@@ -88,6 +88,17 @@ Process @COREDUMP_PID@ (@COREDUMP_COMM@) crashed and dumped core.
 This usually indicates a programming error in the crashing program and
 should be reported to its vendor as a bug.
 
+-- 5aadd8e954dc4b1a8c954d63fd9e1137
+Subject: Core file was truncated to @SIZE_LIMIT@ bytes.
+Defined-By: systemd
+Support: %SUPPORT_URL%
+Documentation: man:coredump.conf(5)
+
+The process had more memory mapped than the configured maximum for processing
+and storage by systemd-coredump(8). Only the first @SIZE_LIMIT@ bytes were
+saved. This core might still be usable, but various tools like gdb(1) will warn
+about the file being truncated.
+
 -- fc2e22bc6ee647b6b90729ab34a250b1 de
 Subject: Speicherabbild für Prozess @COREDUMP_PID@ (@COREDUMP_COMM) generiert
 Defined-By: systemd

man/coredump.conf.xml

@@ -83,16 +83,13 @@
       <varlistentry>
         <term><varname>Storage=</varname></term>
 
-        <listitem><para>Controls where to store cores. One of
-        <literal>none</literal>, <literal>external</literal>,
-        <literal>journal</literal>, and <literal>both</literal>. When
-        <literal>none</literal>, the core dumps will be logged but not
-        stored permanently. When <literal>external</literal> (the
-        default), cores will be stored in <filename>/var/lib/systemd/coredump</filename>.
-        When <literal>journal</literal>, cores will be stored in
-        the journal and rotated following normal journal
-        rotation patterns. When <literal>both</literal>, cores
-        will be stored in both locations.</para>
+        <listitem><para>Controls where to store cores. One of <literal>none</literal>,
+        <literal>external</literal>, and <literal>journal</literal>. When
+        <literal>none</literal>, the core dumps will be logged (included the traceback if
+        possible), but not stored permanently. When <literal>external</literal> (the
+        default), cores will be stored in <filename>/var/lib/systemd/coredump/</filename>.
+        When <literal>journal</literal>, cores will be stored in the journal and rotated
+        following normal journal rotation patterns.</para>
 
         <para>When cores are stored in the journal, they might be
         compressed following journal compression settings, see

src/coredump/coredump.c

@@ -28,9 +28,10 @@
 #include <elfutils/libdwfl.h>
 #endif
 
+#include "sd-daemon.h"
 #include "sd-journal.h"
 #include "sd-login.h"
-#include "sd-daemon.h"
+#include "sd-messages.h"
 
 #include "acl-util.h"
 #include "alloc-util.h"
@@ -93,7 +94,6 @@ typedef enum CoredumpStorage {
         COREDUMP_STORAGE_NONE,
         COREDUMP_STORAGE_EXTERNAL,
         COREDUMP_STORAGE_JOURNAL,
-        COREDUMP_STORAGE_BOTH,
         _COREDUMP_STORAGE_MAX,
         _COREDUMP_STORAGE_INVALID = -1
 } CoredumpStorage;
@@ -102,7 +102,6 @@ static const char* const coredump_storage_table[_COREDUMP_STORAGE_MAX] = {
         [COREDUMP_STORAGE_NONE] = "none",
         [COREDUMP_STORAGE_EXTERNAL] = "external",
         [COREDUMP_STORAGE_JOURNAL] = "journal",
-        [COREDUMP_STORAGE_BOTH] = "both",
 };
 
 DEFINE_PRIVATE_STRING_TABLE_LOOKUP(coredump_storage, CoredumpStorage);
@@ -135,6 +134,10 @@ static int parse_config(void) {
                                  false, NULL);
 }
 
+static inline uint64_t storage_size_max(void) {
+        return arg_storage == COREDUMP_STORAGE_EXTERNAL ? arg_external_size_max : arg_journal_size_max;
+}
+
 static int fix_acl(int fd, uid_t uid) {
 
 #ifdef HAVE_ACL
@@ -247,7 +250,7 @@ static int maybe_remove_external_coredump(const char *filename, uint64_t size) {
 
         /* Returns 1 if might remove, 0 if will not remove, < 0 on error. */
 
-        if (IN_SET(arg_storage, COREDUMP_STORAGE_EXTERNAL, COREDUMP_STORAGE_BOTH) &&
+        if (arg_storage == COREDUMP_STORAGE_EXTERNAL &&
             size <= arg_external_size_max)
                 return 0;
 
@@ -331,12 +334,13 @@ static int save_external_coredump(
                 /* Is coredumping disabled? Then don't bother saving/processing the coredump.
                  * Anything below PAGE_SIZE cannot give a readable coredump (the kernel uses
                  * ELF_EXEC_PAGESIZE which is not easily accessible, but is usually the same as PAGE_SIZE. */
-                log_info("Core dumping has been disabled for process %s (%s).", context[CONTEXT_PID], context[CONTEXT_COMM]);
+                log_info("Resource limits disable core dumping for process %s (%s).",
+                         context[CONTEXT_PID], context[CONTEXT_COMM]);
                 return -EBADSLT;
         }
 
         /* Never store more than the process configured, or than we actually shall keep or process */
-        max_size = MIN(rlimit, MAX(arg_process_size_max, arg_external_size_max));
+        max_size = MIN(rlimit, MAX(arg_process_size_max, storage_size_max()));
 
         r = make_filename(context, &fn);
         if (r < 0)
@@ -349,19 +353,18 @@ static int save_external_coredump(
                 return log_error_errno(fd, "Failed to create temporary file for coredump %s: %m", fn);
 
         r = copy_bytes(input_fd, fd, max_size, false);
-        if (r == -EFBIG) {
-                log_error("Coredump of %s (%s) is larger than configured processing limit, refusing.", context[CONTEXT_PID], context[CONTEXT_COMM]);
-                goto fail;
-        } else if (IN_SET(r, -EDQUOT, -ENOSPC)) {
-                log_error("Not enough disk space for coredump of %s (%s), refusing.", context[CONTEXT_PID], context[CONTEXT_COMM]);
-                goto fail;
-        } else if (r < 0) {
-                log_error_errno(r, "Failed to dump coredump to file: %m");
+        if (r < 0) {
+                log_error_errno(r, "Cannot store coredump of %s (%s): %m", context[CONTEXT_PID], context[CONTEXT_COMM]);
                 goto fail;
-        }
+        } else if (r == 1)
+                log_struct(LOG_INFO,
+                           LOG_MESSAGE("Core file was truncated to %zu bytes.", max_size),
+                           "SIZE_LIMIT=%zu", max_size,
+                           LOG_MESSAGE_ID(SD_MESSAGE_TRUNCATED_CORE),
+                           NULL);
 
         if (fstat(fd, &st) < 0) {
-                log_error_errno(errno, "Failed to fstat coredump %s: %m", coredump_tmpfile_name(tmp));
+                log_error_errno(errno, "Failed to fstat core file %s: %m", coredump_tmpfile_name(tmp));
                 goto fail;
         }
 
@@ -372,8 +375,7 @@ static int save_external_coredump(
 
 #if defined(HAVE_XZ) || defined(HAVE_LZ4)
         /* If we will remove the coredump anyway, do not compress. */
-        if (maybe_remove_external_coredump(NULL, st.st_size) == 0
-            && arg_compress) {
+        if (arg_compress && !maybe_remove_external_coredump(NULL, st.st_size)) {
 
                 _cleanup_free_ char *fn_compressed = NULL, *tmp_compressed = NULL;
                 _cleanup_close_ int fd_compressed = -1;
@@ -705,7 +707,9 @@ static int submit_coredump(
 
                 coredump_filename = strjoina("COREDUMP_FILENAME=", filename);
                 IOVEC_SET_STRING(iovec[n_iovec++], coredump_filename);
-        }
+        } else if (arg_storage == COREDUMP_STORAGE_EXTERNAL)
+                log_info("The core will not be stored: size %zu is greater than %zu (the configured maximum)",
+                         coredump_size, arg_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);
@@ -730,7 +734,9 @@ static int submit_coredump(
                         log_warning("Failed to generate stack trace: %s", dwfl_errmsg(dwfl_errno()));
                 else
                         log_warning_errno(r, "Failed to generate stack trace: %m");
-        }
+        } else
+                log_debug("Not generating stack trace: core size %zu is greater than %zu (the configured maximum)",
+                          coredump_size, arg_process_size_max);
 
         if (!core_message)
 #endif
@@ -740,18 +746,22 @@ static int submit_coredump(
                 IOVEC_SET_STRING(iovec[n_iovec++], core_message);
 
         /* Optionally store the entire coredump in the journal */
-        if (IN_SET(arg_storage, COREDUMP_STORAGE_JOURNAL, COREDUMP_STORAGE_BOTH) &&
-            coredump_size <= arg_journal_size_max) {
-                size_t sz = 0;
-
-                /* Store the coredump itself in the journal */
-
-                r = allocate_journal_field(coredump_fd, (size_t) coredump_size, &coredump_data, &sz);
-                if (r >= 0) {
-                        iovec[n_iovec].iov_base = coredump_data;
-                        iovec[n_iovec].iov_len = sz;
-                        n_iovec++;
-                }
+        if (arg_storage == COREDUMP_STORAGE_JOURNAL) {
+                if (coredump_size <= arg_journal_size_max) {
+                        size_t sz = 0;
+
+                        /* Store the coredump itself in the journal */
+
+                        r = allocate_journal_field(coredump_fd, (size_t) coredump_size, &coredump_data, &sz);
+                        if (r >= 0) {
+                                iovec[n_iovec].iov_base = coredump_data;
+                                iovec[n_iovec].iov_len = sz;
+                                n_iovec++;
+                        } else
+                                log_warning_errno(r, "Failed to attach the core to the journal entry: %m");
+                } else
+                        log_info("The core will not be stored: size %zu is greater than %zu (the configured maximum)",
+                                 coredump_size, arg_journal_size_max);
         }
 
         assert(n_iovec <= n_iovec_allocated);