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