Merge branch 'master' of git://git.fedorahosted.org/git/abrt

7 files changed, 244 insertions, 120 deletions

diff --git a/src/Hooks/Makefile.am b/src/Hooks/Makefile.am
index 7581971..2b04990 100644
--- a/src/Hooks/Makefile.am
+++ b/src/Hooks/Makefile.am
@@ -1,9 +1,10 @@
 libexec_PROGRAMS = abrt-hook-ccpp abrt-hook-python
 bin_PROGRAMS = dumpoops
 
-# CCpp
+# abrt-hook-ccpp
 abrt_hook_ccpp_SOURCES = \
-    CCpp.cpp
+    abrt-hook-ccpp.cpp \
+    hooklib.h hooklib.cpp
 abrt_hook_ccpp_CPPFLAGS = \
     -I$(srcdir)/../../inc \
     -I$(srcdir)/../../lib/Utils \
@@ -34,7 +35,8 @@ dumpoops_LDADD = \
 
 # abrt-hook-python
 abrt_hook_python_SOURCES = \
-    abrt-hook-python.cpp
+    abrt-hook-python.cpp \
+    hooklib.h hooklib.cpp
 abrt_hook_python_CPPFLAGS = \
     -I$(srcdir)/../../inc \
     -I$(srcdir)/../../lib/Utils \
diff --git a/src/Hooks/CCpp.cpp b/src/Hooks/abrt-hook-ccpp.cpp
index 3a13358..1c91dc8 100644
--- a/src/Hooks/CCpp.cpp
+++ b/src/Hooks/abrt-hook-ccpp.cpp
@@ -1,5 +1,5 @@
 /*
-    CCpp.cpp - the hook for C/C++ crashing program
+    abrt-hook-ccpp.cpp - the hook for C/C++ crashing program
 
     Copyright (C) 2009	Zdenek Prikryl (zprikryl@redhat.com)
     Copyright (C) 2009	RedHat inc.
@@ -19,6 +19,7 @@
     51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 */
 #include "abrtlib.h"
+#include "hooklib.h"
 #include "DebugDump.h"
 #include "ABRTException.h"
 #include <syslog.h>
@@ -44,17 +45,17 @@ static char* malloc_readlink(const char *linkname)
 
 static char* get_executable(pid_t pid)
 {
-    char buf[sizeof("/proc/%u/exe") + sizeof(int)*3];
+    char buf[sizeof("/proc/%lu/exe") + sizeof(long)*3];
 
-    sprintf(buf, "/proc/%u/exe", (int)pid);
+    sprintf(buf, "/proc/%lu/exe", (long)pid);
     return malloc_readlink(buf);
 }
 
 static char* get_cwd(pid_t pid)
 {
-    char buf[sizeof("/proc/%u/cwd") + sizeof(int)*3];
+    char buf[sizeof("/proc/%lu/cwd") + sizeof(long)*3];
 
-    sprintf(buf, "/proc/%u/cwd", (int)pid);
+    sprintf(buf, "/proc/%lu/cwd", (long)pid);
     return malloc_readlink(buf);
 }
 
@@ -63,12 +64,12 @@ int main(int argc, char** argv)
     int fd;
     struct stat sb;
 
-    const char* program_name = argv[0];
     if (argc < 5)
     {
+        const char* program_name = argv[0];
         error_msg_and_die("Usage: %s: DUMPDIR PID SIGNO UID CORE_SIZE_LIMIT", program_name);
     }
-    openlog("abrt", 0, LOG_DAEMON);
+    openlog("abrt", LOG_PID, LOG_DAEMON);
     logmode = LOGMODE_SYSLOG;
 
     errno = 0;
@@ -113,80 +114,31 @@ int main(int argc, char** argv)
         char* executable = get_executable(pid);
         if (executable == NULL)
         {
-            error_msg_and_die("can't read /proc/%u/exe link", (int)pid);
+            error_msg_and_die("can't read /proc/%lu/exe link", (long)pid);
         }
         if (strstr(executable, "/abrt-hook-ccpp"))
         {
-            error_msg_and_die("pid %u is '%s', not dumping it to avoid recursion",
-                            (int)pid, executable);
+            error_msg_and_die("pid %lu is '%s', not dumping it to avoid recursion",
+                            (long)pid, executable);
         }
 
         /* Parse abrt.conf and plugins/CCpp.conf */
         unsigned setting_MaxCrashReportsSize = 0;
         bool setting_MakeCompatCore = false;
