coredump: fix bug that loses core dump files when core dumps are compressed and disk...

Previously the save_external_coredump function returned a file
descriptor corresponding to the dumped file.  This descriptor was used
for two different purposes by calling code: a) access to the raw core
dump data; b) testing candidate files (via inode comparisons) while
vacuuming to protect the current core dump from vacuuming.

The descriptor returned always corresponded to a file containing the raw
core dump data.  However if compresson was used and the core dump was
compressed then the descriptor returned did not correspond to the file
that would eventually be left on disk (ie the compressed file).  Thus
the file was never protected by vacuuming.  When disk space was low all
core dumps including the current one would be vacuumed and the
corresponding log message referred to a file that no longer existed.

This resulted in the following error message from coredumpctl if the
missing core dump was requested:
   Cannot retrieve coredump from journal nor disk.
   Failed to retrieve core: No such file or directory

save_external_coredump now returns two descriptors, one to be used for
inode comparisons to prevent overzealous vacuuming and one to be used
for raw data access.  When compression is not used the returned inode
comparison descriptor will be invalid, indicating that the raw data
access descriptor should be used for inode comparisons as well.

Corresponding use of save_external_coredump and the returned
descriptors also updated.

diff --git a/src/journal/coredump.c b/src/journal/coredump.c
index f750ddfcbd625eb54a94d81bbfb39e8c1637db21..7a260c32e02ad96823778d4f55838242a1d9dc47 100644 (file)
--- a/src/journal/coredump.c
+++ b/src/journal/coredump.c
@@ -297,7 +297,8 @@ static int save_external_coredump(
                 const char *info[_INFO_LEN],
                 uid_t uid,
                 char **ret_filename,
-                int *ret_fd,
+                int *ret_node_fd,
+                int *ret_data_fd,
                 uint64_t *ret_size) {
 
         _cleanup_free_ char *fn = NULL, *tmp = NULL;
@@ -307,7 +308,8 @@ static int save_external_coredump(
 
         assert(info);
         assert(ret_filename);
-        assert(ret_fd);
+        assert(ret_node_fd);
+        assert(ret_data_fd);
         assert(ret_size);
 
         r = make_filename(info, &fn);
@@ -386,11 +388,12 @@ static int save_external_coredump(
                 unlink_noerrno(tmp);
 
                 *ret_filename = fn_compressed;     /* compressed */
-                *ret_fd = fd;                      /* uncompressed */
+                *ret_node_fd = fd_compressed;      /* compressed */
+                *ret_data_fd = fd;                 /* uncompressed */
                 *ret_size = (uint64_t) st.st_size; /* uncompressed */
 
                 fn_compressed = NULL;
-                fd = -1;
+                fd = fd_compressed = -1;
 
                 return 0;
 
@@ -400,12 +403,14 @@ static int save_external_coredump(
 
 uncompressed:
 #endif
+
         r = fix_permissions(fd, tmp, fn, info, uid);
         if (r < 0)
                 goto fail;
 
         *ret_filename = fn;
-        *ret_fd = fd;
+        *ret_data_fd = fd;
+        *ret_node_fd = -1;
         *ret_size = (uint64_t) st.st_size;
 
         fn = NULL;
@@ -554,7 +559,7 @@ int main(int argc, char* argv[]) {
         _cleanup_free_ char *exe = NULL, *comm = NULL, *filename = NULL;
         const char *info[_INFO_LEN];
 
-        _cleanup_close_ int coredump_fd = -1;
+        _cleanup_close_ int coredump_fd = -1, coredump_node_fd = -1;
 
         struct iovec iovec[26];
         uint64_t coredump_size;
@@ -633,7 +638,7 @@ int main(int argc, char* argv[]) {
                         if (arg_storage != COREDUMP_STORAGE_NONE)
                                 arg_storage = COREDUMP_STORAGE_EXTERNAL;
 
-                        r = save_external_coredump(info, uid, &filename, &coredump_fd, &coredump_size);
+                        r = save_external_coredump(info, uid, &filename, &coredump_node_fd, &coredump_fd, &coredump_size);
                         if (r < 0)
                                 goto finish;
 
@@ -795,7 +800,7 @@ int main(int argc, char* argv[]) {
         coredump_vacuum(-1, arg_keep_free, arg_max_use);
 
         /* Always stream the coredump to disk, if that's possible */
-        r = save_external_coredump(info, uid, &filename, &coredump_fd, &coredump_size);
+        r = save_external_coredump(info, uid, &filename, &coredump_node_fd, &coredump_fd, &coredump_size);
         if (r < 0)
                 /* skip whole core dumping part */
                 goto log;
@@ -815,7 +820,7 @@ int main(int argc, char* argv[]) {
         }
 
         /* Vacuum again, but exclude the coredump we just created */
-        coredump_vacuum(coredump_fd, arg_keep_free, arg_max_use);
+        coredump_vacuum(coredump_node_fd >= 0 ? coredump_node_fd : coredump_fd, arg_keep_free, arg_max_use);
 
         /* Now, let's drop privileges to become the user who owns the
          * segfaulted process and allocate the coredump memory under