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

5 files changed, 133 insertions, 50 deletions

diff --git a/src/Daemon/Daemon.cpp b/src/Daemon/Daemon.cpp
index c6cae5de..8038f75d 100644
--- a/src/Daemon/Daemon.cpp
+++ b/src/Daemon/Daemon.cpp
@@ -110,7 +110,7 @@ typedef struct cron_callback_data_t
 } cron_callback_data_t;
 
 
-static uint8_t s_sig_caught; /* must be one byte */
+static volatile sig_atomic_t s_sig_caught;
 static int s_signal_pipe[2];
 static int s_signal_pipe_write = -1;
 static unsigned s_timeout;
@@ -216,14 +216,14 @@ static int SetUpCron()
             std::string sM = "";
 
             sH = it_c->first.substr(0, pos);
-            nH = atoi(sH.c_str());
+            nH = xatou(sH.c_str());
             nH = nH > 23 ? 23 : nH;
             nH = nH < 0 ? 0 : nH;
             nM = nM > 59 ? 59 : nM;
             nM = nM < 0 ? 0 : nM;
             timeout += nH * 60 * 60;
             sM = it_c->first.substr(pos + 1);
-            nM = atoi(sM.c_str());
+            nM = xatou(sM.c_str());
             timeout += nM * 60;
         }
         else
@@ -231,7 +231,7 @@ static int SetUpCron()
             std::string sS = "";
 
             sS = it_c->first;
-            nS = atoi(sS.c_str());
+            nS = xatou(sS.c_str());
             nS = nS <= 0 ? 1 : nS;
             timeout = nS;
         }
@@ -399,10 +399,15 @@ static int Lock()
 
 static void handle_fatal_signal(int signo)
 {
-    s_sig_caught = signo;
-    VERB3 log("Got signal %d", signo);
+    // Enable for debugging only, malloc/printf are unsafe in signal handlers
+    //VERB3 log("Got signal %d", signo);
+
+    uint8_t l_sig_caught;
+    s_sig_caught = l_sig_caught = signo;
+    /* Using local copy of s_sig_caught so that concurrent signal
+     * won't change it under us */
     if (s_signal_pipe_write >= 0)
-        write(s_signal_pipe_write, &s_sig_caught, 1);
+        write(s_signal_pipe_write, &l_sig_caught, 1);
 }
 
 /* Signal pipe handler */