-        bool abrt_conf = true;
-        FILE *fp = fopen(CONF_DIR"/abrt.conf", "r");
-        if (fp)
-        {
-            char line[256];
-            while (1)
-            {
-                if (fgets(line, sizeof(line), fp) == NULL)
-                {
-                    /* Next .conf file plz */
-                    if (abrt_conf)
-                    {
-                        abrt_conf = false;
-                        fp = fopen(CONF_DIR"/plugins/CCpp.conf", "r");
-                        if (fp)
-                            continue;
-                    }
-                    break;
-                }
+        parse_conf(CONF_DIR"/plugins/CCpp.conf", &setting_MaxCrashReportsSize, &setting_MakeCompatCore);
 
-                unsigned len = strlen(line);
-                if (len > 0 && line[len-1] == '\n')
-                    line[--len] = '\0';
-                const char *p = skip_whitespace(line);
-#undef DIRECTIVE
-#define DIRECTIVE "MaxCrashReportsSize"
-                if (strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)
-                {
-                    p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
-                    if (*p != '=')
-                        continue;
-                    p = skip_whitespace(p + 1);
-                    if (isdigit(*p))
-                        /* x1.25: go a bit up, so that usual in-daemon trimming
-                         * kicks in first, and we don't "fight" with it. */
-                        setting_MaxCrashReportsSize = (unsigned long)xatou(p) * 5 / 4;
-                    continue;
-                }
-#undef DIRECTIVE
-#define DIRECTIVE "MakeCompatCore"
-                if (strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)
-                {
-                    p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
-                    if (*p != '=')
-                        continue;
-                    p = skip_whitespace(p + 1);
-                    setting_MakeCompatCore = string_to_bool(p);
-                    continue;
-                }
-#undef DIRECTIVE
-                /* add more 'if (strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)' here... */
-            }
-            fclose(fp);
+        if (setting_MaxCrashReportsSize > 0)
+        {
+            check_free_space(setting_MaxCrashReportsSize);
         }
 
-
         char path[PATH_MAX];
 
         /* Check /var/cache/abrt/last-ccpp marker, do not dump repeated crashes
          * if they happen too often. Else, write new marker value.
          */
         snprintf(path, sizeof(path), "%s/last-ccpp", dddir);
-        fd = open(path, O_RDWR | O_CREAT, 0666);
+        fd = open(path, O_RDWR | O_CREAT, 0600);
         if (fd >= 0)
         {
             int sz;
@@ -232,7 +184,7 @@ int main(int argc, char** argv)
                  * but it does not log file name */
                 error_msg_and_die("error saving coredump to %s", path);
             }
-            log("saved core dump of pid %u (%s) to %s (%llu bytes)", (int)pid, executable, path, (long long)core_size);
+            log("saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size);
             return 0;
         }
 
@@ -240,7 +192,7 @@ int main(int argc, char** argv)
         const char *signame = strsignal(signal_no);
         char *reason = xasprintf("Process was terminated by signal %s (%s)", signal_str, signame ? signame : signal_str);
 
-        snprintf(path, sizeof(path), "%s/ccpp-%ld-%u", dddir, (long)time(NULL), (int)pid);
+        snprintf(path, sizeof(path), "%s/ccpp-%ld-%lu", dddir, (long)time(NULL), (long)pid);
         CDebugDump dd;
         dd.Create(path, uid);
         dd.SaveText(FILENAME_ANALYZER, "CCpp");
@@ -263,6 +215,10 @@ int main(int argc, char** argv)
             dd.Close();
             perror_msg_and_die("can't open '%s'", path);
         }
+//TODO: chown to uid:abrt?
+//Currently it is owned by 0:0 but is readable by anyone, so the owner
+//of the crashed binary still can access it, as he has
+//r-x access to the dump dir.
         core_size = copyfd_eof(STDIN_FILENO, core_fd);
         if (core_size < 0 || fsync(core_fd) != 0)
         {
@@ -275,7 +231,7 @@ int main(int argc, char** argv)
         }
         lseek(core_fd, 0, SEEK_SET);
         /* note: core_fd is still open, we may use it later to copy core to user's dir */
