b225ea
From 6724ba03fea310439c02f97d9429b921d12275c5 Mon Sep 17 00:00:00 2001
b225ea
From: Matej Habrnal <mhabrnal@redhat.com>
b225ea
Date: Thu, 19 May 2016 12:10:42 +0200
b225ea
Subject: [PATCH] ccpp: unify log message of ignored crashes
b225ea
b225ea
ABRT will ignore crashes in executables for which absolute path matches one of
b225ea
specified patterns.
b225ea
b225ea
Example of log messages in case of ignoring crashes:
b225ea
- Crash's path is listed in 'IgnoredPath' in CCpp.conf
b225ea
    Process 16431 (will_segfault) of user 0 killed by SIGSEGV - ignoring
b225ea
    (listed in 'IgnoredPaths')
b225ea
b225ea
- Repeating crash
b225ea
    Process 16219 (will_segfault) of user 1000 killed by SIGSEGV -
b225ea
    ignoring (repeated crash)
b225ea
b225ea
- abrt-ccpp-hook crash
b225ea
    Process 16223 (abrt-hook-ccpp) of user 1000 killed by SIGSEGV -
b225ea
    ignoring (avoid recursion)
b225ea
b225ea
- abrt crash
b225ea
    Process 16228 (abrt_test) of user 1000 killed by SIGSEGV -
b225ea
    ignoring ('DebugLevel' == 0)
b225ea
b225ea
- not supported signal
b225ea
    Process 16229 (crash) of user 1000 killed by signal 99 - ignoring
b225ea
    (unsupported signal)
b225ea
b225ea
- abrtd is not running
b225ea
    Process 16229 (crash) of user 1000 killed by signal 99 - ignoring
b225ea
    (abrtd is not running)
b225ea
b225ea
- low free space
b225ea
    Process 16229 (crash) of user 1000 killed by signal 99 - ignoring
b225ea
    (low free space)
b225ea
b225ea
- failed to parse /proc/$PID/status Uid
b225ea
    Process 16229 (crash) of user 1000 killed by signal 99 - ignoring
b225ea
    (Failed to parse /proc/16229/status (Uid))
b225ea
b225ea
- failed to parse /proc/$PID/status Gid
b225ea
    Process 16229 (crash) of user 1000 killed by signal 99 - ignoring
b225ea
    (Failed to parse /proc/16229/status (Gid))
b225ea
b225ea
- failed to get executable
b225ea
    Process 16229 (crash) of user 1000 killed by signal 99 - ignoring