diff --git a/src/Daemon/MiddleWare.cpp b/src/Daemon/MiddleWare.cpp
index b597a411..6b8bfcce 100644
--- a/src/Daemon/MiddleWare.cpp
+++ b/src/Daemon/MiddleWare.cpp
@@ -477,7 +477,7 @@ report_status_t Report(const map_crash_report_t& pCrashReport,
 
                     if (pUID != "")
                     {
-                        home = get_home_dir(atoi(pUID.c_str()));
+                        home = get_home_dir(xatoi_u(pUID.c_str()));
                         if (home != "")
                         {
                             oldSettings = reporter->GetSettings();
@@ -563,11 +563,38 @@ void LoadOpenGPGPublicKey(const char* key)
  * @param pDebugDumpDir A debugdump dir containing all necessary data.
  * @return It return results of operation. See mw_result_t.
  */
-static mw_result_t SavePackageDescriptionToDebugDump(const char *pExecutable,
-                                                     const char *pDebugDumpDir)
+static char *get_argv1_if_full_path(const char* cmdline)
+{
+    char *argv1 = (char*) strchr(cmdline, ' ');
+    if (argv1 != NULL)
+    {
+        /* we found space in cmdline, so it might contain
+         * path to some script like:
+         * /usr/bin/python /usr/bin/system-control-network
+         */
+        argv1++;
+        /* if the string following the space doesn't start
+         * with '/' it's probably not a full path to script
+         * and we can't use it to determine the package name
+         */
+        if (*argv1 != '/')
+        {
+            return NULL;
+        }
+        int len = strchrnul(argv1, ' ') - argv1;
+        /* cut the cmdline arguments */
+        argv1 = xstrndup(argv1, len);
+    }
+    return argv1;
+}
+static mw_result_t SavePackageDescriptionToDebugDump(
+                const char *pExecutable,
+                const char *cmdline,
+                const char *pDebugDumpDir)
 {
     std::string package;
     std::string packageName;
+    std::string scriptName; /* only if "interpreter /path/to/script" */
 
     if (strcmp(pExecutable, "kernel") == 0)
     {
@@ -582,6 +609,42 @@ static mw_result_t SavePackageDescriptionToDebugDump(const char *pExecutable,
             return MW_PACKAGE_ERROR;
         }
 
+        /* Check well-known interpreter names */
+
+        const char *basename = strrchr(pExecutable, '/');
+        if (basename) basename++; else basename = pExecutable;
+
+        /* Add "perl" and such as needed */
+        if (strcmp(basename, "python") == 0)
+        {
+// TODO: we don't verify that python executable is not modified
+// or that python package is properly signed
+// (see CheckFingerprint/CheckHash below)
+
+            /* Try to find package for the script by looking at argv[1].
+             * This will work only of the cmdline contains the whole path.
+             * Example: python /usr/bin/system-control-network
+             */
+            char *script_name = get_argv1_if_full_path(cmdline);
+            if (script_name)
+            {
+                char *script_pkg = GetPackage(script_name);
+                if (script_pkg)
+                {
+                    /* There is a well-formed script name in argv[1],
+                     * and it does belong to some package.
+                     * Replace interpreter's rpm_pkg and pExecutable
+                     * with data pertaining to the script.
+                     */
+                    free(rpm_pkg);
+                    rpm_pkg = script_pkg;
+                    scriptName = script_name;
+                    pExecutable = scriptName.c_str();
+                }
+                free(script_name);
+            }
+        }
+
         package = rpm_pkg;
         packageName = package.substr(0, package.rfind("-", package.rfind("-") - 1));
         VERB2 log("Package:'%s' short:'%s'", rpm_pkg, packageName.c_str());
@@ -610,10 +673,7 @@ static mw_result_t SavePackageDescriptionToDebugDump(const char *pExecutable,
     }
 
     std::string description = GetDescription(packageName.c_str());
-//TODO: if executable in /usr/bin/python, /bin/sh and such,
-//we need to extract component using argv[1]
     std::string component = GetComponent(pExecutable);
-
     try
     {
         CDebugDump dd;
@@ -788,6 +848,7 @@ mw_result_t SaveDebugDump(const char *pDebugDumpDir,
     std::string time;
     std::string analyzer;
     std::string executable;
+    std::string cmdline;
     try
     {
         CDebugDump dd;
@@ -796,6 +857,7 @@ mw_result_t SaveDebugDump(const char *pDebugDumpDir,
         dd.LoadText(FILENAME_UID, UID);
         dd.LoadText(FILENAME_ANALYZER, analyzer);
         dd.LoadText(FILENAME_EXECUTABLE, executable);
+        dd.LoadText(FILENAME_CMDLINE, cmdline);
     }
     catch (CABRTException& e)
     {
@@ -811,7 +873,8 @@ mw_result_t SaveDebugDump(const char *pDebugDumpDir,
     {
         return MW_IN_DB;
     }
-    mw_result_t res = SavePackageDescriptionToDebugDump(executable.c_str(), pDebugDumpDir);
+
+    mw_result_t res = SavePackageDescriptionToDebugDump(executable.c_str(), cmdline.c_str(), pDebugDumpDir);
     if (res != MW_OK)
     {
         return res;
diff --git a/src/Daemon/PluginManager.cpp b/src/Daemon/PluginManager.cpp
index 82f22e85..b6a8b154 100644
--- a/src/Daemon/PluginManager.cpp
+++ b/src/Daemon/PluginManager.cpp
@@ -391,7 +391,7 @@ void CPluginManager::SetPluginSettings(const char *pName,
         return;
     }
 
-    std::string home = get_home_dir(atoi(pUID.c_str()));
+    std::string home = get_home_dir(xatoi_u(pUID.c_str()));
     if (home == "")
     {
         return;
@@ -399,7 +399,7 @@ void CPluginManager::SetPluginSettings(const char *pName,
 
     std::string confDir = home + "/.abrt";
     std::string confPath = confDir + "/" + pName + "."PLUGINS_CONF_EXTENSION;
-    uid_t uid = atoi(pUID.c_str());
+    uid_t uid = xatoi_u(pUID.c_str());
     struct passwd* pw = getpwuid(uid);
     gid_t gid = pw ? pw->pw_gid : uid;
 
@@ -461,7 +461,7 @@ map_plugin_settings_t CPluginManager::GetPluginSettings(const char *pName,
               /*
             if (abrt_plugin->second->GetType() == REPORTER)
             {
-                std::string home = get_home_dir(atoi(pUID.c_str()));
+                std::string home = get_home_dir(xatoi_u(pUID.c_str()));
                 if (home != "")
                 {
                     LoadPluginSettings(home + "/.abrt/" + pName + "."PLUGINS_CONF_EXTENSION, ret);
diff --git a/src/Daemon/Settings.cpp b/src/Daemon/Settings.cpp
index 327851f5..d89bebfd 100644
--- a/src/Daemon/Settings.cpp
+++ b/src/Daemon/Settings.cpp
@@ -176,7 +176,7 @@ static void ParseCommon()
     it = s_mapSectionCommon.find("MaxCrashReportsSize");
     if (it != end)
     {
-        g_settings_nMaxCrashReportsSize = atoi(it->second.c_str());
+        g_settings_nMaxCrashReportsSize = xatoi_u(it->second.c_str());
     }
     it = s_mapSectionCommon.find("ActionsAndReporters");
     if (it != end)
diff --git a/src/Daemon/abrt-debuginfo-install b/src/Daemon/abrt-debuginfo-install
index 6278b63c..dcd3addb 100755
--- a/src/Daemon/abrt-debuginfo-install
+++ b/src/Daemon/abrt-debuginfo-install
@@ -21,8 +21,7 @@
 # Output goes to GUI as debuginfo install log. The script should be careful
 # to give useful, but not overly cluttered info to stdout.
 # Additionally, abrt daemon handles "MISSING:xxxx" messages specially:
-# xxxx will be prepended to backtrace. This is used to inform about
-# missing debuginfos.
+# it is used to inform about missing debuginfos.
 #
 # Exitcodes:
 # 0 - all debuginfos are installed
@@ -46,36 +45,22 @@
 # For better debuggability, eu_unstrip.OUT, yum_provides.OUT etc files
 # are saved in TEMPDIR, and TEMPDIR is not deleted if we exit with exitcode 2
 # ("serious problem").
-#
-# In the future, we may want to use a separate CACHEDIR (say, /var/cache/abrt-di)
-# and use it with this gdb command:
-#   set debug-file-directory /usr/lib/debug/.build-id:CACHEDIR/usr/lib/debug/.build-id
-# but current gdb can't handle DIR1:DIR2.
-# So, currently we are called with CACHEDIR set to "/", and don't pass
-# "set debug-file-directory" to gdb.
-# This is ugly, since it messes up /usr/lib/debug/.build-id over time
-# by piling up debuginfos there without any means to control their amount,
-# but it's the only way to make it work with current gdb.
-
-
-core=$1
-tempdir=$2
+
+
+core="$1"
+tempdir="$2"
 debuginfodirs="${3//:/ }"
 cachedir="${3%%:*}"
 debug=false
 
-exec 2>&1
-
-test -f "$core" || exit 2
-# cachedir is optional
-test x"$cachedir" = x"" || test -d "$cachedir" || exit 2
-# tempdir must not exist
-test -e "$tempdir" && exit 2
 
-mkdir -- "$tempdir" || exit 2
-cd "$tempdir" || exit 2
-$debug && echo "Installing rpms to $tempdir"
+# stderr may be used for status messages too
+exec 2>&1
 
+error_msg_and_die() {
+    echo "$*"
+    exit 2
+}
 
 count_words() {
     echo $#
@@ -92,6 +77,9 @@ print_missing_build_ids() {
 	build_id2=${build_id:2}
 	file="usr/lib/debug/.build-id/$build_id1/$build_id2.debug"
 	test -f "/$file" && continue
+	# On 2nd pass, we may already have some debuginfos in tempdir
+	test -f "$tempdir/$file" && continue
+	# Check cachedir if we have one
 	for d in $debuginfodirs; do
 	    test -f "$d/$file" && continue 2
 	done
@@ -99,6 +87,7 @@ print_missing_build_ids() {
     done
 }
 
+# Note: it is run in `backticks`, use >&2 for error messages
 print_missing_debuginfos() {
     local build_id
     local build_id1
@@ -110,6 +99,9 @@ print_missing_debuginfos() {
 	build_id2=${build_id:2}
 	file="usr/lib/debug/.build-id/$build_id1/$build_id2.debug"
 	test -f "/$file" && continue
+	# On 2nd pass, we may already have some debuginfos in tempdir
+	test -f "$tempdir/$file" && continue
+	# Check cachedir if we have one
 	if test x"$cachedir" != x""; then
 	    for d in $debuginfodirs; do
 		test -f "$d/$file" && continue 2
@@ -138,6 +130,7 @@ cleanup_and_report_missing() {
 }
 
 # $1: iteration (1,2...)
+# Note: it is run in `backticks`, use >&2 for error messages
 print_package_names() {
     # We'll run something like:
     #   yum --enablerepo=*debuginfo* --quiet provides \
@@ -154,8 +147,9 @@ print_package_names() {
     echo "$cmd" >"yum_provides.$1.OUT"
     local yum_provides_OUT="`$cmd 2>&1`"
     local err=$?
-    printf "%s\nexitcode:%s\n" "$yum_provides_OUT" $err >>"yum_provides.$1.OUT"
-    test $err = 0 || exit 2
+    printf "%s\nyum exitcode:%s\n" "$yum_provides_OUT" $err >>"yum_provides.$1.OUT"
+    test $err = 0 || error_msg_and_die "yum provides... exited with $err:
+`head yum_provides.$1.OUT`" >&2
 
     # The output is pretty machine-unfriendly:
     #   glibc-debuginfo-2.10.90-24.x86_64 : Debug information for package glibc
@@ -190,7 +184,7 @@ download_packages() {
     ##yumdownloader --enablerepo=*debuginfo* --quiet $packages >yumdownloader.OUT 2>&1
     ##err=$?
     ##echo "exitcode:$err" >>yumdownloader.OUT
-    ##test $err = 0 || exit 2
+    ##test $err = 0 || error_msg_and_die ...
     >yumdownloader.OUT
     i=1
     for pkg in $packages; do
@@ -207,10 +201,11 @@ download_packages() {
     for file in *.rpm; do
 	# Happens if no .rpm's were downloaded (yumdownloader problem)
 	# In this case, $f is the literal "*.rpm" string
-	test -f "$file" || exit 2
+	test -f "$file" || error_msg_and_die "not a rpm file: '$file'"
 	echo "Unpacking: $file"
 	echo "Processing: $file" >>unpack.OUT
 	rpm2cpio <"$file" 2>>unpack.OUT | cpio -id >>unpack.OUT 2>&1
+#TODO: error check?
     done
 
     # Copy debuginfo files to cachedir
@@ -235,6 +230,7 @@ download_packages() {
 		# Note: this does not preserve symlinks. This is intentional
 		$debug && echo Copying "$file" to "$cachedir/$file" >&2
 		cp --remove-destination "$file" "$cachedir/$file"
+#TODO: error check?
 		continue
 	    fi
 	done
@@ -242,6 +238,18 @@ download_packages() {
 }
 
 
+# Sanity checking
+test -f "$core" || error_msg_and_die "not a file: '$core'"
+# cachedir is optional
+test x"$cachedir" = x"" || test -d "$cachedir" || error_msg_and_die "bad cachedir '$cachedir'"
+# tempdir must not exist
+test -e "$tempdir" && error_msg_and_die "tempdir exists: '$tempdir'"
+
+mkdir -- "$tempdir" || exit 2
+cd "$tempdir" || exit 2
+$debug && echo "Installing rpms to $tempdir"
+
+
 # eu-unstrip output example:
 # 0x400000+0x209000 23c77451cf6adff77fc1f5ee2a01d75de6511dda@0x40024c - - [exe]
 #   or
@@ -254,8 +262,10 @@ echo "Getting list of build IDs"
 # eu-unstrip: /var/cache/abrt/ccpp-1256301004-2754/coredump: Callback returned failure
 eu_unstrip_OUT=`eu-unstrip "--core=$core" -n 2>eu_unstrip.ERR`
 err=$?
-printf "%s\nexitcode:%s\n" "$eu_unstrip_OUT" $err >eu_unstrip.OUT
-test $err = 0 || exit 2
+printf "%s\neu-unstrip exitcode:%s\n" "$eu_unstrip_OUT" $err >eu_unstrip.OUT
+test $err = 0 || error_msg_and_die "eu-unstrip exited with $err:
+`cat eu_unstrip.ERR`
+`head eu_unstrip.OUT`"
 
 # Get space-separated list of all build-ids
 # There can be duplicates (observed in real world)
@@ -283,6 +293,8 @@ iter=0
 while test $((++iter)) -le 2; do
     # Analyze $build_ids and check which debuginfos are present
     missing_debuginfo_files=`print_missing_debuginfos`
+    # Did print_missing_debuginfos fail?
+    test $? = 0 || exit 2
     $debug && echo "missing_debuginfo_files:$missing_debuginfo_files"
 
     test x"$missing_debuginfo_files" = x"" && break
@@ -290,6 +302,8 @@ while test $((++iter)) -le 2; do
     # Map $missing_debuginfo_files to package names.
     # yum is run here.
     packages=`print_package_names $iter`
+    # Did print_package_names fail?
+    test $? = 0 || exit 2
     $debug && echo "packages ($iter):$packages"
 
     # yum may return "" here if it found no packages (say, if coredump
@@ -304,4 +318,5 @@ done
 cleanup_and_report_missing
 
 test x"$missing_build_ids" != x"" && exit 1
+echo "All needed debuginfos are present"
 exit 0