-        log("saved core dump of pid %u (%s) to %s (%llu bytes)", (int)pid, executable, path, (long long)core_size);
+        log("saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size);
         free(executable);
         free(cmdline);
         path[len] = '\0'; /* path now contains directory name */
@@ -288,30 +244,10 @@ int main(int argc, char** argv)
          */
         dd.Close();
 
-        /* rhbz#539551: "abrt going crazy when crashing process is respawned".
-         * Do we want to protect against or ameliorate this? How? Ideas:
-         * (1) nice ourself?
-         * (2) check total size of dump dir, if it overflows, either abort dump
-         *     or delete oldest/biggest dumps? [abort would be simpler,
-         *     abrtd already does "delete on overflow" thing]
-         * (3) detect parallel dumps in progress and back off
-         *     (pause/renice further/??)
-         */
-
+        /* rhbz#539551: "abrt going crazy when crashing process is respawned" */
         if (setting_MaxCrashReportsSize > 0)
         {
-            string worst_dir;
-            while (1)
-            {
-                const char *base_dirname = strrchr(path, '/') + 1; /* never NULL */
-                /* We exclude our own dump from candidates for deletion (3rd param): */
-                double dirsize = get_dirsize_find_largest_dir(DEBUG_DUMPS_DIR, &worst_dir, base_dirname);
-                if (dirsize / (1024*1024) < setting_MaxCrashReportsSize || worst_dir == "")
-                    break;
-                log("size of '%s' >= %u MB, deleting '%s'", DEBUG_DUMPS_DIR, setting_MaxCrashReportsSize, worst_dir.c_str());
-                delete_debug_dump_dir(concat_path_file(DEBUG_DUMPS_DIR, worst_dir.c_str()).c_str());
-                worst_dir = "";
-            }
+            trim_debug_dumps(setting_MaxCrashReportsSize, path);
         }
 
         if (!setting_MakeCompatCore)
@@ -349,7 +285,7 @@ int main(int argc, char** argv)
     }
 
     /* Mimic "core.PID" if requested */
-    char core_basename[sizeof("core.%u") + sizeof(int)*3] = "core";
+    char core_basename[sizeof("core.%lu") + sizeof(long)*3] = "core";
     char buf[] = "0\n";
     fd = open("/proc/sys/kernel/core_uses_pid", O_RDONLY);
     if (fd >= 0)
@@ -359,7 +295,7 @@ int main(int argc, char** argv)
     }
     if (strcmp(buf, "1\n") == 0)
     {
-        sprintf(core_basename, "core.%u", (int)pid);
+        sprintf(core_basename, "core.%lu", (long)pid);
     }
 
     /* man core:
@@ -425,7 +361,7 @@ int main(int argc, char** argv)
         unlink(core_basename);
         return 1;
     }
-    log("saved core dump of pid %u to %s/%s (%llu bytes)", (int)pid, user_pwd, core_basename, (long long)size);
+    log("saved core dump of pid %lu to %s/%s (%llu bytes)", (long)pid, user_pwd, core_basename, (long long)size);
 
     return 0;
 }
diff --git a/src/Hooks/abrt-hook-python.cpp b/src/Hooks/abrt-hook-python.cpp
index 3f79d28..c8a25e3 100644
--- a/src/Hooks/abrt-hook-python.cpp
+++ b/src/Hooks/abrt-hook-python.cpp
@@ -20,27 +20,51 @@
 */
 
 #include <getopt.h>
-#include <unistd.h>
+#include <syslog.h>
 /* We can easily get rid of abrtlib (libABRTUtils.so) usage in this file,
  * but DebugDump will pull it in anyway */
 #include "abrtlib.h"
+#include "hooklib.h"
 #include "DebugDump.h"
+#include "ABRTException.h"
 #if HAVE_CONFIG_H
 # include <config.h>
 #endif
 
 #define MAX_BT_SIZE (1024*1024)
+#define MAX_BT_SIZE_STR "1 MB"
 
 static char *pid;
 static char *executable;
 static char *uuid;
 
-int main(int argc, char** argv)
+/* Note: "" will return false */
+static bool isxdigit_str(const char *str)
 {
-  // Error if daemon is not running.
-  if (!daemon_is_ok())
-    error_msg_and_die("Daemon is not running.");
+  do {
+    if ((*str < '0' || *str > '9') /* not a digit */
+     && ((*str | 0x20) < 'a' || (*str | 0x20) > 'f') /* not A-F or a-f */
+    )
+    {
+      return false;
+    }
+    str++;
+  } while (*str);
+  return true;
+}
+
+static bool printable_str(const char *str)
+{
+  do {
+    if ((unsigned char)(*str) < ' ' || *str == 0x7f)
+      return false;
+    str++;
+  } while (*str);
+  return true;
+}
 
+int main(int argc, char** argv)
+{
   // Parse options
   static const struct option longopts[] = {
     // name       , has_arg          , flag, val
@@ -74,49 +98,73 @@ int main(int argc, char** argv)
       );
     }
   }
-  if (!pid)
+  if (!pid || !executable || !uuid)
+    goto usage;
+  if (strlen(uuid) > 128 || !isxdigit_str(uuid))
+    goto usage;
+  if (strlen(executable) > PATH_MAX || !printable_str(executable))
     goto usage;
