From 885c98b9ea0a2f5084342886eefb29c8198b44a0 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 22 Aug 2018 12:28:48 +0200 Subject: [PATCH 01/16] Move crash-manager.conf to /etc This commit brings back config to /etc to avoid problems with config not being available for crashes happening during bootup (when /opt/ is not mounted yet) Change-Id: Ib384f1fd28192dd199565d888c59f0a33d4d578d --- packaging/crash-worker.spec | 3 ++- src/crash-manager/CMakeLists.txt | 2 +- src/crash-manager/crash-manager.c | 5 ++--- src/crash-manager/crash-manager.h.in | 1 + 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index aebc135..d19aaf9 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -101,6 +101,7 @@ export CFLAGS+=" -Werror" -DTMP_FILES_DIR=%{_sysconfdir}/tmpfiles.d \ -DARCH=%{ARCH} \ -DARCH_BIT=%{ARCH_BIT} \ + -DCRASH_MANAGER_CONFIG_PATH=%{_sysconfdir}/crash-manager.conf \ -DTZ_SYS_ETC=%{TZ_SYS_ETC} \ -DTZ_SYS_BIN=%{TZ_SYS_BIN} \ -DCRASH_ROOT_PATH=%{crash_root_path} \ @@ -173,7 +174,7 @@ sed -i "/${pattern}/D" %{_sysconfdir}/ld.so.preload %{crash_dump_gen}/* %attr(0750,system_fw,system_fw) %{_bindir}/* %{_unitdir}/log_dump.service -%{TZ_SYS_ETC}/crash-manager.conf +%{_sysconfdir}/crash-manager.conf %attr(-,root,root) %{_sysconfdir}/dbus-1/system.d/log_dump.conf %attr(-,root,root) %{_prefix}/lib/sysctl.d/99-crash-manager.conf %attr(-,root,root) %{_datadir}/dbus-1/system-services/org.tizen.system.crash.service diff --git a/src/crash-manager/CMakeLists.txt b/src/crash-manager/CMakeLists.txt index 1ad1f93..ee04dd0 100644 --- a/src/crash-manager/CMakeLists.txt +++ b/src/crash-manager/CMakeLists.txt @@ -43,7 +43,7 @@ ENDIF(USE_COREDUMP_CONF) INSTALL(FILES ${CMAKE_SOURCE_DIR}/src/${PROJECT_NAME}/crash-manager.conf - DESTINATION ${TZ_SYS_ETC} + DESTINATION /etc PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ) INSTALL(FILES ${CMAKE_SOURCE_DIR}/src/${PROJECT_NAME}/99-${PROJECT_NAME}.conf diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 87b6756..96802b3 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -47,7 +47,6 @@ #define LOG_TAG "CRASH_MANAGER" /* Parsing */ -#define CRASH_CONF_FILE tzplatform_mkpath(TZ_SYS_ETC, "crash-manager.conf") #define MINICOREDUMPER_CONF_FILE MINICOREDUMPER_CONF_DIR "/minicoredumper.cfg.json" #define KEY_MAX 255 #define CRASH_SECTION "CrashManager" @@ -292,9 +291,9 @@ static int get_config(void) } report_type = REP_DEFAULT_TYPE; - ini = iniparser_load(CRASH_CONF_FILE); + ini = iniparser_load(CRASH_MANAGER_CONFIG_PATH); if (!ini) { - _E("Failed to load conf file %s", CRASH_CONF_FILE); + _E("Failed to load conf file %s", CRASH_MANAGER_CONFIG_PATH); return 0; } diff --git a/src/crash-manager/crash-manager.h.in b/src/crash-manager/crash-manager.h.in index 149a056..dbcd051 100644 --- a/src/crash-manager/crash-manager.h.in +++ b/src/crash-manager/crash-manager.h.in @@ -25,6 +25,7 @@ #define CRASH_TEMP "@CRASH_TEMP@" #define SYS_ASSERT "@SYS_ASSERT@" #define CRASH_STACK_PATH "@CRASH_STACK_PATH@" +#define CRASH_MANAGER_CONFIG_PATH "@CRASH_MANAGER_CONFIG_PATH@" #define MINICOREDUMPER_PATH "@MINICOREDUMPER_PATH@" #define MINICOREDUMPER_CONF_DIR "@MINICOREDUMPER_CONF_DIR@" #define DEBUGMODE_PATH "@DEBUGMODE_PATH@" -- 2.7.4 From 794ec71575854d223ddcd4d355713c43e92b3fa8 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 22 Aug 2018 12:30:45 +0200 Subject: [PATCH 02/16] Normalize minicoredumper config variable naming Change-Id: I5669d0d46fc15c8193b5e8ea11d1254838a8dfd1 --- packaging/crash-worker.spec | 4 ++-- src/crash-manager/crash-manager.c | 5 ++--- src/crash-manager/crash-manager.h.in | 4 ++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index d19aaf9..9aad40e 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -108,8 +108,8 @@ export CFLAGS+=" -Werror" -DCRASH_PATH=%{crash_path} \ -DCRASH_TEMP=%{crash_temp} \ -DDEBUGMODE_PATH=%{debugmode_path} \ - -DMINICOREDUMPER_PATH=%{_sbindir}/minicoredumper \ - -DMINICOREDUMPER_CONF_DIR=%{_sysconfdir}/minicoredumper \ + -DMINICOREDUMPER_BIN_PATH=%{_sbindir}/minicoredumper \ + -DMINICOREDUMPER_CONFIG_PATH=%{_sysconfdir}/minicoredumper/minicoredumper.cfg.json \ -DCRASH_STACK_PATH=%{_libexecdir}/crash-stack \ -DCRASH_TESTS_PATH=%{_libdir}/crash-worker-tests \ -DSYS_ASSERT=%{on_off sys_assert} \ diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 96802b3..b04d7e2 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -47,7 +47,6 @@ #define LOG_TAG "CRASH_MANAGER" /* Parsing */ -#define MINICOREDUMPER_CONF_FILE MINICOREDUMPER_CONF_DIR "/minicoredumper.cfg.json" #define KEY_MAX 255 #define CRASH_SECTION "CrashManager" @@ -789,7 +788,7 @@ static int execute_minicoredump(int argc, char *argv[]) /* Execute minicoredumper */ char *args[] = { - MINICOREDUMPER_PATH, // minicoredumper filename path + MINICOREDUMPER_BIN_PATH, // minicoredumper filename path crash_info.pid_info, // %p - pid argv[2], // %u - UID argv[3], // %g - GID @@ -797,7 +796,7 @@ static int execute_minicoredump(int argc, char *argv[]) argv[5], // %t - time of dump "localhost", // %h - hostname "core", // %e - exe name (need for result filename) - MINICOREDUMPER_CONF_FILE, // config file + MINICOREDUMPER_CONFIG_PATH, // config file "-d", crash_info.pfx, // temp dir "-o", diff --git a/src/crash-manager/crash-manager.h.in b/src/crash-manager/crash-manager.h.in index dbcd051..f5a8c4b 100644 --- a/src/crash-manager/crash-manager.h.in +++ b/src/crash-manager/crash-manager.h.in @@ -26,8 +26,8 @@ #define SYS_ASSERT "@SYS_ASSERT@" #define CRASH_STACK_PATH "@CRASH_STACK_PATH@" #define CRASH_MANAGER_CONFIG_PATH "@CRASH_MANAGER_CONFIG_PATH@" -#define MINICOREDUMPER_PATH "@MINICOREDUMPER_PATH@" -#define MINICOREDUMPER_CONF_DIR "@MINICOREDUMPER_CONF_DIR@" +#define MINICOREDUMPER_BIN_PATH "@MINICOREDUMPER_BIN_PATH@" +#define MINICOREDUMPER_CONFIG_PATH "@MINICOREDUMPER_CONFIG_PATH@" #define DEBUGMODE_PATH "@DEBUGMODE_PATH@" #endif -- 2.7.4 From c149659f4aca21cceeae209023463f88fa0c7459 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 22 Aug 2018 07:43:14 +0200 Subject: [PATCH 03/16] Release 5.0.1 Major version bump due to versioning scheme change. Major versions will now follow major Tizen releases (eg. crash-worker 5 for Tizen 5). New features: - callstack resolver has been rewritten to not use ptrace This was required to avoid races during resolve process (kernel would wake up ptraced process). Consequently, it's now possible to prepare crash report of VIP process (whose failure causes system to be rebooted). - New-style D-Bus signal ProcessCrashed is now emitted to notify about the crash - .info-only report type now can be selected via configuration option ReportType - Crash dump location can be changed by CrashReportPath config option - Saving core dump (for FULL/.zip reports) can be changed by DumpCore option Change-Id: I91e806d0d516e618802aee7a742120c0281034a1 --- packaging/crash-worker.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index 9aad40e..b71789f 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -10,7 +10,7 @@ Name: crash-worker Summary: Crash-manager -Version: 1.2.1 +Version: 5.0.1 Release: 1 Group: Framework/system License: Apache-2.0 -- 2.7.4 From eac9b858fdf93ede3b1edcacc49b0792844fce0f Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Tue, 14 Aug 2018 15:42:02 +0200 Subject: [PATCH 04/16] Move finding TID to crash-manager Additionally find_crash_tid() returns PID for single-threaded applications, instead of -1. Change-Id: Id6fcc652b95bddda954f25cd448f1e090918c231 --- src/crash-manager/crash-manager.c | 49 ++++++++++++++---------- src/crash-stack/crash-stack.c | 78 +-------------------------------------- src/shared/util.c | 78 +++++++++++++++++++++++++++++++++++++++ src/shared/util.h | 2 + 4 files changed, 112 insertions(+), 95 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index b04d7e2..dd5c0f5 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -530,8 +530,26 @@ static int set_crash_info(int argc, char *argv[]) crash_info.pid_info = argv[1]; crash_info.sig_info = argv[4]; - if (argc > 6) - crash_info.tid_info = argv[6]; + if (argc > 6) { + crash_info.tid_info = strdup(argv[6]); + if (crash_info.tid_info == NULL) { + _E("strdup error: %m"); + return -1; + } + } else { + crash_info.tid_info = NULL; + int pid = atoi(crash_info.pid_info); + int tid = find_crash_tid(pid); + if (tid < 0) { + _I("TID not found"); + tid = pid; + } + + if (asprintf(&crash_info.tid_info, "%d", tid) == -1) { + _E("asprintf error: %m"); + return -1; + } + } ret = get_cmd_info(&crash_info); if (ret <= 0) { @@ -842,23 +860,15 @@ static void execute_crash_stack(int argc, char *argv[]) char command[PATH_MAX]; /* Execute crash-stack */ - if (argc > 8) - ret = snprintf(command, sizeof(command), - "%s --pid %s --tid %s --sig %s --prstatus_fd %d > %s", - CRASH_STACK_PATH, - crash_info.pid_info, - crash_info.tid_info, - crash_info.sig_info, - crash_info.prstatus_fd, - crash_info.info_path); - else - ret = snprintf(command, sizeof(command), - "%s --pid %s --sig %s --prstatus_fd %d > %s", - CRASH_STACK_PATH, - crash_info.pid_info, - crash_info.sig_info, - crash_info.prstatus_fd, - crash_info.info_path); + ret = snprintf(command, sizeof(command), + "%s --pid %s --tid %s --sig %s --prstatus_fd %d > %s", + CRASH_STACK_PATH, + crash_info.pid_info, + crash_info.tid_info, + crash_info.sig_info, + crash_info.prstatus_fd, + crash_info.info_path); + _D(" %s", command); if (ret < 0) { _E("Failed to snprintf for crash-stack command"); @@ -1307,6 +1317,7 @@ int main(int argc, char *argv[]) exit: close(crash_info.prstatus_fd); + free(crash_info.tid_info); free(crash_temp_path); free(crash_root_path); free(crash_crash_path); diff --git a/src/crash-stack/crash-stack.c b/src/crash-stack/crash-stack.c index 6a2fca7..c4fdd03 100644 --- a/src/crash-stack/crash-stack.c +++ b/src/crash-stack/crash-stack.c @@ -577,80 +577,6 @@ static void __print_buffer_info(FILE* bufferfile, FILE *outputfile) } /** - * @brief Check wchan of thread - * - * @param pid PID of the inspected process - * @param tid TID of the thread to check - */ -static int check_thread_wchan(int pid, int tid) -{ - int fd, cnt; - char path[PATH_MAX], buf[100]; - - snprintf(path, PATH_MAX, "/proc/%d/task/%d/wchan", pid, tid); - fd = open(path, O_RDONLY); - if (fd == -1) { - _E("open(%s): %s", path, strerror(errno)); - return -errno; - } - cnt = read(fd, buf, sizeof(buf)); - if (cnt == -1 || cnt == sizeof(buf)) { - _E("read(%s): %s", path, strerror(errno)); - close(fd); - return -errno; - } - buf[cnt] = 0; - close(fd); - - if (strncmp("do_coredump", buf, sizeof(buf)) == 0 || strncmp("pipe_wait", buf, sizeof(buf)) == 0) - return tid; - else - return 0; -} - -/** - * @brief Find crashed tid if tid was not offered - * - * @param pid PID of the inspected process - */ -static int find_crash_tid(int pid) -{ - int threadnum = 1; - int crash_tid = -1; - DIR *dir; - struct dirent *entry; - char task_path[PATH_MAX]; - struct stat sb; - - snprintf(task_path, PATH_MAX, "/proc/%d/task", pid); - if (stat(task_path, &sb) == -1) - return -1; - - threadnum = sb.st_nlink - 2; - - if (threadnum > 1) { - dir = opendir(task_path); - if (!dir) { - _E("opendir(%s): %s", task_path, strerror(errno)); - return -1; - } else { - while ((entry = readdir(dir)) != NULL) { - if (strcmp(entry->d_name, ".") == 0 || - strcmp(entry->d_name, "..") == 0) - continue; - crash_tid = check_thread_wchan(pid, - atoi(entry->d_name)); - if (crash_tid > 0) - break; - } - closedir(dir); - return crash_tid; - } - } - return -1; -} - -/** * @brief Main function. * * Main module accepts should be launched with: @@ -700,8 +626,8 @@ int main(int argc, char **argv) if (NULL == outputfile) outputfile = stdout; if (tid == 0) { - if ((tid = find_crash_tid(pid)) < 0) - tid = pid; + fprintf(errfile, "TID not provided\n"); + return errno; } mode_t oldmode = umask(0077); diff --git a/src/shared/util.c b/src/shared/util.c index 21b76d3..3795415 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -739,6 +739,84 @@ int log_kmsg(char *fmt, ...) fclose(file); return result; } + +/** + * @brief Check wchan of thread + * + * @param pid PID of the inspected process + * @param tid TID of the thread to check + */ +static int check_thread_wchan(int pid, int tid) +{ + int fd, cnt; + char path[PATH_MAX], buf[100]; + + snprintf(path, sizeof(path), "/proc/%d/task/%d/wchan", pid, tid); + fd = open(path, O_RDONLY); + if (fd == -1) { + _E("cannot open %s: %m\n", path); + return -errno; + } + cnt = read(fd, buf, sizeof(buf)); + if (cnt == -1 || cnt == sizeof(buf)) { + _E("read %s error: %m\n", path); + close(fd); + return -errno; + } + buf[cnt] = 0; + close(fd); + + if (strncmp("do_coredump", buf, sizeof(buf)) == 0 || strncmp("pipe_wait", buf, sizeof(buf)) == 0) + return tid; + else + return 0; +} + +/** + * @brief Find crashed tid if tid was not offered + * + * @param pid PID of the inspected process + */ +int find_crash_tid(int pid) +{ + int threadnum = 1; + int crash_tid = -1; + DIR *dir; + struct dirent *entry; + char task_path[PATH_MAX]; + struct stat sb; + + snprintf(task_path, sizeof(task_path), "/proc/%d/task", pid); + if (stat(task_path, &sb) == -1) { + _E("no such file: %s", task_path); + return -1; + } + + threadnum = sb.st_nlink - 2; + + if (threadnum > 1) { + dir = opendir(task_path); + if (!dir) { + _E("cannot open %s\n", task_path); + return -1; + } else { + while ((entry = readdir(dir)) != NULL) { + if (strcmp(entry->d_name, ".") == 0 || + strcmp(entry->d_name, "..") == 0) + continue; + crash_tid = check_thread_wchan(pid, + atoi(entry->d_name)); + if (crash_tid > 0) + break; + } + closedir(dir); + return crash_tid; + } + } else if (threadnum == 1) { + return pid; + } + return -1; +} /** * @} */ diff --git a/src/shared/util.h b/src/shared/util.h index a4044e2..301b4e6 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -60,6 +60,8 @@ int validate_env_name(char *name, int len); int validate_file_name(char *name, int len); int log_kmsg(char *fmt, ...); + +int find_crash_tid(int pid); /** * @} */ -- 2.7.4 From e974864a29847f8172323ae44013a3c48e602298 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Thu, 23 Aug 2018 12:45:51 +0200 Subject: [PATCH 05/16] Fix report path for INFO report type Change-Id: I973c452cd08a4ac18d4ad7460b21ceb28d9d6364 --- src/crash-manager/crash-manager.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index dd5c0f5..eaafab9 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -1190,14 +1190,13 @@ static void move_info_file(void) if ((lock_fd = lock_dumpdir()) < 0) return; - char dest_path[PATH_MAX]; - snprintf(dest_path, sizeof(dest_path), "%s/%s.info", + snprintf(crash_info.result_path, sizeof(crash_info.result_path), "%s/%s.info", crash_crash_path, crash_info.name); - if (!rename(crash_info.info_path, dest_path)) + if (!rename(crash_info.info_path, crash_info.result_path)) clean_dump(); else _E("Failed to move %s to %s", - crash_info.info_path, dest_path); + crash_info.info_path, crash_info.result_path); unlock_dumpdir(lock_fd); if (remove_dir(crash_info.temp_dir, 1) < 0) -- 2.7.4 From ac2dd9d930046efd17e9e6392cd5dd4a476d25d7 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Thu, 23 Aug 2018 13:51:11 +0200 Subject: [PATCH 06/16] Update license information in spec file Change-Id: Iee77a2f8580167eb6e7087465fd820f0fae575a1 --- packaging/crash-worker.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index b71789f..c411292 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -13,7 +13,7 @@ Summary: Crash-manager Version: 5.0.1 Release: 1 Group: Framework/system -License: Apache-2.0 +License: Apache-2.0 and BSD Source0: %{name}-%{version}.tar.gz Source1001: crash-worker.manifest BuildRequires: pkgconfig(dlog) -- 2.7.4 From 307f57188421fe18a6279b24d6fb70f8d2f72b81 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Thu, 23 Aug 2018 13:54:30 +0200 Subject: [PATCH 07/16] Release 5.0.2 Changes from last version: - For INFO report type D-Bus signal has a proper report path - Update license (imported tbstack code is BSD-licensed) - Move finding TID to crash-manager - fixes support on older kernels Change-Id: Ie7e038d4740fcd442e5c599c9d22c91a24c2ce74 --- packaging/crash-worker.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index c411292..d695053 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -10,7 +10,7 @@ Name: crash-worker Summary: Crash-manager -Version: 5.0.1 +Version: 5.0.2 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From 2ef257d5011e852a9118f6039ccdde4e46c538f2 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Tue, 28 Aug 2018 12:35:24 +0200 Subject: [PATCH 08/16] Fix SVACE warnings Warnings: 352484, 352485, 352486, 352488, 352489, 352490, 352491, 352492, 352493, 352494, 352495, 352496, 352497, 352498, 352501, 352502, 352487, 358117, 358162 Change-Id: I205ddaee97836c3146b25bbb380e11464713cb25 --- src/crash-manager/crash-manager.c | 29 +++++++++++++++++++++-------- src/crash-stack/crash-stack.c | 16 ++++++++-------- src/crash-stack/mem_map.c | 30 ++++++++++-------------------- src/crash-stack/proc.c | 20 ++++++++++---------- src/crash-stack/unwind.c | 4 ++-- 5 files changed, 51 insertions(+), 48 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index eaafab9..3f3e089 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -275,6 +275,7 @@ static int get_config(void) dictionary *ini = NULL; char key[KEY_MAX]; int value; + int result = 1; char *value_str; system_max_use = SYSTEM_MAX_USE; @@ -368,7 +369,8 @@ static int get_config(void) crash_root_path = strdup(value_str); if (crash_root_path == NULL) { _E("strdup error: %m\n"); - return -1; + result = -1; + goto out; } } @@ -388,8 +390,9 @@ static int get_config(void) } } +out: iniparser_freedict(ini); - return 1; + return result; } static int make_dump_dir(void) @@ -485,31 +488,31 @@ static int set_prstatus() crash_info.prstatus_fd = shm_open(prstatus_name, O_RDWR | O_CREAT, 0600); if (crash_info.prstatus_fd < 0) { - _E("shm_open: %s", strerror(errno)); + _E("shm_open: %m"); goto close_fd; } ret = shm_unlink(prstatus_name); if (ret < 0) { - _E("shm_unlink: %s", strerror(errno)); + _E("shm_unlink: %m"); goto close_fd; } ret = fcntl(crash_info.prstatus_fd, F_GETFD); if (ret < 0) { - _E("fcntl(): %s", strerror(errno)); + _E("fcntl(): %m"); goto close_fd; } ret = fcntl(crash_info.prstatus_fd, F_SETFD, ret & ~FD_CLOEXEC); if (ret < 0) { - _E("fcntl(): %s", strerror(errno)); + _E("fcntl(): %m"); goto close_fd; } ret = ftruncate(crash_info.prstatus_fd, sizeof(struct elf_prstatus)); if (ret < 0) { - _E("ftruncate(): %s", strerror(errno)); + _E("ftruncate(): %m"); goto close_fd; } @@ -837,14 +840,24 @@ static int execute_minicoredump(int argc, char *argv[]) if (!dump_core) { int ret = -1; int errno_unlink = 0; + char errno_buff[128]; + char *err_str = NULL; int dirfd = open(crash_info.pfx, O_DIRECTORY); if (dirfd != -1) { ret = unlinkat(dirfd, coredump_name, 0); errno_unlink = errno; close(dirfd); } + + if (ret != 0) { + err_str = strerror_r(errno_unlink, errno_buff, sizeof(errno_buff)); + if (err_str == NULL) + _E("strerror_r() error: %m\n"); + goto out; + } + _D("Saving core disabled - removing coredump %s/%s: %s", crash_info.pfx, coredump_name, - ret == 0 ? "success" : strerror(errno_unlink)); + ret == 0 ? "success" : err_str); } out: diff --git a/src/crash-stack/crash-stack.c b/src/crash-stack/crash-stack.c index c4fdd03..5979c2f 100644 --- a/src/crash-stack/crash-stack.c +++ b/src/crash-stack/crash-stack.c @@ -304,7 +304,7 @@ static void __crash_stack_print_threads(FILE* outputfile, pid_t pid, pid_t tid) /* print thread */ dir = opendir(task_path); if (!dir) { - _E("opendir(%s): %s", task_path, strerror(errno)); + _E("opendir(%s): %m", task_path); } else { while ((dentry = readdir(dir))) { if (strcmp(dentry->d_name, ".") == 0 || @@ -335,7 +335,7 @@ static void __crash_stack_print_maps(FILE* outputfile, pid_t pid) snprintf(file_path, PATH_MAX, "/proc/%d/maps", pid); if ((fd = open(file_path, O_RDONLY)) < 0) { - _E("open(%s): %s", file_path, strerror(errno)); + _E("open(%s): %m", file_path); } else { /* parsing the maps to get code segment address*/ head = get_addr_list_from_maps(fd); @@ -398,7 +398,7 @@ static struct addr_node *get_addr_list_from_maps(int fd) PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); if (t_node == NULL) { - _E("mmap(): %s", strerror(errno)); + _E("mmap(): %m"); return NULL; } memcpy(t_node->perm, perm, PERM_LEN+1); @@ -488,7 +488,7 @@ static void __crash_stack_print_meminfo(FILE* outputfile, pid_t pid) fprintf(outputfile, "\nMemory information\n"); if ((fd = open("/proc/meminfo", O_RDONLY)) < 0) { - _E("open(/proc/meminfo): %s", strerror(errno)); + _E("open(/proc/meminfo): %m"); } else { while (fgets_fd(linebuf, BUF_SIZE, fd) != NULL) { sscanf(linebuf, "%16s %16s %*s", infoname, memsize); @@ -508,7 +508,7 @@ static void __crash_stack_print_meminfo(FILE* outputfile, pid_t pid) snprintf(file_path, PATH_MAX, "/proc/%d/status", pid); if ((fd = open(file_path, O_RDONLY)) < 0) { - _E("open(%s): %s", file_path, strerror(errno)); + _E("open(%s): %m", file_path); } else { while (fgets_fd(linebuf, BUF_SIZE, fd) != NULL) { sscanf(linebuf, "%16s %16s %*s", infoname, memsize); @@ -567,7 +567,7 @@ static void __print_buffer_info(FILE* bufferfile, FILE *outputfile) char buf[1024]; if (fseek(bufferfile, 0, SEEK_SET) < 0) { - _E("fseek(): %s", strerror(errno)); + _E("fseek(): %m"); return; } while (!(feof(bufferfile) || ferror(bufferfile)) && (cnt = fread(buf, sizeof(char), sizeof(buf), bufferfile)) != 0) { @@ -632,13 +632,13 @@ int main(int argc, char **argv) mode_t oldmode = umask(0077); if (mkstemp(bufferfile_path) < 0) { - _E("mkstemp(%s): %s", bufferfile_path, strerror(errno)); + _E("mkstemp(%s): %m", bufferfile_path); return errno; } umask(oldmode); if ((bufferfile = fopen(bufferfile_path, "w+")) == NULL) { - _E("fopen(%s): %s", bufferfile_path, strerror(errno)); + _E("fopen(%s): %m", bufferfile_path); return errno; } unlink(bufferfile_path); diff --git a/src/crash-stack/mem_map.c b/src/crash-stack/mem_map.c index f74bc23..3a0d463 100644 --- a/src/crash-stack/mem_map.c +++ b/src/crash-stack/mem_map.c @@ -195,9 +195,8 @@ static struct mem_data_chunk *mem_region_alloc_chunk(struct mem_region *region, chunk->length = (size_t)end - (size_t)start; rc = posix_memalign((void **)&chunk->data, align, chunk->length); if (rc < 0) { - int err = errno; free(chunk); - _E("posix_memalign:%s", strerror(err)); + _E("posix_memalign:%m"); return NULL; } @@ -336,15 +335,13 @@ static int mem_region_map_file(struct mem_region *region) region->fd = open(region->path, O_RDONLY); if (region->fd < 0) { - int err = errno; mem_region_print(region); - _E("open(%s):%s", region->path, strerror(err)); + _E("open(%s):%m", region->path); return -1; } if (fstat(region->fd, &stat_buf) < 0) { - int err = errno; - _E("Unable to stat file %s: %s", region->path, strerror(err)); + _E("Unable to stat file %s: %m", region->path); return -1; } @@ -363,17 +360,14 @@ static int mem_region_map_file(struct mem_region *region) region->offset); if (data == MAP_FAILED) { - int err = errno; - _E("Unable to mmap file %s (length 0x%zx, read, offset 0x%zx): %s", - region->path, region->length, region->offset, - strerror(err)); + _E("Unable to mmap file %s (length 0x%zx, read, offset 0x%zx): %m", + region->path, region->length, region->offset); return -1; } region->data_head = malloc(sizeof(struct mem_data_chunk)); if (region->data_head == NULL) { - int err = errno; - _E("Unable to allocate memory:%s", strerror(err)); + _E("Unable to allocate memory:%m"); munmap(data, length); return -1; } @@ -384,8 +378,7 @@ static int mem_region_map_file(struct mem_region *region) region->data_index = malloc(sizeof(struct mem_data_chunk**)); if (region->data_index == NULL){ - int err = errno; - _E("Unable to allocate memory:%s", strerror(err)); + _E("Unable to allocate memory:%m"); free(region->data_head); munmap(data, length); return -1; @@ -410,8 +403,7 @@ static int mem_region_init_vdso(struct mem_region *region) region->data_index = malloc(sizeof(struct mem_data_chunk**)); if (region->data_index == NULL) { - int err = errno; - _E("Unable to allocate memory:%s", strerror(err)); + _E("Unable to allocate memory:%m"); return -1; } *region->data_index = region->data_head; @@ -426,8 +418,7 @@ static int mem_region_init_vsyscall(struct mem_region *region) { region->data_head = malloc(sizeof(struct mem_data_chunk)); if (region->data_head == NULL) { - int err = errno; - _E("Unable to allocate memory:%s", strerror(err)); + _E("Unable to allocate memory:%m"); return -1; } mem_data_chunk_init(region->data_head); @@ -436,8 +427,7 @@ static int mem_region_init_vsyscall(struct mem_region *region) region->data_head->length = region->length; region->data_index = malloc(sizeof(struct mem_data_chunk**)); if (region->data_index == NULL) { - int err = errno; - _E("Unable to allocate memory:%s", strerror(err)); + _E("Unable to allocate memory:%m"); return -1; } *region->data_index = region->data_head; diff --git a/src/crash-stack/proc.c b/src/crash-stack/proc.c index f082440..fd346c6 100644 --- a/src/crash-stack/proc.c +++ b/src/crash-stack/proc.c @@ -95,7 +95,7 @@ int proc_state(int pid) snprintf(buf, sizeof(buf), "/proc/%d/status", pid); if ((f = fopen(buf, "r")) == NULL) { - _E("Cannot open %s: %s", buf, strerror(errno)); + _E("Cannot open %s: %m", buf); return -1; } @@ -127,18 +127,18 @@ struct mem_map *create_maps(int pid) capacity = 0x100000; buf = calloc(1, capacity); if (buf == NULL) { - _E("Unable to allocate memory: %s", strerror(errno)); + _E("Unable to allocate memory: %m"); return NULL; } snprintf(buf, capacity, "/proc/%d/maps", pid); if ((f = fopen(buf, "r")) == NULL) { - _E("Cannot open %s: %s", buf, strerror(errno)); + _E("Cannot open %s: %m", buf); goto create_maps_end; } map = malloc(sizeof(struct mem_map)); if (map == NULL){ - _E("Unable to allocate memory: %s", strerror(errno)); + _E("Unable to allocate memory: %m"); goto create_maps_end; } mem_map_init(map); @@ -160,7 +160,7 @@ struct mem_map *create_maps(int pid) capacity *= 2; buf = realloc(buf, capacity); if (buf == NULL) { - _E("Unable to reallocate memory: %s", strerror(errno)); + _E("Unable to reallocate memory: %m"); mem_map_destroy(map); map = NULL; goto create_maps_end; @@ -196,7 +196,7 @@ struct mem_map *create_maps(int pid) region = malloc(sizeof(struct mem_region)); if (region == NULL) { - _E("Unable to allocate memory: %s", strerror(errno)); + _E("Unable to allocate memory: %m"); mem_map_destroy(map); map = NULL; break; @@ -411,7 +411,7 @@ static int copy_memory_process_vm_readv(int pid, if (errno == ENOSYS) rc = ENOSYS; else - _E("process_vm_readv: %s", strerror(errno)); + _E("process_vm_readv: %m"); goto process_vm_readv_end; } @@ -459,7 +459,7 @@ static int copy_memory_proc_mem(int pid, struct mem_data_chunk **frames, snprintf(fname, sizeof(fname), "/proc/%d/mem", pid); if ((fd = open(fname, O_RDONLY)) == -1) { - _E("Cannot open %s: %s", fname, strerror(errno)); + _E("Cannot open %s: %m", fname); return -1; } @@ -472,8 +472,8 @@ static int copy_memory_proc_mem(int pid, struct mem_data_chunk **frames, ssize_t rd = pread(fd, to, count, from); if (rd == -1) { - _E("pread() at %s:0x%lx (#%d) failed: %s [%d]", - fname, from, i, strerror(errno), errno); + _E("pread() at %s:0x%lx (#%d) failed: %m [%d]", + fname, from, i, errno); goto proc_mem_end; } diff --git a/src/crash-stack/unwind.c b/src/crash-stack/unwind.c index c98433b..bce9ef8 100644 --- a/src/crash-stack/unwind.c +++ b/src/crash-stack/unwind.c @@ -331,7 +331,7 @@ static int push_symbol(struct symbols *array, const GElf_Sym *s) array->s_cap <<= 1; new_data = malloc(sizeof(GElf_Sym) * array->s_cap); if (new_data == NULL) { - _E("malloc(): %s", strerror(errno)); + _E("malloc(): %m"); return -1; } memcpy(new_data, array->s_data, sizeof(GElf_Sym) * (array->s_size-1)); @@ -840,7 +840,7 @@ void *_TB_create (pid_t pid) { long label; if ((page = sysconf(_SC_PAGESIZE)) < 0) { - _E("sysconf(_SC_PAGESIZE): %s", strerror(errno)); + _E("sysconf(_SC_PAGESIZE): %m"); return NULL; } --page; -- 2.7.4 From d21b390ed91612cd3bbe01afbe554d7736a7970f Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Tue, 28 Aug 2018 14:22:37 +0200 Subject: [PATCH 09/16] Release 5.0.3 This version contains fixes of SVACE warnings Change-Id: Id51181c8bf839922b83d0d85d3596a414d005ee5 --- packaging/crash-worker.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index d695053..0ed23dc 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -10,7 +10,7 @@ Name: crash-worker Summary: Crash-manager -Version: 5.0.2 +Version: 5.0.3 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From 4086db3bf7e7cf7bf6aecc0a7d40891f6f02573d Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 29 Aug 2018 10:36:21 +0200 Subject: [PATCH 10/16] crash-manager: Simplify dump_core error reporting Change-Id: If8ff95811e4e5f16cb3a7a2e48dfe34a0bd3b1cb --- src/crash-manager/crash-manager.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 3f3e089..98630b5 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -840,24 +840,20 @@ static int execute_minicoredump(int argc, char *argv[]) if (!dump_core) { int ret = -1; int errno_unlink = 0; - char errno_buff[128]; - char *err_str = NULL; int dirfd = open(crash_info.pfx, O_DIRECTORY); if (dirfd != -1) { ret = unlinkat(dirfd, coredump_name, 0); errno_unlink = errno; close(dirfd); + errno = errno_unlink; /* for %m below */ } - if (ret != 0) { - err_str = strerror_r(errno_unlink, errno_buff, sizeof(errno_buff)); - if (err_str == NULL) - _E("strerror_r() error: %m\n"); - goto out; - } - - _D("Saving core disabled - removing coredump %s/%s: %s", crash_info.pfx, coredump_name, - ret == 0 ? "success" : err_str); + if (ret != 0) + _E("Saving core disabled - removing coredump %s/%s failed: %m", + crash_info.pfx, coredump_name); + else + _D("Saving core disabled - removed coredump %s/%s", + crash_info.pfx, coredump_name); } out: -- 2.7.4 From 51aa986303a4f8a8209d7bfb3a5f4583aa8030a1 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Tue, 28 Aug 2018 15:27:30 +0200 Subject: [PATCH 11/16] Move the function to read /proc//exe symlink to util.c Change-Id: Ieb533fb66dd3d06eb5bbc827aa016a308cf892a3 --- src/crash-manager/crash-manager.c | 47 ++-------------------------------- src/crash-stack/CMakeLists.txt | 2 +- src/crash-stack/crash-stack.c | 16 ++++-------- src/shared/util.c | 53 ++++++++++++++++++++++++++++++++++++++- src/shared/util.h | 4 +++ 5 files changed, 64 insertions(+), 58 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 3f3e089..53b0175 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -428,51 +428,8 @@ static int make_dump_dir(void) static int get_cmd_info(struct crash_info *cinfo) { - int fd; - int ret; - char cmdline[PATH_MAX]; - char cmdline_path[PATH_MAX]; - char exe_link[PATH_MAX]; - - snprintf(cmdline_path, sizeof(cmdline_path), - "/proc/%s/cmdline", cinfo->pid_info); - fd = open(cmdline_path, O_RDONLY); - if (fd < 0) { - _E("Failed to open %s", cmdline_path); - return -1; - } - - ret = read(fd, cmdline, sizeof(cmdline) - 1); - if (ret <= 0) { - _E("Failed to read %s", cmdline_path); - goto error; - } - - cmdline[ret] = '\0'; - - strncpy(cinfo->cmd_line, cmdline, sizeof(cinfo->cmd_line)); - - snprintf(exe_link, sizeof(exe_link), - "/proc/%s/exe", cinfo->pid_info); - - ret = readlink(exe_link, cinfo->cmd_path, - sizeof(cinfo->cmd_path) - 1); - if (ret <= 0) { - _E("Failed to read link %s", exe_link); - goto error; - } - - cinfo->cmd_path[ret] = '\0'; - - if (access(cinfo->cmd_path, F_OK) == -1) { - _E("Invalid path %s", cinfo->cmd_path); - ret = -1; - goto error; - } - -error: - close(fd); - return ret; + return get_cmd_line(cinfo->pid_info, cinfo->cmd_line, sizeof(cinfo->cmd_path)) && + get_exe_path(cinfo->pid_info, cinfo->cmd_path, sizeof(cinfo->cmd_path)); } static int set_prstatus() diff --git a/src/crash-stack/CMakeLists.txt b/src/crash-stack/CMakeLists.txt index ddeba3a..fd26fac 100644 --- a/src/crash-stack/CMakeLists.txt +++ b/src/crash-stack/CMakeLists.txt @@ -4,7 +4,7 @@ set(CRASH_STACK_BIN "crash-stack") # Common source code files INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/src) -set(CRASH_STACK_SRCS crash-stack.c crash-stack-libunw.c unwind.c proc.c mem_map.c) +set(CRASH_STACK_SRCS crash-stack.c crash-stack-libunw.c unwind.c proc.c mem_map.c ${CMAKE_SOURCE_DIR}/src/shared/util.c) # Add architecture dependent source files if (${CMAKE_SYSTEM_PROCESSOR} STREQUAL "aarch64") diff --git a/src/crash-stack/crash-stack.c b/src/crash-stack/crash-stack.c index 5979c2f..2f65b7c 100644 --- a/src/crash-stack/crash-stack.c +++ b/src/crash-stack/crash-stack.c @@ -50,6 +50,7 @@ #define LOG_TAG "CRASH_STACK" #include "shared/log.h" +#include "shared/util.h" #define BUF_SIZE (BUFSIZ) #define HEXA 16 @@ -257,22 +258,15 @@ void callstack_destructor(Callstack *callstack) */ static void __crash_stack_print_exe(FILE* outputfile, pid_t pid) { - int fd, ret; char file_path[PATH_MAX]; - char cmd_path[PATH_MAX]; + char pid_str[11]; - snprintf(cmd_path, PATH_MAX, "/proc/%d/cmdline", pid); - if ((fd = open(cmd_path, O_RDONLY)) < 0) - return; + snprintf(pid_str, sizeof(pid_str), "%d", pid); - if ((ret = read(fd, file_path, sizeof(file_path) - 1)) <= 0) { - close(fd); - return; - } - file_path[ret] = '\0'; + if (get_exe_path(pid_str, file_path, sizeof(file_path)) == 0) + snprintf(file_path, sizeof(file_path), ""); fprintf(outputfile, "Executable File Path: %s\n", file_path); - close(fd); } /** diff --git a/src/shared/util.c b/src/shared/util.c index 3795415..2eb4399 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -14,8 +14,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - +#ifndef _GNU_SOURCE #define _GNU_SOURCE +#endif #include #include #include @@ -817,6 +818,56 @@ int find_crash_tid(int pid) } return -1; } + +int get_cmd_line(char* pid, char *cmd, size_t len) +{ + char cmdline_path[PATH_MAX]; + + snprintf(cmdline_path, sizeof(cmdline_path), + "/proc/%s/cmdline", pid); + + int fd = open(cmdline_path, O_RDONLY); + if (fd < 0) { + _E("Failed to open %s: %m", cmdline_path); + return 0; + } + + ssize_t ret = read(fd, cmd, len - 1); + close(fd); + + if (ret <= 0) { + _E("Failed to read %s: %m", cmdline_path); + return 0; + } + + cmd[ret] = '\0'; + + return 1; +} + +int get_exe_path(char* pid, char *path, size_t len) +{ + char exe_link[PATH_MAX]; + + snprintf(exe_link, sizeof(exe_link), + "/proc/%s/exe", pid); + + ssize_t ret = readlink(exe_link, path, len - 1); + + if (ret <= 0) { + _E("Failed to read link %s: %m", exe_link); + return 0; + } + + path[ret] = '\0'; + + if (access(path, F_OK) == -1) { + _E("Invalid path %s", path); + return 0; + } + + return 1; +} /** * @} */ diff --git a/src/shared/util.h b/src/shared/util.h index 301b4e6..0cb35f2 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -62,6 +62,10 @@ int validate_file_name(char *name, int len); int log_kmsg(char *fmt, ...); int find_crash_tid(int pid); + +int get_exe_path(char* pid, char *path, size_t len); + +int get_cmd_line(char* pid, char *cmd, size_t len); /** * @} */ -- 2.7.4 From a86e51598e09ab3377ec6c7326f5c693ce7e1e8a Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Wed, 29 Aug 2018 13:06:11 +0200 Subject: [PATCH 12/16] Release 5.0.4 Changes from last version: - crash-stack use the same function to read /proc//exe symlink as the crash-manager - crash-manager: Simplify dump_core error reporting Change-Id: Id5d65baedcfe73c88e7aadee2cc7aef62c91269c --- packaging/crash-worker.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index 0ed23dc..56987f3 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -10,7 +10,7 @@ Name: crash-worker Summary: Crash-manager -Version: 5.0.3 +Version: 5.0.4 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From b91eba438a2f81bbbc4bf5744bb1a15addb4d25c Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Thu, 30 Aug 2018 13:38:39 +0200 Subject: [PATCH 13/16] Use parsed command line arguments instead of raw string values Change-Id: I28c9479c2d6cb5d754e7ff3ee37f8cab34235456 --- src/crash-manager/crash-manager.c | 100 +++++++++++++++++++------------------- src/crash-stack/crash-stack.c | 5 +- src/shared/util.c | 8 +-- src/shared/util.h | 4 +- 4 files changed, 57 insertions(+), 60 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index a778b97..3603907 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -113,12 +113,14 @@ static int report_type; /* Paths and variables */ static struct crash_info { - char *pid_info; - char *tid_info; - char *sig_info; + pid_t pid_info; + pid_t tid_info; + int uid_info; + int gid_info; + int sig_info; char cmd_line[PATH_MAX]; char cmd_path[PATH_MAX]; - char time_info[80]; + time_t time_info; char temp_dir[PATH_MAX]; char name[FILENAME_MAX]; char result_path[PATH_MAX]; @@ -437,7 +439,7 @@ static int set_prstatus() int ret; char prstatus_name[NAME_MAX+1]; - ret = snprintf(prstatus_name, NAME_MAX, "/%s.prstatus", crash_info.pid_info); + ret = snprintf(prstatus_name, NAME_MAX, "/%d.prstatus", crash_info.pid_info); if (ret < 0) { _E("Failed to snprintf for prstatus path"); goto close_fd; @@ -485,29 +487,18 @@ static int set_crash_info(int argc, char *argv[]) { int ret; char *temp_dir_ret = NULL; - time_t time_val; + char date[80]; struct tm loc_tm; - crash_info.pid_info = argv[1]; - crash_info.sig_info = argv[4]; - if (argc > 6) { - crash_info.tid_info = strdup(argv[6]); - if (crash_info.tid_info == NULL) { - _E("strdup error: %m"); - return -1; - } - } else { - crash_info.tid_info = NULL; - int pid = atoi(crash_info.pid_info); - int tid = find_crash_tid(pid); - if (tid < 0) { + crash_info.pid_info = strtol(argv[1], NULL, 10); + crash_info.sig_info = atoi(argv[4]); + if (argc > 6) + crash_info.tid_info = strtol(argv[6], NULL, 10); + else { + crash_info.tid_info = find_crash_tid(crash_info.pid_info); + if (crash_info.tid_info < 0) { _I("TID not found"); - tid = pid; - } - - if (asprintf(&crash_info.tid_info, "%d", tid) == -1) { - _E("asprintf error: %m"); - return -1; + crash_info.tid_info = crash_info.pid_info; } } @@ -517,10 +508,9 @@ static int set_crash_info(int argc, char *argv[]) return -1; } - time_val = atoll(argv[5]); - localtime_r(&time_val, &loc_tm); - strftime(crash_info.time_info, sizeof(crash_info.time_info), - "%Y%m%d%H%M%S", &loc_tm); + crash_info.time_info = strtol(argv[5], NULL, 10); + localtime_r(&crash_info.time_info, &loc_tm); + strftime(date, sizeof(date), "%Y%m%d%H%M%S", &loc_tm); ret = snprintf(crash_info.temp_dir, sizeof(crash_info.temp_dir), "%s/crash.XXXXXX", crash_temp_path); @@ -534,10 +524,10 @@ static int set_crash_info(int argc, char *argv[]) return -1; } - ret = snprintf(crash_info.name, sizeof(crash_info.name), "%s_%s_%s", + ret = snprintf(crash_info.name, sizeof(crash_info.name), "%s_%d_%s", basename(crash_info.cmd_line), crash_info.pid_info, - crash_info.time_info); + date); if (ret < 0) { _E("Failed to snprintf for name"); goto rm_temp; @@ -592,7 +582,7 @@ static int set_crash_info(int argc, char *argv[]) #ifdef SYS_ASSERT ret = snprintf(crash_info.sysassert_cs_path, sizeof(crash_info.sysassert_cs_path), - "/tmp/crash_stack/%s_%s.info", + "/tmp/crash_stack/%s_%d.info", basename(crash_info.cmd_line), crash_info.pid_info); if (ret < 0) { _E("Failed to snprintf for sys-assert callstack path"); @@ -719,7 +709,7 @@ static void copy_maps() { char maps_path[PATH_MAX]; char temp_maps_path[PATH_MAX]; - snprintf(maps_path, sizeof(maps_path), "/proc/%s/maps", + snprintf(maps_path, sizeof(maps_path), "/proc/%d/maps", crash_info.pid_info); snprintf(temp_maps_path, sizeof(temp_maps_path), "%s/%s", @@ -751,27 +741,36 @@ static void save_so_info() get_and_save_so_info(maps_path, so_info_path); } -static int execute_minicoredump(int argc, char *argv[]) +static int execute_minicoredump() { +#define SNPRINTF_OR_EXIT(name, format) if (snprintf(name##_str, sizeof(name##_str), format, crash_info.name##_info) < 0) goto out; + char *coredump_name = NULL; char *prstatus_fd_str = NULL; - int ret = 0; + int ret = -1; if (asprintf(&coredump_name, "%s.coredump", crash_info.name) == -1 || asprintf(&prstatus_fd_str, "%d", crash_info.prstatus_fd) == -1) { _E("Unable to allocate memory"); - ret = -1; goto out; } + char pid_str[11], uid_str[11], gid_str[11], sig_str[11], time_str[11]; + + SNPRINTF_OR_EXIT(pid, "%d") + SNPRINTF_OR_EXIT(uid, "%d") + SNPRINTF_OR_EXIT(gid, "%d") + SNPRINTF_OR_EXIT(sig, "%d") + SNPRINTF_OR_EXIT(time, "%ld") + /* Execute minicoredumper */ char *args[] = { MINICOREDUMPER_BIN_PATH, // minicoredumper filename path - crash_info.pid_info, // %p - pid - argv[2], // %u - UID - argv[3], // %g - GID - crash_info.sig_info, // %s - number of signal - argv[5], // %t - time of dump + pid_str, // %p - pid + uid_str, // %u - UID + gid_str, // %g - GID + sig_str, // %s - number of signal + time_str, // %t - time of dump "localhost", // %h - hostname "core", // %e - exe name (need for result filename) MINICOREDUMPER_CONFIG_PATH, // config file @@ -791,6 +790,7 @@ static int execute_minicoredump(int argc, char *argv[]) args[12], args[13], args[14]); run_command_timeout(args[0], args, NULL, MINICOREDUMPER_TIMEOUT); + ret = 0; /* Minicoredumper must be executed to dump at least PRSTATUS for other tools, coredump, however, might have been disabled. */ @@ -818,16 +818,17 @@ out: free(prstatus_fd_str); return ret; +#undef SNPRINTF_OR_EXIT } -static void execute_crash_stack(int argc, char *argv[]) +static void execute_crash_stack() { int ret; char command[PATH_MAX]; /* Execute crash-stack */ ret = snprintf(command, sizeof(command), - "%s --pid %s --tid %s --sig %s --prstatus_fd %d > %s", + "%s --pid %d --tid %d --sig %d --prstatus_fd %d > %s", CRASH_STACK_PATH, crash_info.pid_info, crash_info.tid_info, @@ -844,11 +845,11 @@ static void execute_crash_stack(int argc, char *argv[]) system_command(command); } -static int execute_crash_modules(int argc, char *argv[]) +static int execute_crash_modules() { _D("Execute crash module: "); - if (execute_minicoredump(argc, argv) < 0) { + if (execute_minicoredump() < 0) { _E("Failed to run minicoredumper - can not continue"); return -1; } @@ -859,7 +860,7 @@ static int execute_crash_modules(int argc, char *argv[]) if (crash_info.have_sysassert_report) return -1; #endif - execute_crash_stack(argc, argv); + execute_crash_stack(); return 0; } @@ -1242,7 +1243,7 @@ int main(int argc, char *argv[]) copy_maps(); } /* Exec crash modules */ - if (execute_crash_modules(argc, argv) < 0) { + if (execute_crash_modules() < 0) { res = EXIT_FAILURE; goto exit; } @@ -1269,8 +1270,8 @@ int main(int argc, char *argv[]) struct NotifyParams notify_params = { .prstatus_fd = crash_info.prstatus_fd, - .pid = strtol(crash_info.pid_info, NULL, 10), - .tid = strtol(crash_info.tid_info, NULL, 10), + .pid = crash_info.pid_info, + .tid = crash_info.tid_info, .cmd_name = basename(crash_info.cmd_line), .cmd_path = crash_info.cmd_path, .report_path = crash_info.result_path, @@ -1282,7 +1283,6 @@ int main(int argc, char *argv[]) exit: close(crash_info.prstatus_fd); - free(crash_info.tid_info); free(crash_temp_path); free(crash_root_path); free(crash_crash_path); diff --git a/src/crash-stack/crash-stack.c b/src/crash-stack/crash-stack.c index 2f65b7c..35516eb 100644 --- a/src/crash-stack/crash-stack.c +++ b/src/crash-stack/crash-stack.c @@ -259,11 +259,8 @@ void callstack_destructor(Callstack *callstack) static void __crash_stack_print_exe(FILE* outputfile, pid_t pid) { char file_path[PATH_MAX]; - char pid_str[11]; - snprintf(pid_str, sizeof(pid_str), "%d", pid); - - if (get_exe_path(pid_str, file_path, sizeof(file_path)) == 0) + if (get_exe_path(pid, file_path, sizeof(file_path)) == 0) snprintf(file_path, sizeof(file_path), ""); fprintf(outputfile, "Executable File Path: %s\n", file_path); diff --git a/src/shared/util.c b/src/shared/util.c index 2eb4399..97f3499 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -819,12 +819,12 @@ int find_crash_tid(int pid) return -1; } -int get_cmd_line(char* pid, char *cmd, size_t len) +int get_cmd_line(pid_t pid, char *cmd, size_t len) { char cmdline_path[PATH_MAX]; snprintf(cmdline_path, sizeof(cmdline_path), - "/proc/%s/cmdline", pid); + "/proc/%d/cmdline", pid); int fd = open(cmdline_path, O_RDONLY); if (fd < 0) { @@ -845,12 +845,12 @@ int get_cmd_line(char* pid, char *cmd, size_t len) return 1; } -int get_exe_path(char* pid, char *path, size_t len) +int get_exe_path(pid_t pid, char *path, size_t len) { char exe_link[PATH_MAX]; snprintf(exe_link, sizeof(exe_link), - "/proc/%s/exe", pid); + "/proc/%d/exe", pid); ssize_t ret = readlink(exe_link, path, len - 1); diff --git a/src/shared/util.h b/src/shared/util.h index 0cb35f2..5d40d91 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -63,9 +63,9 @@ int log_kmsg(char *fmt, ...); int find_crash_tid(int pid); -int get_exe_path(char* pid, char *path, size_t len); +int get_exe_path(pid_t pid, char *path, size_t len); -int get_cmd_line(char* pid, char *cmd, size_t len); +int get_cmd_line(pid_t pid, char *cmd, size_t len); /** * @} */ -- 2.7.4 From 35ef82f3738efc9872f5b6f926d47256a7fe38b2 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Fri, 31 Aug 2018 12:07:56 +0200 Subject: [PATCH 14/16] Stop using crash_info struct as a global variable Change-Id: I1dee72cb7abf29e7ff2bf5bdc4e0852bda1329b1 --- src/crash-manager/crash-manager.c | 256 +++++++++++++++++++------------------- 1 file changed, 129 insertions(+), 127 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 3603907..cf6525e 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -112,7 +112,7 @@ static char* crash_temp_path; static int report_type; /* Paths and variables */ -static struct crash_info { +struct crash_info { pid_t pid_info; pid_t tid_info; int uid_info; @@ -135,7 +135,7 @@ static struct crash_info { bool have_sysassert_report; #endif int prstatus_fd; -} crash_info; +}; /* pkgmgrinfo filter list function for getting application ID */ static int appinfo_get_appid_func(pkgmgrinfo_appinfo_h handle, @@ -434,19 +434,19 @@ static int get_cmd_info(struct crash_info *cinfo) get_exe_path(cinfo->pid_info, cinfo->cmd_path, sizeof(cinfo->cmd_path)); } -static int set_prstatus() +static int set_prstatus(struct crash_info *cinfo) { int ret; char prstatus_name[NAME_MAX+1]; - ret = snprintf(prstatus_name, NAME_MAX, "/%d.prstatus", crash_info.pid_info); + ret = snprintf(prstatus_name, NAME_MAX, "/%d.prstatus", cinfo->pid_info); if (ret < 0) { _E("Failed to snprintf for prstatus path"); goto close_fd; } - crash_info.prstatus_fd = shm_open(prstatus_name, O_RDWR | O_CREAT, 0600); - if (crash_info.prstatus_fd < 0) { + cinfo->prstatus_fd = shm_open(prstatus_name, O_RDWR | O_CREAT, 0600); + if (cinfo->prstatus_fd < 0) { _E("shm_open: %m"); goto close_fd; } @@ -457,19 +457,19 @@ static int set_prstatus() goto close_fd; } - ret = fcntl(crash_info.prstatus_fd, F_GETFD); + ret = fcntl(cinfo->prstatus_fd, F_GETFD); if (ret < 0) { _E("fcntl(): %m"); goto close_fd; } - ret = fcntl(crash_info.prstatus_fd, F_SETFD, ret & ~FD_CLOEXEC); + ret = fcntl(cinfo->prstatus_fd, F_SETFD, ret & ~FD_CLOEXEC); if (ret < 0) { _E("fcntl(): %m"); goto close_fd; } - ret = ftruncate(crash_info.prstatus_fd, sizeof(struct elf_prstatus)); + ret = ftruncate(cinfo->prstatus_fd, sizeof(struct elf_prstatus)); if (ret < 0) { _E("ftruncate(): %m"); goto close_fd; @@ -478,55 +478,55 @@ static int set_prstatus() return 0; close_fd: - if (crash_info.prstatus_fd >= 0) - close(crash_info.prstatus_fd); + if (cinfo->prstatus_fd >= 0) + close(cinfo->prstatus_fd); return -1; } -static int set_crash_info(int argc, char *argv[]) +static int set_crash_info(struct crash_info *cinfo, int argc, char *argv[]) { int ret; char *temp_dir_ret = NULL; char date[80]; struct tm loc_tm; - crash_info.pid_info = strtol(argv[1], NULL, 10); - crash_info.sig_info = atoi(argv[4]); + cinfo->pid_info = strtol(argv[1], NULL, 10); + cinfo->sig_info = atoi(argv[4]); if (argc > 6) - crash_info.tid_info = strtol(argv[6], NULL, 10); + cinfo->tid_info = strtol(argv[6], NULL, 10); else { - crash_info.tid_info = find_crash_tid(crash_info.pid_info); - if (crash_info.tid_info < 0) { + cinfo->tid_info = find_crash_tid(cinfo->pid_info); + if (cinfo->tid_info < 0) { _I("TID not found"); - crash_info.tid_info = crash_info.pid_info; + cinfo->tid_info = cinfo->pid_info; } } - ret = get_cmd_info(&crash_info); + ret = get_cmd_info(cinfo); if (ret <= 0) { _E("Failed to get command info"); return -1; } - crash_info.time_info = strtol(argv[5], NULL, 10); - localtime_r(&crash_info.time_info, &loc_tm); + cinfo->time_info = strtol(argv[5], NULL, 10); + localtime_r(&cinfo->time_info, &loc_tm); strftime(date, sizeof(date), "%Y%m%d%H%M%S", &loc_tm); - ret = snprintf(crash_info.temp_dir, sizeof(crash_info.temp_dir), + ret = snprintf(cinfo->temp_dir, sizeof(cinfo->temp_dir), "%s/crash.XXXXXX", crash_temp_path); if (ret < 0) { _E("Failed to snprintf for temp_dir"); return -1; } - temp_dir_ret = mkdtemp(crash_info.temp_dir); + temp_dir_ret = mkdtemp(cinfo->temp_dir); if (!temp_dir_ret || access(temp_dir_ret, F_OK)) { _E("Failed to mkdtemp for temp_dir"); return -1; } - ret = snprintf(crash_info.name, sizeof(crash_info.name), "%s_%d_%s", - basename(crash_info.cmd_line), - crash_info.pid_info, + ret = snprintf(cinfo->name, sizeof(cinfo->name), "%s_%d_%s", + basename(cinfo->cmd_line), + cinfo->pid_info, date); if (ret < 0) { _E("Failed to snprintf for name"); @@ -534,103 +534,103 @@ static int set_crash_info(int argc, char *argv[]) } if (allow_zip) - ret = snprintf(crash_info.result_path, - sizeof(crash_info.result_path), - "%s/%s.zip", crash_crash_path, crash_info.name); + ret = snprintf(cinfo->result_path, + sizeof(cinfo->result_path), + "%s/%s.zip", crash_crash_path, cinfo->name); else - ret = snprintf(crash_info.result_path, - sizeof(crash_info.result_path), - "%s/%s", crash_crash_path, crash_info.name); + ret = snprintf(cinfo->result_path, + sizeof(cinfo->result_path), + "%s/%s", crash_crash_path, cinfo->name); if (ret < 0) { _E("Failed to snprintf for result path"); goto rm_temp; } - ret = snprintf(crash_info.pfx, sizeof(crash_info.pfx), "%s/%s", - crash_info.temp_dir, crash_info.name); + ret = snprintf(cinfo->pfx, sizeof(cinfo->pfx), "%s/%s", + cinfo->temp_dir, cinfo->name); if (ret < 0) { _E("Failed to snprintf for pfx"); goto rm_temp; } - ret = mkdir(crash_info.pfx, 0775); + ret = mkdir(cinfo->pfx, 0775); if (ret < 0) { - _E("Failed to mkdir for %s", crash_info.pfx); + _E("Failed to mkdir for %s", cinfo->pfx); goto rm_temp; } - ret = snprintf(crash_info.info_path, sizeof(crash_info.info_path), - "%s/%s.info", crash_info.pfx, crash_info.name); + ret = snprintf(cinfo->info_path, sizeof(cinfo->info_path), + "%s/%s.info", cinfo->pfx, cinfo->name); if (ret < 0) { _E("Failed to snprintf for info path"); goto rm_temp; } - ret = snprintf(crash_info.core_path, sizeof(crash_info.core_path), - "%s/%s.coredump", crash_info.pfx, crash_info.name); + ret = snprintf(cinfo->core_path, sizeof(cinfo->core_path), + "%s/%s.coredump", cinfo->pfx, cinfo->name); if (ret < 0) { _E("Failed to snprintf for core path"); goto rm_temp; } - ret = snprintf(crash_info.log_path, sizeof(crash_info.log_path), - "%s/%s.log", crash_info.pfx, crash_info.name); + ret = snprintf(cinfo->log_path, sizeof(cinfo->log_path), + "%s/%s.log", cinfo->pfx, cinfo->name); if (ret < 0) { _E("Failed to snprintf for log path"); goto rm_temp; } #ifdef SYS_ASSERT - ret = snprintf(crash_info.sysassert_cs_path, - sizeof(crash_info.sysassert_cs_path), + ret = snprintf(cinfo->sysassert_cs_path, + sizeof(cinfo->sysassert_cs_path), "/tmp/crash_stack/%s_%d.info", - basename(crash_info.cmd_line), crash_info.pid_info); + basename(cinfo->cmd_line), cinfo->pid_info); if (ret < 0) { _E("Failed to snprintf for sys-assert callstack path"); goto rm_temp; } #endif - if (set_prstatus() < 0) + if (set_prstatus(cinfo) < 0) goto rm_temp; - if (get_appid(crash_info.cmd_line, crash_info.appid, sizeof(crash_info.appid)) < 0) { - snprintf(crash_info.appid, sizeof(crash_info.appid), "%s", basename(crash_info.cmd_line)); - snprintf(crash_info.pkgid, sizeof(crash_info.pkgid), "%s", basename(crash_info.cmd_line)); + if (get_appid(cinfo->cmd_line, cinfo->appid, sizeof(cinfo->appid)) < 0) { + snprintf(cinfo->appid, sizeof(cinfo->appid), "%s", basename(cinfo->cmd_line)); + snprintf(cinfo->pkgid, sizeof(cinfo->pkgid), "%s", basename(cinfo->cmd_line)); } else { - if (get_pkgid(crash_info.appid, crash_info.pkgid, sizeof(crash_info.pkgid)) < 0) - snprintf(crash_info.pkgid, sizeof(crash_info.pkgid), "%s", crash_info.appid); + if (get_pkgid(cinfo->appid, cinfo->pkgid, sizeof(cinfo->pkgid)) < 0) + snprintf(cinfo->pkgid, sizeof(cinfo->pkgid), "%s", cinfo->appid); } return 0; rm_temp: - remove_dir(crash_info.temp_dir, 1); + remove_dir(cinfo->temp_dir, 1); return -1; } #ifdef SYS_ASSERT -static int get_sysassert_cs(void) +static int get_sysassert_cs(struct crash_info *cinfo) { int ret; char move_path[PATH_MAX]; - if (access(crash_info.sysassert_cs_path, F_OK)) { + if (access(cinfo->sysassert_cs_path, F_OK)) { _E("The sys-assert cs file not found: %s", - crash_info.sysassert_cs_path); - crash_info.have_sysassert_report = 0; + cinfo->sysassert_cs_path); + cinfo->have_sysassert_report = 0; return -1; } else - crash_info.have_sysassert_report = 1; + cinfo->have_sysassert_report = 1; ret = snprintf(move_path, sizeof(move_path), "%s/%s", - crash_info.pfx, basename(crash_info.sysassert_cs_path)); + cinfo->pfx, basename(cinfo->sysassert_cs_path)); if (ret < 0) { _E("Failed to snprintf for move path"); return -1; } - if (move_file(crash_info.sysassert_cs_path, move_path) < 0) { + if (move_file(cinfo->sysassert_cs_path, move_path) < 0) { _E("Failed to move %s to %s", - crash_info.sysassert_cs_path, move_path); + cinfo->sysassert_cs_path, move_path); return -1; } @@ -638,7 +638,7 @@ static int get_sysassert_cs(void) } #endif -static void launch_crash_popup(void) +static void launch_crash_popup(struct crash_info *cinfo) { GDBusConnection *conn; GVariantBuilder *builder; @@ -657,8 +657,8 @@ static void launch_crash_popup(void) builder = g_variant_builder_new(G_VARIANT_TYPE("a{ss}")); g_variant_builder_add(builder, "{ss}", "_SYSPOPUP_CONTENT_", "crash"); g_variant_builder_add(builder, "{ss}", "_PROCESS_NAME_", - basename(crash_info.cmd_line)); - g_variant_builder_add(builder, "{ss}", "_EXEPATH_", crash_info.cmd_path); + basename(cinfo->cmd_line)); + g_variant_builder_add(builder, "{ss}", "_EXEPATH_", cinfo->cmd_path); parameters = g_variant_new("(a{ss})", builder); g_variant_builder_unref(builder); @@ -689,14 +689,14 @@ exit: g_variant_unref(parameters); } -static int dump_system_state(void) +static int dump_system_state(const struct crash_info *cinfo) { int ret; char command[PATH_MAX]; ret = snprintf(command, sizeof(command), "/usr/bin/dump_systemstate -d -k -j -f %s", - crash_info.log_path); + cinfo->log_path); if (ret < 0) { _E("Failed to snprintf for dump_systemstate command"); return -1; @@ -705,52 +705,52 @@ static int dump_system_state(void) return system_command_parallel(command); } -static void copy_maps() +static void copy_maps(const struct crash_info *cinfo) { char maps_path[PATH_MAX]; char temp_maps_path[PATH_MAX]; snprintf(maps_path, sizeof(maps_path), "/proc/%d/maps", - crash_info.pid_info); + cinfo->pid_info); snprintf(temp_maps_path, sizeof(temp_maps_path), "%s/%s", - crash_info.temp_dir, TEMP_MAPS_FILENAME); + cinfo->temp_dir, TEMP_MAPS_FILENAME); copy_file(maps_path, temp_maps_path); } -static void remove_maps() +static void remove_maps(const struct crash_info *cinfo) { char temp_maps_path[PATH_MAX]; snprintf(temp_maps_path, sizeof(temp_maps_path), "%s/%s", - crash_info.temp_dir, TEMP_MAPS_FILENAME); + cinfo->temp_dir, TEMP_MAPS_FILENAME); if (unlink(temp_maps_path) < 0) _E("Cannot remove %s: %m\n", temp_maps_path); } -static void save_so_info() +static void save_so_info(const struct crash_info *cinfo) { char maps_path[PATH_MAX]; char so_info_path[PATH_MAX]; snprintf(maps_path, sizeof(maps_path), "%s/%s", - crash_info.temp_dir, TEMP_MAPS_FILENAME); + cinfo->temp_dir, TEMP_MAPS_FILENAME); snprintf(so_info_path, sizeof(so_info_path), - "%s/%s.%s", crash_info.pfx, crash_info.name, "so_info"); + "%s/%s.%s", cinfo->pfx, cinfo->name, "so_info"); get_and_save_so_info(maps_path, so_info_path); } -static int execute_minicoredump() +static int execute_minicoredump(struct crash_info *cinfo) { -#define SNPRINTF_OR_EXIT(name, format) if (snprintf(name##_str, sizeof(name##_str), format, crash_info.name##_info) < 0) goto out; +#define SNPRINTF_OR_EXIT(name, format) if (snprintf(name##_str, sizeof(name##_str), format, cinfo->name##_info) < 0) goto out; char *coredump_name = NULL; char *prstatus_fd_str = NULL; int ret = -1; - if (asprintf(&coredump_name, "%s.coredump", crash_info.name) == -1 - || asprintf(&prstatus_fd_str, "%d", crash_info.prstatus_fd) == -1) { + if (asprintf(&coredump_name, "%s.coredump", cinfo->name) == -1 + || asprintf(&prstatus_fd_str, "%d", cinfo->prstatus_fd) == -1) { _E("Unable to allocate memory"); goto out; } @@ -775,7 +775,7 @@ static int execute_minicoredump() "core", // %e - exe name (need for result filename) MINICOREDUMPER_CONFIG_PATH, // config file "-d", - crash_info.pfx, // temp dir + cinfo->pfx, // temp dir "-o", coredump_name, // coredump filename "-P", @@ -797,7 +797,7 @@ static int execute_minicoredump() if (!dump_core) { int ret = -1; int errno_unlink = 0; - int dirfd = open(crash_info.pfx, O_DIRECTORY); + int dirfd = open(cinfo->pfx, O_DIRECTORY); if (dirfd != -1) { ret = unlinkat(dirfd, coredump_name, 0); errno_unlink = errno; @@ -807,10 +807,10 @@ static int execute_minicoredump() if (ret != 0) _E("Saving core disabled - removing coredump %s/%s failed: %m", - crash_info.pfx, coredump_name); + cinfo->pfx, coredump_name); else _D("Saving core disabled - removed coredump %s/%s", - crash_info.pfx, coredump_name); + cinfo->pfx, coredump_name); } out: @@ -821,7 +821,7 @@ out: #undef SNPRINTF_OR_EXIT } -static void execute_crash_stack() +static void execute_crash_stack(const struct crash_info *cinfo) { int ret; char command[PATH_MAX]; @@ -830,11 +830,11 @@ static void execute_crash_stack() ret = snprintf(command, sizeof(command), "%s --pid %d --tid %d --sig %d --prstatus_fd %d > %s", CRASH_STACK_PATH, - crash_info.pid_info, - crash_info.tid_info, - crash_info.sig_info, - crash_info.prstatus_fd, - crash_info.info_path); + cinfo->pid_info, + cinfo->tid_info, + cinfo->sig_info, + cinfo->prstatus_fd, + cinfo->info_path); _D(" %s", command); if (ret < 0) { @@ -845,11 +845,11 @@ static void execute_crash_stack() system_command(command); } -static int execute_crash_modules() +static int execute_crash_modules(struct crash_info *cinfo) { _D("Execute crash module: "); - if (execute_minicoredump() < 0) { + if (execute_minicoredump(cinfo) < 0) { _E("Failed to run minicoredumper - can not continue"); return -1; } @@ -857,10 +857,10 @@ static int execute_crash_modules() #ifdef SYS_ASSERT /* Use process_vm_readv() version as fallback if sys-assert * failed to generate report */ - if (crash_info.have_sysassert_report) + if (cinfo->have_sysassert_report) return -1; #endif - execute_crash_stack(); + execute_crash_stack(cinfo); return 0; } @@ -1100,21 +1100,21 @@ static void clean_dump(void) free(dump_list); } -static void compress(void) +static void compress(struct crash_info *cinfo) { int ret, lock_fd; char zip_path[PATH_MAX]; char command[PATH_MAX]; ret = snprintf(zip_path, sizeof(zip_path), "%s/report.zip", - crash_info.temp_dir); + cinfo->temp_dir); if (ret < 0) { _E("Failed to snprintf for zip path"); return; } ret = snprintf(command, sizeof(command), "cd %s && /bin/zip -y -r %s %s > /dev/null 2>&1", - crash_info.temp_dir, zip_path, crash_info.name); + cinfo->temp_dir, zip_path, cinfo->name); if (ret < 0) { _E("Failed to snprintf for zip command"); return; @@ -1123,50 +1123,50 @@ static void compress(void) if ((lock_fd = lock_dumpdir()) < 0) return; - if (!rename(zip_path, crash_info.result_path)) + if (!rename(zip_path, cinfo->result_path)) clean_dump(); else - _E("Failed to move %s to %s", zip_path, crash_info.result_path); + _E("Failed to move %s to %s", zip_path, cinfo->result_path); unlock_dumpdir(lock_fd); - if (remove_dir(crash_info.temp_dir, 1) < 0) + if (remove_dir(cinfo->temp_dir, 1) < 0) _E("Failed to delete temp directory"); } -static void move_dump_dir(void) +static void move_dump_dir(const struct crash_info *cinfo) { int lock_fd; if ((lock_fd = lock_dumpdir()) < 0) return; - if (!rename(crash_info.pfx, crash_info.result_path)) + if (!rename(cinfo->pfx, cinfo->result_path)) clean_dump(); else _E("Failed to move %s to %s", - crash_info.pfx, crash_info.result_path); + cinfo->pfx, cinfo->result_path); unlock_dumpdir(lock_fd); - if (remove_dir(crash_info.temp_dir, 1) < 0) + if (remove_dir(cinfo->temp_dir, 1) < 0) _E("Failed to delete temp directory"); } -static void move_info_file(void) +static void move_info_file(struct crash_info *cinfo) { int lock_fd; if ((lock_fd = lock_dumpdir()) < 0) return; - snprintf(crash_info.result_path, sizeof(crash_info.result_path), "%s/%s.info", - crash_crash_path, crash_info.name); - if (!rename(crash_info.info_path, crash_info.result_path)) + snprintf(cinfo->result_path, sizeof(cinfo->result_path), "%s/%s.info", + crash_crash_path, cinfo->name); + if (!rename(cinfo->info_path, cinfo->result_path)) clean_dump(); else _E("Failed to move %s to %s", - crash_info.info_path, crash_info.result_path); + cinfo->info_path, cinfo->result_path); unlock_dumpdir(lock_fd); - if (remove_dir(crash_info.temp_dir, 1) < 0) + if (remove_dir(cinfo->temp_dir, 1) < 0) _E("Failed to delete temp directory"); } @@ -1190,6 +1190,8 @@ static int wait_for_opt(unsigned int timeout) int main(int argc, char *argv[]) { + struct crash_info cinfo; + /* Execute dump_systemstate in parallel */ static int dump_state_pid; int debug_mode = access(DEBUGMODE_PATH, F_OK) == 0; @@ -1225,64 +1227,64 @@ int main(int argc, char *argv[]) } /* Set crash info */ - if (set_crash_info(argc, argv) < 0) { + if (set_crash_info(&cinfo, argc, argv) < 0) { res = EXIT_FAILURE; goto exit; } #ifdef SYS_ASSERT /* Fetch callstack of sys-assert */ - get_sysassert_cs(); + get_sysassert_cs(&cinfo); #endif if (report_type >= REP_TYPE_FULL) { /* Exec dump_systemstate */ - dump_state_pid = dump_system_state(); + dump_state_pid = dump_system_state(&cinfo); /* Copy maps file to temp dir */ - copy_maps(); + copy_maps(&cinfo); } /* Exec crash modules */ - if (execute_crash_modules() < 0) { + if (execute_crash_modules(&cinfo) < 0) { res = EXIT_FAILURE; goto exit; } if (report_type >= REP_TYPE_FULL) { /* Save shared objects info (file names, bulid IDs, rpm package names) */ - save_so_info(); - remove_maps(); + save_so_info(&cinfo); + remove_maps(&cinfo); /* Wait dump_system_state */ wait_system_command(dump_state_pid); /* Tar compression */ if (allow_zip) - compress(); + compress(&cinfo); else - move_dump_dir(); + move_dump_dir(&cinfo); } else { - move_info_file(); + move_info_file(&cinfo); } /* launch crash-popup only if the .debugmode file is exist*/ if (debug_mode) - launch_crash_popup(); + launch_crash_popup(&cinfo); struct NotifyParams notify_params = { - .prstatus_fd = crash_info.prstatus_fd, - .pid = crash_info.pid_info, - .tid = crash_info.tid_info, - .cmd_name = basename(crash_info.cmd_line), - .cmd_path = crash_info.cmd_path, - .report_path = crash_info.result_path, - .appid = crash_info.appid, - .pkgid = crash_info.pkgid + .prstatus_fd = cinfo.prstatus_fd, + .pid = cinfo.pid_info, + .tid = cinfo.tid_info, + .cmd_name = basename(cinfo.cmd_line), + .cmd_path = cinfo.cmd_path, + .report_path = cinfo.result_path, + .appid = cinfo.appid, + .pkgid = cinfo.pkgid }; send_notify(¬ify_params); exit: - close(crash_info.prstatus_fd); + close(cinfo.prstatus_fd); free(crash_temp_path); free(crash_root_path); free(crash_crash_path); -- 2.7.4 From 403dcf7bfaf147d74c62c23a6a0726aee65c44fe Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Fri, 31 Aug 2018 12:44:13 +0200 Subject: [PATCH 15/16] Merge move_dump_dir() and move_info_file() Change-Id: I246bf743eefb199577bd00d6b3fd0defa6948eff --- src/crash-manager/crash-manager.c | 32 +++++++------------------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index cf6525e..3aa16d6 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -1133,43 +1133,23 @@ static void compress(struct crash_info *cinfo) _E("Failed to delete temp directory"); } -static void move_dump_dir(const struct crash_info *cinfo) +static void move_dump_data(const char *from_path, const struct crash_info *cinfo) { int lock_fd; if ((lock_fd = lock_dumpdir()) < 0) return; - if (!rename(cinfo->pfx, cinfo->result_path)) + if (!rename(from_path, cinfo->result_path)) clean_dump(); else _E("Failed to move %s to %s", - cinfo->pfx, cinfo->result_path); + from_path, cinfo->result_path); unlock_dumpdir(lock_fd); if (remove_dir(cinfo->temp_dir, 1) < 0) _E("Failed to delete temp directory"); } -static void move_info_file(struct crash_info *cinfo) -{ - int lock_fd; - - if ((lock_fd = lock_dumpdir()) < 0) - return; - - snprintf(cinfo->result_path, sizeof(cinfo->result_path), "%s/%s.info", - crash_crash_path, cinfo->name); - if (!rename(cinfo->info_path, cinfo->result_path)) - clean_dump(); - else - _E("Failed to move %s to %s", - cinfo->info_path, cinfo->result_path); - - unlock_dumpdir(lock_fd); - if (remove_dir(cinfo->temp_dir, 1) < 0) - _E("Failed to delete temp directory"); -} - static int wait_for_opt(unsigned int timeout) { unsigned int count = 0; @@ -1262,9 +1242,11 @@ int main(int argc, char *argv[]) if (allow_zip) compress(&cinfo); else - move_dump_dir(&cinfo); + move_dump_data(cinfo.pfx, &cinfo); } else { - move_info_file(&cinfo); + snprintf(cinfo.result_path, sizeof(cinfo.result_path), "%s/%s.info", + crash_crash_path, cinfo.name); + move_dump_data(cinfo.info_path, &cinfo); } /* launch crash-popup only if the .debugmode file is exist*/ if (debug_mode) -- 2.7.4 From 441ff63bdb63c421183c929bd783ba1084ff719d Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Fri, 31 Aug 2018 14:04:49 +0200 Subject: [PATCH 16/16] Add timeout to run the crash-stack In case the crash-stack hangs, the crash-manager will still be able to create a raport. Change-Id: I62b3022dc1aec6507821473619c73cf90337643f --- src/crash-manager/crash-manager.c | 62 +++++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 3aa16d6..4c760bf 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -74,6 +74,7 @@ #define WAIT_FOR_OPT_TIMEOUT_SEC 60 #define MINICOREDUMPER_TIMEOUT 12*60 +#define CRASH_STACK_TIMEOUT 12*60 #define TEMP_MAPS_FILENAME "maps.temp" enum { @@ -741,10 +742,12 @@ static void save_so_info(const struct crash_info *cinfo) get_and_save_so_info(maps_path, so_info_path); } +// These macros are used in execute_minicoredump() and execute_crash_stack() +#define SNPRINTF_OR_EXIT_W(name, format, member) if (snprintf(name##_str, sizeof(name##_str), format, cinfo->member) < 0) goto out; +#define SNPRINTF_OR_EXIT(name, format) SNPRINTF_OR_EXIT_W(name, format, name##_info) + static int execute_minicoredump(struct crash_info *cinfo) { -#define SNPRINTF_OR_EXIT(name, format) if (snprintf(name##_str, sizeof(name##_str), format, cinfo->name##_info) < 0) goto out; - char *coredump_name = NULL; char *prstatus_fd_str = NULL; int ret = -1; @@ -818,33 +821,56 @@ out: free(prstatus_fd_str); return ret; -#undef SNPRINTF_OR_EXIT } -static void execute_crash_stack(const struct crash_info *cinfo) +static int execute_crash_stack(const struct crash_info *cinfo) { - int ret; - char command[PATH_MAX]; + int ret = -1; + int fd = -1; + + char pid_str[11], tid_str[11], sig_str[11], prstatus_fd_str[11]; + + SNPRINTF_OR_EXIT(pid, "%d") + SNPRINTF_OR_EXIT(tid, "%d") + SNPRINTF_OR_EXIT(sig, "%d") + SNPRINTF_OR_EXIT_W(prstatus_fd, "%d", prstatus_fd) /* Execute crash-stack */ - ret = snprintf(command, sizeof(command), - "%s --pid %d --tid %d --sig %d --prstatus_fd %d > %s", + char *args[] = { CRASH_STACK_PATH, - cinfo->pid_info, - cinfo->tid_info, - cinfo->sig_info, - cinfo->prstatus_fd, - cinfo->info_path); + "--pid", + pid_str, + "--tid", + tid_str, + "--sig", + sig_str, + "--prstatus_fd", + prstatus_fd_str, + NULL + }; - _D(" %s", command); - if (ret < 0) { - _E("Failed to snprintf for crash-stack command"); - return; + _D(" %s %s %s %s %s %s %s %s %s", + args[0], args[1], args[2], args[3], + args[4], args[5], args[6], args[7], args[8]); + + fd = open(cinfo->info_path, O_WRONLY | O_CREAT, 0600); + if (fd < 0) { + _E("open %s error: %m", cinfo->info_path); + goto out; } - system_command(command); + ret = run_command_write_fd_timeout(CRASH_STACK_PATH, args, NULL, fd, NULL, 0, CRASH_STACK_TIMEOUT); + +out: + if (fd >= 0) + close(fd); + + return ret; } +#undef SNPRINTF_OR_EXIT +#undef SNPRINTF_OR_EXIT_W + static int execute_crash_modules(struct crash_info *cinfo) { _D("Execute crash module: "); -- 2.7.4