From 7bd77a63f226a572946f30db3e76f23f971f46d5 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Wed, 20 May 2015 06:07:15 +0200
Subject: [ABRT PATCH] ccpp: do not unlink failed and big user cores

* We might end up deleting an already existing file.
* Kernel does not delete nor truncate core files. Admittedly, kernel
  knows how process's memory is structured, dumps it per logical
  segments and checks whether a next segment can be written.
* 'ulimit -c' does not seem to be a hard limit. Kernel wrote 8192 bytes
  despite $(ulimit -c) == 6.

Related: #1212818

Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
 src/hooks/abrt-hook-ccpp.c | 70 +++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 41 deletions(-)

diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c
index fdd9b06..9b38ed7 100644
--- a/src/hooks/abrt-hook-ccpp.c
+++ b/src/hooks/abrt-hook-ccpp.c
@@ -129,8 +129,8 @@ static off_t copyfd_sparse(int src_fd, int dst_fd1, int dst_fd2, off_t size2)
 		size2 -= rd;
 		if (size2 < 0)
 			dst_fd2 = -1;
-//TODO: truncate to 0 or even delete the second file
-//(currently we delete the file later)
+// truncate to 0 or even delete the second file?
+// No, kernel does not delete nor truncate core files.
 	}
  out:
 
@@ -502,13 +502,20 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu
 
 user_core_fail:
     if (user_core_fd >= 0)
-    {
         close(user_core_fd);
-        unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0);
-    }
     return -1;
 }
 
+static int close_user_core(int user_core_fd, off_t core_size)
+{
+    if (user_core_fd >= 0 && (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 || core_size < 0))
+    {
+        perror_msg("Error writing '%s' at '%s'", core_basename, user_pwd);
+        return -1;
+    }
+    return 0;
+}
+
 static bool dump_fd_info(const char *dest_filename, char *source_filename, int source_base_ofs, uid_t uid, gid_t gid)
 {
     FILE *fp = fopen(dest_filename, "wx");
@@ -569,7 +576,7 @@ static int create_or_die(const char *filename)
     if (dd)
         dd_delete(dd);
     if (user_core_fd >= 0)
-        unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0);
+        close(user_core_fd);
 
     errno = sv_errno;
     perror_msg_and_die("Can't open '%s'", filename);
@@ -577,6 +584,7 @@ static int create_or_die(const char *filename)
 
 int main(int argc, char** argv)
 {
+    int err = 1;
     /* Kernel starts us with all fd's closed.
      * But it's dangerous:
      * fprintf(stderr) can dump messages into random fds, etc.
@@ -778,9 +786,8 @@ int main(int argc, char** argv)
             error_msg_and_die("Error saving '%s'", path);
         }
         log("Saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size);
-        if (proc_cwd != NULL)
-            closedir(proc_cwd);
-        return 0;
+        err = 0;
+        goto finito;
     }
 
     unsigned path_len = snprintf(path, sizeof(path), "%s/ccpp-%s-%lu.new",
@@ -895,26 +902,17 @@ int main(int argc, char** argv)
          * ls: cannot access core*: No such file or directory <=== BAD
          */
         off_t core_size = copyfd_sparse(STDIN_FILENO, abrt_core_fd, user_core_fd, ulimit_c);
+
+        close_user_core(user_core_fd, core_size);
+
         if (fsync(abrt_core_fd) != 0 || close(abrt_core_fd) != 0 || core_size < 0)
         {
             unlink(path);
             dd_delete(dd);
-            if (user_core_fd >= 0)
-                unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0);
             /* copyfd_sparse logs the error including errno string,
              * but it does not log file name */
             error_msg_and_die("Error writing '%s'", path);
         }
-        if (user_core_fd >= 0
-            /* error writing user coredump? */
-         && (fsync(user_core_fd) != 0 || close(user_core_fd) != 0
-            /* user coredump is too big? */
-            || (ulimit_c == 0 /* paranoia */ || core_size > ulimit_c)
-            )
-        ) {
-            /* nuke it (silently) */
-            unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0);
-        }
 
 /* Because of #1211835 and #1126850 */
 #if 0
@@ -984,9 +982,9 @@ int main(int argc, char** argv)
         }
 
         free(rootdir);
-        if (proc_cwd != NULL)
-            closedir(proc_cwd);
-        return 0;
+
+        err = 0;
+        goto finito;
     }
 
     /* We didn't create abrt dump, but may need to create compat coredump */
@@ -994,26 +992,16 @@ int main(int argc, char** argv)
     if (user_core_fd >= 0)
     {
         off_t core_size = copyfd_size(STDIN_FILENO, user_core_fd, ulimit_c, COPYFD_SPARSE);
-        if (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 || core_size < 0)
-        {
-            /* perror first, otherwise unlink may trash errno */
-            perror_msg("Error writing '%s' at '%s'", core_basename, user_pwd);
-            unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0);
-            if (proc_cwd != NULL)
-                closedir(proc_cwd);
-            return 1;
-        }
-        if (ulimit_c == 0 || core_size > ulimit_c)
-        {
-            unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0);
-            if (proc_cwd != NULL)
-                closedir(proc_cwd);
-            return 1;
-        }
+        if (close_user_core(user_core_fd, core_size) != 0)
+            goto finito;
+
+        err = 0;
         log("Saved core dump of pid %lu to %s at %s (%llu bytes)", (long)pid, core_basename, user_pwd, (long long)core_size);
     }
 
+ finito:
     if (proc_cwd != NULL)
         closedir(proc_cwd);
-    return 0;
+
+    return err;
 }
-- 
1.8.3.1