-// is it really ok if other params aren't specified? abrtd might get confused...
+  // pid string is sanitized later by xatou()
+
+  openlog("abrt", LOG_PID, LOG_DAEMON);
+  logmode = LOGMODE_SYSLOG;
+
+  // Error if daemon is not running
+  if (!daemon_is_ok())
+    error_msg_and_die("daemon is not running, python crash dump aborted");
+
+  unsigned setting_MaxCrashReportsSize = 0;
+  parse_conf(NULL, &setting_MaxCrashReportsSize, NULL);
+  if (setting_MaxCrashReportsSize > 0)
+  {
+    check_free_space(setting_MaxCrashReportsSize);
+  }
 
   // Read the backtrace from stdin
   char *bt = (char*)xmalloc(MAX_BT_SIZE);
   ssize_t len = full_read(STDIN_FILENO, bt, MAX_BT_SIZE-1);
   if (len < 0)
   {
-    perror_msg_and_die("Read error");
+    perror_msg_and_die("read error");
   }
   bt[len] = '\0';
   if (len == MAX_BT_SIZE-1)
   {
-    error_msg("Backtrace size limit exceeded, trimming to 1 MB");
+    error_msg("backtrace size limit exceeded, trimming to " MAX_BT_SIZE_STR);
   }
 
+  // This also checks that pid is a valid numeric string
+  char *cmdline = get_cmdline(xatou(pid)); /* never NULL */
+
   // Create directory with the debug dump
   char path[PATH_MAX];
   snprintf(path, sizeof(path), DEBUG_DUMPS_DIR"/pyhook-%ld-%s",
 	   (long)time(NULL), pid);
-
   CDebugDump dd;
-  dd.Create(path, geteuid());
-  dd.SaveText(FILENAME_ANALYZER, "Python");
-  if (executable)
-    dd.SaveText(FILENAME_EXECUTABLE, executable);
+  try {
+    dd.Create(path, getuid());
+  } catch (CABRTException &e) {
+    error_msg_and_die("error while creating crash dump %s: %s", path, e.what());
+  }
 
-  pid_t pidt = xatoi(pid);
-  char *cmdline = get_cmdline(pidt);
+  dd.SaveText(FILENAME_ANALYZER, "Python");
+  dd.SaveText(FILENAME_EXECUTABLE, executable);
+  dd.SaveText("backtrace", bt);
+  free(bt);
   dd.SaveText("cmdline", cmdline);
   free(cmdline);
-
-  if (uuid)
-    dd.SaveText("uuid", uuid);
-
-  char uid[sizeof(int) * 3 + 2];
-  sprintf(uid, "%d", (int)getuid());
+  dd.SaveText("uuid", uuid);
+  char uid[sizeof(long) * 3 + 2];
+  sprintf(uid, "%lu", (long)getuid());
   dd.SaveText("uid", uid);
 
-  dd.SaveText("backtrace", bt);
-  free(bt);
   dd.Close();
+  log("saved python crash dump of pid %s to %s", pid, path);
+
+  if (setting_MaxCrashReportsSize > 0)
+  {
+    trim_debug_dumps(setting_MaxCrashReportsSize, path);
+  }
 
   return 0;
 }
diff --git a/src/Hooks/abrt_exception_handler.py.in b/src/Hooks/abrt_exception_handler.py.in
index a0b0519..e0e1954 100644
--- a/src/Hooks/abrt_exception_handler.py.in
+++ b/src/Hooks/abrt_exception_handler.py.in
@@ -213,7 +213,7 @@ def handleMyException((etype, value, tb)):
         text += ("\nLocal variables in innermost frame:\n")
         try:
             for (key, val) in frame.f_locals.items():
-                text += "%s: %s\n" % (key, val)
+                text += "%s: %s\n" % (key, repr(val))
         except:
             pass
 
diff --git a/src/Hooks/dumpoops.cpp b/src/Hooks/dumpoops.cpp
index 4b6778d..01e65c4 100644
--- a/src/Hooks/dumpoops.cpp
+++ b/src/Hooks/dumpoops.cpp
@@ -83,6 +83,7 @@ int main(int argc, char **argv)
 	void *handle;
 
 	errno = 0;
+//TODO: use it directly, not via dlopen?
 	handle = dlopen(PLUGINS_LIB_DIR"/libKerneloopsScanner.so", RTLD_NOW);
 	if (!handle)
 		perror_msg_and_die("can't load %s", PLUGINS_LIB_DIR"/libKerneloopsScanner.so");
