From 5f3e0a74689f2acf05c37f5203acec0b8d067458 Mon Sep 17 00:00:00 2001 From: Hayden Walles Date: Wed, 23 Dec 2015 13:59:31 -0500 Subject: [PATCH] coredump: fix bug that loses core dump files when core dumps are compressed and disk space is low. 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. --- src/journal/coredump.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/journal/coredump.c b/src/journal/coredump.c index f750ddfcbd6..7a260c32e02 100644 --- 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