From 658622eb5e1b81d394f066df44bc9f0abe9cc807 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 11 Jan 2010 07:18:56 +0100 Subject: RunApp: safer chdir. Overhauled "sparn a child and get its output" in general Signed-off-by: Denys Vlasenko --- lib/Plugins/CCpp.cpp | 70 +++++++++++++++++----------------------------------- 1 file changed, 22 insertions(+), 48 deletions(-) (limited to 'lib/Plugins/CCpp.cpp') diff --git a/lib/Plugins/CCpp.cpp b/lib/Plugins/CCpp.cpp index 1a01c01..26562be 100644 --- a/lib/Plugins/CCpp.cpp +++ b/lib/Plugins/CCpp.cpp @@ -99,54 +99,28 @@ static string concat_str_vector(char **strings) /* Returns status. See `man 2 wait` for status information. */ static int ExecVP(char **pArgs, uid_t uid, int redirect_stderr, string& pOutput) { - int pipeout[2]; - pid_t child; - - xpipe(pipeout); - child = fork(); - if (child == -1) - { - perror_msg_and_die("fork"); - } - if (child == 0) - { - VERB1 log("Executing: %s", concat_str_vector(pArgs).c_str()); - close(pipeout[0]); /* read side of the pipe */ - xmove_fd(pipeout[1], STDOUT_FILENO); - /* Make sure stdin is safely open to nothing */ - xmove_fd(xopen("/dev/null", O_RDONLY), STDIN_FILENO); - - struct passwd* pw = getpwuid(uid); - gid_t gid = pw ? pw->pw_gid : uid; - setgroups(1, &gid); - xsetregid(gid, gid); - xsetreuid(uid, uid); - setsid(); - - /* Nuke everything which may make setlocale() switch to non-POSIX locale: - * we need to avoid having gdb output in some obscure language. - */ - unsetenv("LANG"); - unsetenv("LC_ALL"); - unsetenv("LC_COLLATE"); - unsetenv("LC_CTYPE"); - unsetenv("LC_MESSAGES"); - unsetenv("LC_MONETARY"); - unsetenv("LC_NUMERIC"); - unsetenv("LC_TIME"); - - if (redirect_stderr) - { - /* We want parent to see errors in the same stream */ - xdup2(STDOUT_FILENO, STDERR_FILENO); - } - execvp(pArgs[0], pArgs); - /* VERB1 since sometimes we expect errors here */ - VERB1 perror_msg("Can't execute '%s'", pArgs[0]); - exit(1); - } + /* Nuke everything which may make setlocale() switch to non-POSIX locale: + * we need to avoid having gdb output in some obscure language. + */ + static const char *const unsetenv_vec[] = { + "LANG", + "LC_ALL", + "LC_COLLATE", + "LC_CTYPE", + "LC_MESSAGES", + "LC_MONETARY", + "LC_NUMERIC", + "LC_TIME", + NULL + }; + + int flags = EXECFLG_INPUT_NUL | EXECFLG_OUTPUT | EXECFLG_SETGUID | EXECFLG_SETSID | EXECFLG_QUIET; + if (redirect_stderr) + flags |= EXECFLG_ERR2OUT; + VERB1 flags &= ~EXECFLG_QUIET; - close(pipeout[1]); /* write side of the pipe */ + int pipeout[2]; + pid_t child = fork_execv_on_steroids(flags, pArgs, pipeout, (char**)unsetenv_vec, /*dir:*/ NULL, uid); int r; char buff[1024]; @@ -155,8 +129,8 @@ static int ExecVP(char **pArgs, uid_t uid, int redirect_stderr, string& pOutput) buff[r] = '\0'; pOutput += buff; } - close(pipeout[0]); + int status; waitpid(child, &status, 0); /* prevent having zombie child process */ -- cgit From b1c4304104910c4bc066cd43f9784fe2f3ddf1ad Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 11 Jan 2010 07:21:31 +0100 Subject: *: cast pids and uigs to long, not int Signed-off-by: Denys Vlasenko --- lib/Plugins/CCpp.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/Plugins/CCpp.cpp') diff --git a/lib/Plugins/CCpp.cpp b/lib/Plugins/CCpp.cpp index 26562be..a7f005c 100644 --- a/lib/Plugins/CCpp.cpp +++ b/lib/Plugins/CCpp.cpp @@ -368,7 +368,7 @@ static void InstallDebugInfos(const char *pDebugDumpDir, char *coredump = xasprintf("%s/"FILENAME_COREDUMP, pDebugDumpDir); /* SELinux guys are not happy with /tmp, using /var/run/abrt */ - char *tempdir = xasprintf(LOCALSTATEDIR"/run/abrt/tmp-%u-%lu", (int)getpid(), (long)time(NULL)); + char *tempdir = xasprintf(LOCALSTATEDIR"/run/abrt/tmp-%lu-%lu", (long)getpid(), (long)time(NULL)); /* log() goes to stderr/syslog, it's ok to use it here */ VERB1 log("Executing: %s %s %s %s", "abrt-debuginfo-install", coredump, tempdir, debuginfo_dirs); /* We want parent to see errors in the same stream */ -- cgit From 14ef0cfe72faf6696df3ef8f42927e9458ccbeeb Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 11 Jan 2010 07:22:13 +0100 Subject: *: misc fixes Signed-off-by: Denys Vlasenko --- lib/Plugins/CCpp.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/Plugins/CCpp.cpp') diff --git a/lib/Plugins/CCpp.cpp b/lib/Plugins/CCpp.cpp index a7f005c..46c2f28 100644 --- a/lib/Plugins/CCpp.cpp +++ b/lib/Plugins/CCpp.cpp @@ -627,7 +627,7 @@ static bool DebuginfoCheckPolkit(int uid) //parent int status; - if (waitpid(child_pid, &status, 0) > 0 && WEXITSTATUS(status) == 0) + if (waitpid(child_pid, &status, 0) > 0 && WIFEXITED(status) && WEXITSTATUS(status) == 0) { return true; //authorization OK } -- cgit From edf6beb585dc38c365ccbdaae85756b2814e1329 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 11 Jan 2010 12:09:57 +0100 Subject: *: assorted fixes prompted by security analysis; more to come Signed-off-by: Denys Vlasenko --- lib/Plugins/CCpp.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'lib/Plugins/CCpp.cpp') diff --git a/lib/Plugins/CCpp.cpp b/lib/Plugins/CCpp.cpp index 46c2f28..4bf1bc1 100644 --- a/lib/Plugins/CCpp.cpp +++ b/lib/Plugins/CCpp.cpp @@ -353,6 +353,7 @@ static void InstallDebugInfos(const char *pDebugDumpDir, int pipeout[2]; //TODO: can we use ExecVP? xpipe(pipeout); + fflush(NULL); pid_t child = fork(); if (child < 0) { @@ -548,6 +549,8 @@ string CAnalyzerCCpp::GetGlobalUUID(const char *pDebugDumpDir) int pipeout[2]; xpipe(pipeout); /* stdout of abrt-backtrace */ + + fflush(NULL); pid_t child = fork(); if (child == -1) perror_msg_and_die("fork"); @@ -609,8 +612,9 @@ string CAnalyzerCCpp::GetGlobalUUID(const char *pDebugDumpDir) return CreateHash(hash_base.c_str()); } -static bool DebuginfoCheckPolkit(int uid) +static bool DebuginfoCheckPolkit(uid_t uid) { + fflush(NULL); int child_pid = fork(); if (child_pid < 0) { @@ -627,8 +631,10 @@ static bool DebuginfoCheckPolkit(int uid) //parent int status; - if (waitpid(child_pid, &status, 0) > 0 && WIFEXITED(status) && WEXITSTATUS(status) == 0) - { + if (waitpid(child_pid, &status, 0) > 0 + && WIFEXITED(status) + && WEXITSTATUS(status) == 0 + ) { return true; //authorization OK } log("UID %d is not authorized to install debuginfos", uid); -- cgit From 20645ae11a8a9a89cc712896b2f72a25bc62c8db Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 11 Jan 2010 12:12:53 +0100 Subject: DebugDump: more consistent logic in setting mode and uid:gid on dump dir With comments! yay. Before it, too restrictive mode was preventing python craches to be handled. Signed-off-by: Karel Klic Signed-off-by: Denys Vlasenko --- lib/Plugins/CCpp.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib/Plugins/CCpp.cpp') diff --git a/lib/Plugins/CCpp.cpp b/lib/Plugins/CCpp.cpp index 4bf1bc1..8b0c2b8 100644 --- a/lib/Plugins/CCpp.cpp +++ b/lib/Plugins/CCpp.cpp @@ -520,6 +520,8 @@ string CAnalyzerCCpp::GetGlobalUUID(const char *pDebugDumpDir) { log(_("Getting global universal unique identification...")); +//TODO: convert to fork_execv_on_steroids(), nuke static concat_str_vector + string backtrace_path = concat_path_file(pDebugDumpDir, FILENAME_BACKTRACE); string executable; string package; -- cgit From 4af592f48f98f54f96e9baeecf853cf1a67b9c86 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 11 Jan 2010 15:21:23 +0100 Subject: CCpp: use our own sha1 implementation (less pain with nss libs) Signed-off-by: Denys Vlasenko --- lib/Plugins/CCpp.cpp | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) (limited to 'lib/Plugins/CCpp.cpp') diff --git a/lib/Plugins/CCpp.cpp b/lib/Plugins/CCpp.cpp index 8b0c2b8..6455f44 100644 --- a/lib/Plugins/CCpp.cpp +++ b/lib/Plugins/CCpp.cpp @@ -34,6 +34,8 @@ #include "CommLayerInner.h" #include "Polkit.h" +#include "CCpp_sha1.h" + using namespace std; #define CORE_PATTERN_IFACE "/proc/sys/kernel/core_pattern" @@ -55,10 +57,13 @@ CAnalyzerCCpp::CAnalyzerCCpp() : static string CreateHash(const char *pInput) { string ret; - HASHContext* hc; - unsigned char hash[SHA1_LENGTH]; + char hash_str[SHA1_LENGTH*2 + 1]; unsigned int len; +#if 0 +{ + unsigned char hash[SHA1_LENGTH]; + HASHContext *hc; hc = HASH_Create(HASH_AlgSHA1); if (!hc) { @@ -69,7 +74,6 @@ static string CreateHash(const char *pInput) HASH_End(hc, hash, &len, sizeof(hash)); HASH_Destroy(hc); - char hash_str[SHA1_LENGTH*2 + 1]; char *d = hash_str; unsigned char *s = hash; while (len) @@ -80,6 +84,28 @@ static string CreateHash(const char *pInput) len--; } *d = '\0'; +//log("hash1:%s str:'%s'", hash_str, pInput); +} +#endif + + unsigned char hash2[SHA1_LENGTH]; + sha1_ctx_t sha1ctx; + sha1_begin(&sha1ctx); + sha1_hash(pInput, strlen(pInput), &sha1ctx); + sha1_end(hash2, &sha1ctx); + len = SHA1_LENGTH; + + char *d = hash_str; + unsigned char *s = hash2; + while (len) + { + *d++ = "0123456789abcdef"[*s >> 4]; + *d++ = "0123456789abcdef"[*s & 0xf]; + s++; + len--; + } + *d = '\0'; +//log("hash2:%s str:'%s'", hash_str, pInput); return hash_str; } -- cgit From fd761d18b02686b9a39e3fcf4882ca40a1b5c6a3 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 11 Jan 2010 16:09:28 +0100 Subject: *: remove nss dependencies Signed-off-by: Denys Vlasenko --- lib/Plugins/CCpp.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'lib/Plugins/CCpp.cpp') diff --git a/lib/Plugins/CCpp.cpp b/lib/Plugins/CCpp.cpp index 6455f44..6c200d0 100644 --- a/lib/Plugins/CCpp.cpp +++ b/lib/Plugins/CCpp.cpp @@ -24,9 +24,8 @@ #include #include #include -#include -#include -#include +//#include +//#include #include "abrtlib.h" #include "CCpp.h" #include "ABRTException.h" @@ -56,12 +55,11 @@ CAnalyzerCCpp::CAnalyzerCCpp() : static string CreateHash(const char *pInput) { - string ret; - char hash_str[SHA1_LENGTH*2 + 1]; unsigned int len; #if 0 { + char hash_str[SHA1_LENGTH*2 + 1]; unsigned char hash[SHA1_LENGTH]; HASHContext *hc; hc = HASH_Create(HASH_AlgSHA1); @@ -88,12 +86,13 @@ static string CreateHash(const char *pInput) } #endif - unsigned char hash2[SHA1_LENGTH]; + char hash_str[SHA1_RESULT_LEN*2 + 1]; + unsigned char hash2[SHA1_RESULT_LEN]; sha1_ctx_t sha1ctx; sha1_begin(&sha1ctx); sha1_hash(pInput, strlen(pInput), &sha1ctx); sha1_end(hash2, &sha1ctx); - len = SHA1_LENGTH; + len = SHA1_RESULT_LEN; char *d = hash_str; unsigned char *s = hash2; -- cgit From df1d7cd05d8f6abba8c713e773828deee19baf22 Mon Sep 17 00:00:00 2001 From: Karel Klic Date: Wed, 13 Jan 2010 14:21:20 +0100 Subject: Better error messages for abrt-backtrace execution --- lib/Plugins/CCpp.cpp | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) (limited to 'lib/Plugins/CCpp.cpp') diff --git a/lib/Plugins/CCpp.cpp b/lib/Plugins/CCpp.cpp index 6c200d0..e0db9cf 100644 --- a/lib/Plugins/CCpp.cpp +++ b/lib/Plugins/CCpp.cpp @@ -621,13 +621,29 @@ string CAnalyzerCCpp::GetGlobalUUID(const char *pDebugDumpDir) { perror_msg("abrt-backtrace not executed properly, " "status: %x signal: %d", status, WIFSIGNALED(status)); - } else + } + else { int exit_status = WEXITSTATUS(status); - if (exit_status != 0) + if (exit_status == 79) /* EX_PARSINGFAILED */ + { + /* abrt-backtrace returns alternative backtrace + representation in this case, so everything will work + as expected except worse duplication detection */ + log_msg("abrt-backtrace failed to parse the backtrace"); + } + else if (exit_status == 80) /* EX_THREADDETECTIONFAILED */ + { + /* abrt-backtrace returns backtrace with all threads + in this case, so everything will work as expected + except worse duplication detection */ + log_msg("abrt-backtrace failed to determine crash frame"); + } + else if (exit_status != 0) { + /* this is unexpected problem and it should be investigated */ error_msg("abrt-backtrace run failed, exit value: %d", - exit_status); + exit_status); } } -- cgit