diff --git a/src/Hooks/hooklib.cpp b/src/Hooks/hooklib.cpp
new file mode 100644
index 0000000..41b9627
--- /dev/null
+++ b/src/Hooks/hooklib.cpp
@@ -0,0 +1,116 @@
+/*
+    Copyright (C) 2009	RedHat inc.
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License along
+    with this program; if not, write to the Free Software Foundation, Inc.,
+    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+*/
+#include "abrtlib.h"
+#include "hooklib.h"
+#include "DebugDump.h"
+#include <sys/statvfs.h>
+
+using namespace std;
+
+void parse_conf(const char *additional_conf, unsigned *setting_MaxCrashReportsSize, bool *setting_MakeCompatCore)
+{
+    FILE *fp = fopen(CONF_DIR"/abrt.conf", "r");
+    if (!fp)
+        return;
+
+    char line[256];
+    while (1)
+    {
+        if (fgets(line, sizeof(line), fp) == NULL)
+        {
+            fclose(fp);
+            if (additional_conf)
+            {
+                /* Next .conf file plz */
+                fp = fopen(additional_conf, "r");
+                if (fp)
+                {
+                    additional_conf = NULL;
+                    continue;
+                }
+            }
+            break;
+        }
+
+        unsigned len = strlen(line);
+        if (len > 0 && line[len-1] == '\n')
+            line[--len] = '\0';
+        const char *p = skip_whitespace(line);
+#undef DIRECTIVE
+#define DIRECTIVE "MaxCrashReportsSize"
+        if (setting_MaxCrashReportsSize && strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)
+        {
+            p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
+            if (*p != '=')
+                continue;
+            p = skip_whitespace(p + 1);
+            if (isdigit(*p))
+            {
+                /* x1.25: go a bit up, so that usual in-daemon trimming
+                 * kicks in first, and we don't "fight" with it. */
+                *setting_MaxCrashReportsSize = (unsigned long)xatou(p) * 5 / 4;
+            }
+            continue;
+        }
+#undef DIRECTIVE
+#define DIRECTIVE "MakeCompatCore"
+        if (setting_MakeCompatCore && strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)
+        {
+            p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
+            if (*p != '=')
+                continue;
+            p = skip_whitespace(p + 1);
+            *setting_MakeCompatCore = string_to_bool(p);
+            continue;
+        }
+#undef DIRECTIVE
+        /* add more 'if (strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)' here... */
+    }
+}
+
+void check_free_space(unsigned setting_MaxCrashReportsSize)
+{
+    /* Check that at least MaxCrashReportsSize/4 MBs are free. */
+    struct statvfs vfs;
+    if (statvfs(DEBUG_DUMPS_DIR, &vfs) != 0
+     || (vfs.f_bfree / (1024*1024 / 4)) * vfs.f_bsize < setting_MaxCrashReportsSize
+    ) {
+        error_msg_and_die("Low free disk space detected, aborting dump");
+    }
+}
+
+/* rhbz#539551: "abrt going crazy when crashing process is respawned".
+ * Check total size of dump dir, if it overflows,
+ * delete oldest/biggest dumps.
+ */
+void trim_debug_dumps(unsigned setting_MaxCrashReportsSize, const char *exclude_path)
+{
+    int count = 10;
+    string worst_dir;
+    while (--count >= 0)
+    {
+        const char *base_dirname = strrchr(exclude_path, '/') + 1; /* never NULL */
+        /* We exclude our own dump from candidates for deletion (3rd param): */
+        double dirsize = get_dirsize_find_largest_dir(DEBUG_DUMPS_DIR, &worst_dir, base_dirname);
+        if (dirsize / (1024*1024) < setting_MaxCrashReportsSize || worst_dir == "")
+            break;
+        log("size of '%s' >= %u MB, deleting '%s'", DEBUG_DUMPS_DIR, setting_MaxCrashReportsSize, worst_dir.c_str());
+        delete_debug_dump_dir(concat_path_file(DEBUG_DUMPS_DIR, worst_dir.c_str()).c_str());
+        worst_dir = "";
+    }
+}
diff --git a/src/Hooks/hooklib.h b/src/Hooks/hooklib.h
new file mode 100644
index 0000000..0794ff6
--- /dev/null
+++ b/src/Hooks/hooklib.h
@@ -0,0 +1,21 @@
+/*
+    Copyright (C) 2009	RedHat inc.
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License along
+    with this program; if not, write to the Free Software Foundation, Inc.,
+    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+*/
+
+void parse_conf(const char *additional_conf, unsigned *setting_MaxCrashReportsSize, bool *setting_MakeCompatCore);
+void check_free_space(unsigned setting_MaxCrashReportsSize);
+void trim_debug_dumps(unsigned setting_MaxCrashReportsSize, const char *exclude_path);