doczkal / rpms / abrt

Forked from rpms/abrt 4 years ago
Clone

Blame SOURCES/0112-ccpp-revert-the-UID-GID-changes-if-user-core-fails.patch

a60cd7
From 7269a2cc88735aee0d1fa62491b9efe73ab5c6e8 Mon Sep 17 00:00:00 2001
a60cd7
From: Jakub Filak <jfilak@redhat.com>
a60cd7
Date: Mon, 4 May 2015 13:23:43 +0200
a60cd7
Subject: [ABRT PATCH] ccpp: revert the UID/GID changes if user core fails
a60cd7
a60cd7
Thanks Florian Weimer <fweimer@redhat.com>
a60cd7
a60cd7
Signed-off-by: Jakub Filak <jfilak@redhat.com>
a60cd7
---
a60cd7
 src/hooks/abrt-hook-ccpp.c | 58 ++++++++++++++++++++++++++++------------------
a60cd7
 1 file changed, 36 insertions(+), 22 deletions(-)
a60cd7
a60cd7
diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c
a60cd7
index 02f15d5..fdd9b06 100644
a60cd7
--- a/src/hooks/abrt-hook-ccpp.c
a60cd7
+++ b/src/hooks/abrt-hook-ccpp.c
a60cd7
@@ -351,9 +351,6 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu
a60cd7
         return -1;
a60cd7
     }
a60cd7
 
a60cd7
-    xsetegid(get_fsgid());
a60cd7
-    xseteuid(fsuid);
a60cd7
-
a60cd7
     if (strcmp(core_basename, "core") == 0)
a60cd7
     {
a60cd7
         /* Mimic "core.PID" if requested */
a60cd7
@@ -446,36 +443,53 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu
a60cd7
      * and the description of the /proc/sys/fs/suid_dumpable file in proc(5).)
a60cd7
      */
a60cd7
 
a60cd7
-    /* Set SELinux context like kernel when creating core dump file */
a60cd7
-    if (newcon != NULL && setfscreatecon_raw(newcon) < 0)
a60cd7
-    {
a60cd7
-        perror_msg("setfscreatecon_raw(%s)", newcon);
a60cd7
-        return -1;
a60cd7
-    }
a60cd7
+    int user_core_fd = -1;
a60cd7
+    int selinux_fail = 1;
a60cd7
 
a60cd7
-    struct stat sb;
a60cd7
-    errno = 0;
a60cd7
-    /* Do not O_TRUNC: if later checks fail, we do not want to have file already modified here */
a60cd7
-    int user_core_fd = openat(dirfd(proc_cwd), core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW | g_user_core_flags, 0600); /* kernel makes 0600 too */
a60cd7
+    /*
a60cd7
+     * These calls must be reverted as soon as possible.
a60cd7
+     */
a60cd7
+    xsetegid(get_fsgid());
a60cd7
+    xseteuid(fsuid);
a60cd7
 
a60cd7
-    if (newcon != NULL && setfscreatecon_raw(NULL) < 0)
a60cd7
+    /* Set SELinux context like kernel when creating core dump file.
a60cd7
+     * This condition is TRUE if */
a60cd7
+    if (/* SELinux is disabled  */ newcon == NULL
a60cd7
+     || /* or the call succeeds */ setfscreatecon_raw(newcon) >= 0)
a60cd7
     {
a60cd7
-        error_msg("setfscreatecon_raw(NULL)");
a60cd7
-        goto user_core_fail;
a60cd7
+        /* Do not O_TRUNC: if later checks fail, we do not want to have file already modified here */
a60cd7
+        user_core_fd = openat(dirfd(proc_cwd), core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW | g_user_core_flags, 0600); /* kernel makes 0600 too */
a60cd7
+
a60cd7
+        /* Do the error check here and print the error message in order to
a60cd7
+         * avoid interference in 'errno' usage caused by SELinux functions */
a60cd7
+        if (user_core_fd < 0)
a60cd7
+            perror_msg("Can't open '%s' at '%s'", core_basename, user_pwd);
a60cd7
+
a60cd7
+        /* Fail if SELinux is enabled and the call fails */
a60cd7
+        if (newcon != NULL && setfscreatecon_raw(NULL) < 0)
a60cd7
+            perror_msg("setfscreatecon_raw(NULL)");
a60cd7
+        else
a60cd7
+            selinux_fail = 0;
a60cd7
     }
a60cd7
+    else
a60cd7
+        perror_msg("setfscreatecon_raw(%s)", newcon);
a60cd7
 
a60cd7
+    /*
a60cd7
+     * DON'T JUMP OVER THIS REVERT OF THE UID/GID CHANGES
a60cd7
+     */
a60cd7
     xsetegid(0);
a60cd7
     xseteuid(0);
a60cd7
-    if (user_core_fd < 0
a60cd7
-     || fstat(user_core_fd, &sb) != 0
a60cd7
+
a60cd7
+    if (user_core_fd < 0 || selinux_fail)
a60cd7
+        goto user_core_fail;
a60cd7
+
a60cd7
+    struct stat sb;
a60cd7
+    if (fstat(user_core_fd, &sb) != 0
a60cd7
      || !S_ISREG(sb.st_mode)
a60cd7
      || sb.st_nlink != 1
a60cd7
      || sb.st_uid != fsuid
a60cd7
     ) {
a60cd7
-        if (user_core_fd < 0)
a60cd7
-            perror_msg("Can't open '%s' at '%s'", core_basename, user_pwd);
a60cd7
-        else
a60cd7
-            perror_msg("'%s' at '%s' is not a regular file with link count 1 owned by UID(%d)", core_basename, user_pwd, fsuid);
a60cd7
+        perror_msg("'%s' at '%s' is not a regular file with link count 1 owned by UID(%d)", core_basename, user_pwd, fsuid);
a60cd7
         goto user_core_fail;
a60cd7
     }
a60cd7
     if (ftruncate(user_core_fd, 0) != 0) {
a60cd7
-- 
a60cd7
1.8.3.1
a60cd7