b225ea
    (Can't read /proc/16229/exe link)
b225ea
b225ea
- core size limit is bogus
b225ea
    Process 16229 (crash) of user 1000 killed by signal 99 - ignoring
b225ea
    (RLIMIT_CORE 'foo' is bogus)
b225ea
b225ea
I the case the crash is not ignored the log msg is following:
b225ea
    Process 21768 (will_segfault) of user 1000 killed by SIGSEGV -
b225ea
    dumping core
b225ea
b225ea
Related to: #1337186
b225ea
b225ea
Signed-off-by: Matej Habrnal <mhabrnal@redhat.com>
b225ea
---
b225ea
 src/hooks/abrt-hook-ccpp.c | 211 ++++++++++++++++++++++++++++-----------------
b225ea
 1 file changed, 133 insertions(+), 78 deletions(-)
b225ea
b225ea
diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c
b225ea
index 2c05c78..dc4dec6 100644
b225ea
--- a/src/hooks/abrt-hook-ccpp.c
b225ea
+++ b/src/hooks/abrt-hook-ccpp.c
b225ea
@@ -695,7 +695,7 @@ static int test_configuration(bool setting_SaveFullCore, bool setting_CreateCore
b225ea
     return 0;
b225ea
 }
b225ea
 
b225ea
-static void error_msg_not_process_crash(const char *pid_str, const char *process_str,
b225ea
+static void error_msg_process_crash(const char *pid_str, const char *process_str,
b225ea
         long unsigned uid, int signal_no, const char *signame, const char *message, ...)
b225ea
 {
b225ea
     va_list p;
b225ea
@@ -706,10 +706,10 @@ static void error_msg_not_process_crash(const char *pid_str, const char *process
b225ea
     char *process_name = (process_str) ?  xasprintf(" (%s)", process_str) : xstrdup("");
b225ea
 
b225ea
     if (signame)
b225ea
-        error_msg("Process %s (%s) of user %lu killed by SIG%s - %s", pid_str,
b225ea
+        error_msg("Process %s%s of user %lu killed by SIG%s - %s", pid_str,
b225ea
                         process_name, uid, signame, message_full);
b225ea
     else
b225ea
-        error_msg("Process %s (%s) of user %lu killed by signal %d - %s", pid_str,
b225ea
+        error_msg("Process %s%s of user %lu killed by signal %d - %s", pid_str,
b225ea
                         process_name, uid, signal_no, message_full);
b225ea
 
b225ea
     free(process_name);
b225ea
@@ -718,6 +718,20 @@ static void error_msg_not_process_crash(const char *pid_str, const char *process
b225ea
     return;
b225ea
 }
b225ea
 
b225ea
+static void error_msg_ignore_crash(const char *pid_str, const char *process_str,
b225ea
+        long unsigned uid, int signal_no, const char *signame, const char *message, ...)
b225ea
+{
b225ea
+    va_list p;
b225ea
+    va_start(p, message);
b225ea
+    char *message_full = xvasprintf(message, p);
b225ea
+    va_end(p);
b225ea
+
b225ea
+    error_msg_process_crash(pid_str, process_str, uid, signal_no, signame, "ignoring (%s)", message_full);
b225ea
+
b225ea
+    free(message_full);
b225ea
+    return;
b225ea
+}
b225ea
+
b225ea
 int main(int argc, char** argv)
b225ea
 {
b225ea
     int err = 1;
b225ea
@@ -798,24 +812,35 @@ int main(int argc, char** argv)
b225ea
         }
b225ea
     }
b225ea
 
b225ea
-    errno = 0;
b225ea
     const char* signal_str = argv[1];
b225ea
     int signal_no = xatoi_positive(signal_str);
b225ea
     const char *signame = NULL;
b225ea
     bool signal_is_fatal_bool = signal_is_fatal(signal_no, &signame);
b225ea
+
b225ea
+    const char *pid_str = argv[3];
b225ea
+    /* xatoi_positive() handles errors */
b225ea
+    uid_t uid = xatoi_positive(argv[4]);
b225ea
+
b225ea
+    errno = 0;
b225ea
     off_t ulimit_c = strtoull(argv[2], NULL, 10);
b225ea
+    if (errno)
b225ea
+    {
b225ea
+        error_msg_ignore_crash(pid_str, NULL, (long unsigned)uid, signal_no,
b225ea
+                signame, "RLIMIT_CORE '%s' is bogus", argv[2]);
b225ea
+        xfunc_die();
b225ea
+    }
b225ea
+
b225ea
     if (ulimit_c < 0) /* unlimited? */
b225ea
     {
b225ea
         /* set to max possible >0 value */
b225ea
         ulimit_c = ~((off_t)1 << (sizeof(off_t)*8-1));
b225ea
     }
b225ea
-    const char *pid_str = argv[3];
b225ea
-    pid_t local_pid = xatoi_positive(argv[3]);
b225ea
-    uid_t uid = xatoi_positive(argv[4]);
b225ea
-    if (errno || local_pid <= 0)
b225ea
-    {
b225ea
-        perror_msg_and_die("PID '%s' or limit '%s' is bogus", argv[3], argv[2]);
b225ea
-    }
b225ea
+
b225ea
+    const char *global_pid_str = argv[8];
b225ea
+    pid_t pid = xatoi_positive(argv[8]);
b225ea
+
b225ea
+    user_pwd = get_cwd(pid); /* may be NULL on error */
b225ea
+    log_notice("user_pwd:'%s'", user_pwd);
b225ea
 
b225ea
     {
b225ea
         char *s = xmalloc_fopen_fgetline_fclose(VAR_RUN"/abrt/saved_core_pattern");
b225ea
@@ -825,8 +850,6 @@ int main(int argc, char** argv)
b225ea
         else
b225ea
             free(s);
b225ea
     }
b225ea
-    const char *global_pid_str = argv[8];
b225ea
-    pid_t pid = xatoi_positive(argv[8]);
b225ea
 
b225ea
     pid_t tid = 0;
b225ea
     if (argv[9])
b225ea
@@ -836,56 +859,24 @@ int main(int argc, char** argv)
b225ea
 
b225ea
     char path[PATH_MAX];
b225ea
 
b225ea
-    int src_fd_binary = -1;
b225ea
-    char *executable = get_executable(pid, setting_SaveBinaryImage ? &src_fd_binary : NULL);
b225ea
-    if (executable == NULL)
b225ea
-    {
b225ea
-        error_msg_not_process_crash(pid_str, NULL, (long unsigned)uid, signal_no,
b225ea
-                signame, "ignoring (can't read /proc/PID/exe link)");
b225ea
-
b225ea
-        xfunc_die();
b225ea
-    }
b225ea
-
b225ea
-    if (strstr(executable, "/abrt-hook-ccpp"))
b225ea
-    {
b225ea
-        error_msg_and_die("PID %lu is '%s', not dumping it to avoid recursion",
b225ea
-                        (long)pid, executable);
b225ea
-    }
b225ea
-
b225ea
-    const char *last_slash = strrchr(executable, '/');
b225ea
-    if (is_path_ignored(setting_ignored_paths, executable))
b225ea
-    {
b225ea
-        error_msg_not_process_crash(pid_str, last_slash + 1, (long unsigned)uid, signal_no,
b225ea
-                signame, "ignoring (listed in 'IgnoredPaths')");
b225ea
-
b225ea
-        return 0;
b225ea
-    }
b225ea
-
b225ea
-    /* dumping core for user, if allowed */
b225ea
-    if (setting_allowed_users || setting_allowed_groups)
b225ea
-    {
b225ea
-        if (setting_allowed_users && is_user_allowed(uid, setting_allowed_users))
b225ea
-            log_debug("User %lu is listed in 'AllowedUsers'", (long unsigned)uid);
b225ea
-        else if (setting_allowed_groups && is_user_in_allowed_group(uid, setting_allowed_groups))
b225ea
-            log_debug("User %lu is member of group listed in 'AllowedGroups'", (long unsigned)uid);
b225ea
-        else
b225ea
-        {
b225ea
-            error_msg_not_process_crash(pid_str, last_slash + 1, (long unsigned)uid, signal_no,
b225ea
-                signame, "ignoring (not allowed in 'AllowedUsers' nor 'AllowedGroups')");
b225ea
-
b225ea
-            xfunc_die();
b225ea
-        }
b225ea
-    }
b225ea
-
b225ea
-    user_pwd = get_cwd(pid);
b225ea
-    log_notice("user_pwd:'%s'", user_pwd);
b225ea
-
b225ea
     sprintf(path, "/proc/%lu/status", (long)pid);
b225ea
     char *proc_pid_status = xmalloc_xopen_read_close(path, /*maxsz:*/ NULL);
b225ea
 
b225ea
     uid_t fsuid = uid;
b225ea
     uid_t tmp_fsuid = get_fsuid(proc_pid_status);
b225ea
+    if (tmp_fsuid < 0)
b225ea
+    {
b225ea
+        error_msg_ignore_crash(pid_str, NULL, (long unsigned)uid, signal_no,
b225ea
+                signame, "Failed to parse /proc/%lu/status (Uid)", (long)pid);
b225ea
+        xfunc_die();
b225ea
+    }
b225ea
     const int fsgid = get_fsgid(proc_pid_status);
b225ea
+    if (fsgid < 0)
b225ea
+    {
b225ea
+        error_msg_ignore_crash(pid_str, NULL, (long unsigned)uid, signal_no,
b225ea
+                signame, "Failed to parse /proc/%lu/status (Gid)", (long)pid);
b225ea
+        xfunc_die();
b225ea
+    }
b225ea
 
b225ea
     int suid_policy = dump_suid_policy();
b225ea
     if (tmp_fsuid != uid)
b225ea
@@ -901,8 +892,7 @@ int main(int argc, char** argv)
b225ea
         }
b225ea
     }
b225ea
 
b225ea
-    /* If PrivateReports is on, root owns all problem directories */
b225ea
-    const uid_t dduid = g_settings_privatereports ? 0 : fsuid;
b225ea
+    snprintf(path, sizeof(path), "%s/last-ccpp", g_settings_dump_location);
b225ea
 
b225ea
     /* Open a fd to compat coredump, if requested and is possible */
b225ea
     int user_core_fd = -1;
b225ea
@@ -910,18 +900,72 @@ int main(int argc, char** argv)
b225ea
         /* note: checks "user_pwd == NULL" inside; updates core_basename */
b225ea
         user_core_fd = open_user_core(uid, fsuid, fsgid, pid, &argv[1]);
b225ea
 
b225ea
+    int src_fd_binary = -1;
b225ea
+    char *executable = get_executable(pid, setting_SaveBinaryImage ? &src_fd_binary : NULL);
b225ea
     if (executable == NULL)
b225ea
     {
b225ea
         /* readlink on /proc/$PID/exe failed, don't create abrt dump dir */
b225ea
-        error_msg("Can't read /proc/%lu/exe link", (long)pid);
b225ea
+        error_msg_ignore_crash(pid_str, NULL, (long unsigned)uid, signal_no,
b225ea
+                signame, "Can't read /proc/%lu/exe link", (long)pid);
b225ea
+
b225ea
+        xfunc_die();
b225ea
+    }
b225ea
+
b225ea
+    const char *last_slash = strrchr(executable, '/');
b225ea
+    /* if the last_slash was found, skip it */
b225ea
+    if (last_slash) ++last_slash;
b225ea
+
b225ea
+    if (is_path_ignored(setting_ignored_paths, executable))
b225ea
+    {
b225ea
+        error_msg_ignore_crash(pid_str, last_slash, (long unsigned)uid, signal_no,
b225ea
+                signame, "listed in 'IgnoredPaths'");
b225ea
+
b225ea
+        return 0;
b225ea
+    }
b225ea
+
b225ea
+    if (strstr(executable, "/abrt-hook-ccpp"))
b225ea
+    {
b225ea
+        error_msg_ignore_crash(pid_str, last_slash, (long unsigned)uid, signal_no,
b225ea
+                signame, "avoid recursion");
b225ea
+
b225ea
+        xfunc_die();
b225ea
+    }
b225ea
+
b225ea
+    /* Check /var/tmp/abrt/last-ccpp marker, do not dump repeated crashes
b225ea
+     * if they happen too often. Else, write new marker value.
b225ea
+     */
b225ea
+    if (check_recent_crash_file(path, executable))
b225ea
+    {
b225ea
+        error_msg_ignore_crash(pid_str, last_slash, (long unsigned)uid, signal_no,
b225ea
+                signame, "repeated crash");
b225ea
+
b225ea
+        /* It is a repeating crash */
b225ea
         return create_user_core(user_core_fd, pid, ulimit_c);
b225ea
     }
b225ea
 
b225ea
+    const bool abrt_crash = (last_slash && (strncmp(last_slash, "abrt", 4) == 0));
b225ea
+    if (abrt_crash && g_settings_debug_level == 0)
b225ea
+    {
b225ea
+        error_msg_ignore_crash(pid_str, last_slash, (long unsigned)uid, signal_no,
b225ea
+                signame, "'DebugLevel' == 0");
b225ea
+
b225ea
+        goto finito;
b225ea
+    }
b225ea
+
b225ea
+    /* unsupported signal */
b225ea
     if (!signal_is_fatal_bool)
b225ea
+    {
b225ea
+        error_msg_ignore_crash(pid_str, last_slash, (long unsigned)uid, signal_no,
b225ea
+                signame, "unsupported signal");
b225ea
+
b225ea
         return create_user_core(user_core_fd, pid, ulimit_c); // not a signal we care about
b225ea
+    }
b225ea
 
b225ea
     if (!daemon_is_ok())
b225ea
     {
b225ea
+        error_msg_ignore_crash(pid_str, last_slash, (long unsigned)uid, signal_no,
b225ea
+                signame, "abrtd is not running");
b225ea
+
b225ea
         /* not an error, exit with exit code 0 */
b225ea
         log("abrtd is not running. If it crashed, "
b225ea
             "/proc/sys/kernel/core_pattern contains a stale value, "
b225ea
@@ -930,32 +974,40 @@ int main(int argc, char** argv)
b225ea
         return create_user_core(user_core_fd, pid, ulimit_c);
b225ea
     }
b225ea
 
b225ea
+    /* dumping core for user, if allowed */
b225ea
+    if (setting_allowed_users || setting_allowed_groups)
b225ea
+    {
b225ea
+        if (setting_allowed_users && is_user_allowed(uid, setting_allowed_users))
b225ea
+            log_debug("User %lu is listed in 'AllowedUsers'", (long unsigned)uid);
b225ea
+        else if (setting_allowed_groups && is_user_in_allowed_group(uid, setting_allowed_groups))
b225ea
+            log_debug("User %lu is member of group listed in 'AllowedGroups'", (long unsigned)uid);
b225ea
+        else
b225ea
+        {
b225ea
+            error_msg_ignore_crash(pid_str, last_slash, (long unsigned)uid, signal_no,
b225ea
+                signame, "not allowed in 'AllowedUsers' nor 'AllowedGroups'");
b225ea
+
b225ea
+            xfunc_die();
b225ea
+        }
b225ea
+    }
b225ea
+
b225ea
+    /* low free space */
b225ea
     if (g_settings_nMaxCrashReportsSize > 0)
b225ea
     {
b225ea
         /* If free space is less than 1/4 of MaxCrashReportsSize... */
b225ea
         if (low_free_space(g_settings_nMaxCrashReportsSize, g_settings_dump_location))
b225ea
+        {
b225ea
+            error_msg_ignore_crash(pid_str, last_slash, (long unsigned)uid, signal_no,
b225ea
+                                    signame, "low free space");
b225ea
             return create_user_core(user_core_fd, pid, ulimit_c);
b225ea
+        }
b225ea
     }
b225ea
 
b225ea
-    /* Check /var/tmp/abrt/last-ccpp marker, do not dump repeated crashes
b225ea
-     * if they happen too often. Else, write new marker value.
b225ea
-     */
b225ea
-    snprintf(path, sizeof(path), "%s/last-ccpp", g_settings_dump_location);
b225ea
-    if (check_recent_crash_file(path, executable))
b225ea
-    {
b225ea
-        /* It is a repeating crash */
b225ea
-        return create_user_core(user_core_fd, pid, ulimit_c);
b225ea
-    }
b225ea
+    /* processing crash - inform user about it */
b225ea
+    error_msg_process_crash(pid_str, last_slash, (long unsigned)uid,
b225ea
+                signal_no, signame, "dumping core");
b225ea
 
b225ea
-    if (last_slash && strncmp(++last_slash, "abrt", 4) == 0)
b225ea
+    if (abrt_crash)
b225ea
     {
b225ea
-        if (g_settings_debug_level == 0)
b225ea
-        {
b225ea
-            log_warning("Ignoring crash of %s (SIG%s).",
b225ea
-                        executable, signame ? signame : signal_str);
b225ea
-            goto finito;
b225ea
-        }
b225ea
-
b225ea
         /* If abrtd/abrt-foo crashes, we don't want to create a _directory_,
b225ea
          * since that can make new copy of abrtd to process it,
b225ea
          * and maybe crash again...
b225ea
@@ -974,7 +1026,7 @@ int main(int argc, char** argv)
b225ea
              * but it does not log file name */
b225ea
             error_msg_and_die("Error saving '%s'", path);
b225ea
         }
b225ea
-        log("Saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size);
b225ea
+        log_notice("Saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size);
b225ea
         err = 0;
b225ea
         goto finito;
b225ea
     }
b225ea
@@ -986,6 +1038,9 @@ int main(int argc, char** argv)
b225ea
         return create_user_core(user_core_fd, pid, ulimit_c);
b225ea
     }
b225ea
 
b225ea
+    /* If PrivateReports is on, root owns all problem directories */
b225ea
+    const uid_t dduid = g_settings_privatereports ? 0 : fsuid;
b225ea
+
b225ea
     /* use dduid (either fsuid or 0) instead of uid, so we don't expose any
b225ea
      * sensitive information of suided app in /var/tmp/abrt
b225ea
      *
b225ea
-- 
b225ea
1.8.3.1
b225ea