From 637e65d99a0fbe224753b73307e5e5143f257e97 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 18 Jul 2019 13:32:48 +0200 Subject: [PATCH 01/16] crash-manager: dbus: Add information about signal that caused crash Signal number is going to be available under "sys.signal" key in broadcasted signal. Change-Id: I1c1e57e9229ca0d896ce095782d3c510ab5fe96e --- src/crash-manager/crash-manager.c | 4 +++- src/crash-manager/dbus_notify.c | 14 +++++++++++++- tests/system/dbus_notify/dbus_notify.sh.template | 10 +++++++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index bf55763..59b0537 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -679,7 +679,7 @@ static void launch_dbus_notify(struct crash_info *cinfo) { assert(cinfo); - char pid_str[11], tid_str[11]; + char pid_str[11], tid_str[11], sig_str[11]; char *prstatus_fd_str = NULL; if (asprintf(&prstatus_fd_str, "%d", cinfo->prstatus_fd) == -1) { @@ -689,6 +689,7 @@ static void launch_dbus_notify(struct crash_info *cinfo) SNPRINTF_OR_EXIT(pid, "%d") SNPRINTF_OR_EXIT(tid, "%d") + SNPRINTF_OR_EXIT(sig, "%d") char *av[] = { CRASH_NOTIFY_BIN_PATH, "--cmdline", cinfo->cmd_line, @@ -699,6 +700,7 @@ static void launch_dbus_notify(struct crash_info *cinfo) "--pkgid", cinfo->pkgid, "--reportpath", cinfo->result_path, "--prstatus_fd", prstatus_fd_str, + "--signal", sig_str, NULL }; spawn(av, NULL, NULL, NULL, NULL); diff --git a/src/crash-manager/dbus_notify.c b/src/crash-manager/dbus_notify.c index 3a87808..8ba0c3c 100644 --- a/src/crash-manager/dbus_notify.c +++ b/src/crash-manager/dbus_notify.c @@ -55,6 +55,7 @@ struct NotifyParams { int prstatus_fd; pid_t pid; pid_t tid; + int signal; char *cmd_name; char *cmd_path; char *report_path; @@ -165,6 +166,13 @@ static GVariant* build_message_data(const struct NotifyParams *notify_params) g_variant_builder_open(&md_builder, G_VARIANT_TYPE("a{sv}")); + if (notify_params->signal) { + g_variant_builder_open(&md_builder, G_VARIANT_TYPE("{sv}")); + g_variant_builder_add(&md_builder, "s", "sys.signal"); + g_variant_builder_add(&md_builder, "v", g_variant_new_int32(notify_params->signal)); + g_variant_builder_close(&md_builder); + } + struct RegInfo *reg_info; int regs_count = _get_important_registers(notify_params->prstatus_fd, ®_info); @@ -239,6 +247,7 @@ static bool parse_cmdline(int ac, char *av[], struct NotifyParams *params) FLAG_PKGID, FLAG_REPORTPATH, FLAG_PRSTATUS_FD, + FLAG_SIGNAL, }; static const struct option options[] = { { .name = "cmdline", .has_arg = required_argument, .flag = NULL, .val = FLAG_CMDLINE }, @@ -249,6 +258,7 @@ static bool parse_cmdline(int ac, char *av[], struct NotifyParams *params) { .name = "pkgid", .has_arg = required_argument, .flag = NULL, .val = FLAG_PKGID }, { .name = "reportpath", .has_arg = required_argument, .flag = NULL, .val = FLAG_REPORTPATH }, { .name = "prstatus_fd", .has_arg = required_argument, .flag = NULL, .val = FLAG_PRSTATUS_FD }, + { .name = "signal", .has_arg = required_argument, .flag = NULL, .val = FLAG_SIGNAL }, { NULL }, }; @@ -272,6 +282,8 @@ static bool parse_cmdline(int ac, char *av[], struct NotifyParams *params) params->pkgid = optarg; else if (FLAG_PRSTATUS_FD == val) params->prstatus_fd = atoi(optarg); + else if (FLAG_SIGNAL == val) + params->signal = atoi(optarg); } while (val != -1); return params->cmd_name && params->cmd_path && params->appid && params->pkgid && params->prstatus_fd > 0; @@ -281,7 +293,7 @@ static void usage(const char *const progname) { assert(progname); - printf("%s --prstatus_fd N --pid PID --tid TID --cmdline CMDLINE --cmdpath CMDPATH --appid APPID --pkgid PKGID\n", progname); + printf("%s --prstatus_fd N --pid PID --tid TID --cmdline CMDLINE --cmdpath CMDPATH --appid APPID --pkgid PKGID --signal SIGNALNR\n", progname); } int main(int ac, char *av[]) diff --git a/tests/system/dbus_notify/dbus_notify.sh.template b/tests/system/dbus_notify/dbus_notify.sh.template index c7e7349..89972cc 100644 --- a/tests/system/dbus_notify/dbus_notify.sh.template +++ b/tests/system/dbus_notify/dbus_notify.sh.template @@ -27,6 +27,10 @@ fi # string "arm.lr" # variant uint32 3070220008 # ) +# dict entry( +# string "sys.signal" +# variant int32 6 +# ) # ] ( ${CRASH_WORKER_SYSTEM_TESTS}/utils/kenny & @@ -64,7 +68,11 @@ for i in $(seq 1 10); do score=$(($score + 1)) fi - if [ $score -eq 3 ]; then + if egrep -A1 "string \"sys.signal" $TMPFILE | grep 'int32 6'; then + score=$(($score + 1)) + fi + + if [ $score -eq 4 ]; then exit_with_code "SUCCESS" ${SUCCESS_CODE} fi fi -- 2.7.4 From db04cb109be580223004b8f22470428be5dae4ea Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Fri, 19 Jul 2019 15:20:15 +0200 Subject: [PATCH 02/16] defs: Make comm size public It will be used by multiple clients. Change-Id: Ifa86c14a0b1cf3b0c1cf7693c1f3c836c95a0d2d --- include/defs.h.in | 2 ++ src/crash-manager/dbus_notify.c | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/include/defs.h.in b/include/defs.h.in index b6eab96..a333ff7 100644 --- a/include/defs.h.in +++ b/include/defs.h.in @@ -1,6 +1,8 @@ #ifndef __DEFS_H__ #define __DEFS_H__ +#define KERNEL_DEFINED_TASK_COMM_LEN 16 // from include/linux/sched.h + #define CRASH_PATH "@CRASH_PATH@" #define CRASH_ROOT_PATH "@CRASH_ROOT_PATH@" #define CRASH_TEMP "@CRASH_TEMP@" diff --git a/src/crash-manager/dbus_notify.c b/src/crash-manager/dbus_notify.c index 8ba0c3c..9c42dec 100644 --- a/src/crash-manager/dbus_notify.c +++ b/src/crash-manager/dbus_notify.c @@ -20,6 +20,7 @@ #define LOG_TAG "CRASH_MANAGER" #include "shared/log.h" #include "dbus-util.h" +#include "defs.h" #include #include @@ -40,8 +41,6 @@ #define CRASH_INTERFACE_CRASH CRASH_INTERFACE_NAME".Crash" #define PROCESS_CRASHED "ProcessCrashed" -#define KERNEL_DEFINED_TASK_COMM_LEN 16 // from include/linux/sched.h - #define ARM_REG_LR 14 #define ARM_REG_PC 15 #define AARCH64_REG_LR 30 -- 2.7.4 From e0121a8b42ed0d6f83b91aa2e967ac00524a2851 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Fri, 19 Jul 2019 15:22:54 +0200 Subject: [PATCH 03/16] shared: Generalize function to read any /proc/pid entry Change-Id: Ide42d323a2ce6f3901d7a5c26baa9ff5153a512f --- src/crash-manager/crash-manager.c | 23 +++++++++++---- src/shared/util.c | 60 ++++++++++++++++++++++++++------------- src/shared/util.h | 4 ++- 3 files changed, 61 insertions(+), 26 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 59b0537..3cbc777 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -251,12 +251,26 @@ static int make_dump_dir(void) return 0; } -static int get_cmd_info(struct crash_info *cinfo) +static bool get_cmd_info(struct crash_info *cinfo) { - cinfo->cmd_line = get_cmd_line(cinfo->pid_info); + assert(cinfo); + + cinfo->cmd_line = malloc(PATH_MAX); + if (!cinfo->cmd_line) + return false; + + if (!read_proc_file(cinfo->pid_info, "cmdline", cinfo->cmd_line, PATH_MAX, NULL)) + goto err; + cinfo->cmd_path = get_exe_path(cinfo->pid_info); + if (!cinfo->cmd_path) + goto err; - return (cinfo->cmd_line != NULL && cinfo->cmd_path != NULL); + return true; + +err: + free(cinfo->cmd_line); + return false; } static int set_prstatus(struct crash_info *cinfo) @@ -461,8 +475,7 @@ static int set_crash_info(struct crash_info *cinfo, int argc, char *argv[]) } } - ret = get_cmd_info(cinfo); - if (ret <= 0) { + if (!get_cmd_info(cinfo)) { _E("Failed to get command info"); return -1; } diff --git a/src/shared/util.c b/src/shared/util.c index f3685d4..84fa25e 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -14,6 +14,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + +#include #include #include #include @@ -467,36 +469,54 @@ int find_crash_tid(int pid) return -1; } -char* get_cmd_line(pid_t pid) +bool filter_drop_trailing_whitespace(char *buf, int size, int datalen) { - char cmdline_path[PATH_MAX]; + assert(buf); + assert(datalen <= size); - snprintf(cmdline_path, sizeof(cmdline_path), - "/proc/%d/cmdline", pid); + bool filtered = false; - int fd = open(cmdline_path, O_RDONLY); - if (fd < 0) { - _E("Failed to open %s: %m\n", cmdline_path); - return NULL; + for (int i = datalen; i >= 0 && isspace(buf[i]); --i) { + buf[i] = '\0'; + filtered = true; } - char buffer[PATH_MAX]; - ssize_t ret = read(fd, buffer, sizeof(buffer) - 1); - close(fd); + return filtered; +} - if (ret <= 0) { - _E("Failed to read %s: %m\n", cmdline_path); - return NULL; +bool read_proc_file(pid_t pid, const char * const file, char *outbuf, size_t bufsize, charp0filter filter) +{ + assert(file); + assert(outbuf); + assert(bufsize > 0); + + char path[PATH_MAX]; + + snprintf(path, sizeof(path), "/proc/%d/%s", pid, file); + + int fd = open(path, O_RDONLY); + if (fd < 0) { + _E("Failed to open %s: %m\n", path); + return false; } - buffer[ret] = '\0'; - char *result; - if (asprintf(&result, "%s", buffer) == -1) { - _E("asprintf() error: %m\n"); - return NULL; + ssize_t ret = read(fd, outbuf, bufsize - 1); + if (ret < 0) { + _E("Failed to read %s: %m\n", path); + goto err_close; } + outbuf[ret] = '\0'; - return result; + close(fd); + + if (ret > 0 && filter) + (void)filter(outbuf, bufsize, ret - 1); + + return true; + +err_close: + close(fd); + return false; } char* get_exe_path(pid_t pid) diff --git a/src/shared/util.h b/src/shared/util.h index 561c8bb..0f81ca7 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -60,7 +60,9 @@ int find_crash_tid(int pid); char* get_exe_path(pid_t pid); -char* get_cmd_line(pid_t pid); +typedef bool (*charp0filter)(char *buf, int size, int datalen); +bool filter_drop_trailing_whitespace(char *buf, int size, int datalen); +bool read_proc_file(pid_t pid, const char * const file, char *outbuf, size_t bufsize, charp0filter filter); char* concatenate(char *const vec[]); -- 2.7.4 From 1e86859b1b1af18207290407f84e0c7e10aa2faf Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Fri, 19 Jul 2019 15:23:42 +0200 Subject: [PATCH 04/16] crash-manager: dbus: Add /proc/tid/comm info to signal Change-Id: Ibfbc159204b54a9c4a2eaacc491c56a950e246b5 --- src/crash-manager/crash-manager.c | 4 ++++ src/crash-manager/dbus_notify.c | 12 ++++++++++++ tests/system/dbus_notify/dbus_notify.sh.template | 10 +++++++++- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 3cbc777..b275dbd 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -694,6 +694,9 @@ static void launch_dbus_notify(struct crash_info *cinfo) char pid_str[11], tid_str[11], sig_str[11]; char *prstatus_fd_str = NULL; + char tid_comm_str[KERNEL_DEFINED_TASK_COMM_LEN + 1] = { 0, }; + + (void)read_proc_file(cinfo->tid_info, "comm", tid_comm_str, sizeof(tid_comm_str), filter_drop_trailing_whitespace); if (asprintf(&prstatus_fd_str, "%d", cinfo->prstatus_fd) == -1) { _E("Unable to allocate memory: %m"); @@ -714,6 +717,7 @@ static void launch_dbus_notify(struct crash_info *cinfo) "--reportpath", cinfo->result_path, "--prstatus_fd", prstatus_fd_str, "--signal", sig_str, + "--tid-comm", tid_comm_str, NULL }; spawn(av, NULL, NULL, NULL, NULL); diff --git a/src/crash-manager/dbus_notify.c b/src/crash-manager/dbus_notify.c index 9c42dec..0254092 100644 --- a/src/crash-manager/dbus_notify.c +++ b/src/crash-manager/dbus_notify.c @@ -60,6 +60,7 @@ struct NotifyParams { char *report_path; char *appid; char *pkgid; + char *tid_comm; }; static int _get_important_registers(int fd, struct RegInfo **reg_info) @@ -172,6 +173,13 @@ static GVariant* build_message_data(const struct NotifyParams *notify_params) g_variant_builder_close(&md_builder); } + if (notify_params->tid_comm) { + g_variant_builder_open(&md_builder, G_VARIANT_TYPE("{sv}")); + g_variant_builder_add(&md_builder, "s", "sys.tid.comm"); + g_variant_builder_add(&md_builder, "v", g_variant_new_string(notify_params->tid_comm)); + g_variant_builder_close(&md_builder); + } + struct RegInfo *reg_info; int regs_count = _get_important_registers(notify_params->prstatus_fd, ®_info); @@ -247,6 +255,7 @@ static bool parse_cmdline(int ac, char *av[], struct NotifyParams *params) FLAG_REPORTPATH, FLAG_PRSTATUS_FD, FLAG_SIGNAL, + FLAG_TID_COMM, }; static const struct option options[] = { { .name = "cmdline", .has_arg = required_argument, .flag = NULL, .val = FLAG_CMDLINE }, @@ -258,6 +267,7 @@ static bool parse_cmdline(int ac, char *av[], struct NotifyParams *params) { .name = "reportpath", .has_arg = required_argument, .flag = NULL, .val = FLAG_REPORTPATH }, { .name = "prstatus_fd", .has_arg = required_argument, .flag = NULL, .val = FLAG_PRSTATUS_FD }, { .name = "signal", .has_arg = required_argument, .flag = NULL, .val = FLAG_SIGNAL }, + { .name = "tid-comm", .has_arg = required_argument, .flag = NULL, .val = FLAG_TID_COMM }, { NULL }, }; @@ -283,6 +293,8 @@ static bool parse_cmdline(int ac, char *av[], struct NotifyParams *params) params->prstatus_fd = atoi(optarg); else if (FLAG_SIGNAL == val) params->signal = atoi(optarg); + else if (FLAG_TID_COMM == val) + params->tid_comm = optarg; } while (val != -1); return params->cmd_name && params->cmd_path && params->appid && params->pkgid && params->prstatus_fd > 0; diff --git a/tests/system/dbus_notify/dbus_notify.sh.template b/tests/system/dbus_notify/dbus_notify.sh.template index 89972cc..9e51679 100644 --- a/tests/system/dbus_notify/dbus_notify.sh.template +++ b/tests/system/dbus_notify/dbus_notify.sh.template @@ -31,6 +31,10 @@ fi # string "sys.signal" # variant int32 6 # ) +# dict entry( +# string "sys.tid.comm" +# variant string "sleep" +# ) # ] ( ${CRASH_WORKER_SYSTEM_TESTS}/utils/kenny & @@ -72,7 +76,11 @@ for i in $(seq 1 10); do score=$(($score + 1)) fi - if [ $score -eq 4 ]; then + if egrep -A1 "string \"sys.tid.comm" $TMPFILE | egrep 'variant.*string \"kenny\"'; then + score=$(($score + 1)) + fi + + if [ $score -eq 5 ]; then exit_with_code "SUCCESS" ${SUCCESS_CODE} fi fi -- 2.7.4 From bc1937017cad065cb53fbd5a34d5e89204535214 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Tue, 16 Jul 2019 08:16:04 +0200 Subject: [PATCH 05/16] Save livedumper reports in the livedump/ subdirectory This change makes possible to distinguish the report created by the application crash from that created on the request. Change-Id: Iaafb82522cad40e12366ee06eb6313027937b5b2 --- src/crash-manager/crash-manager.c | 43 ++++++++++++++++---------- tests/system/livedumper/livedumper.sh.template | 4 +-- tests/system/utils/minicore-utils.sh | 5 +++ 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index b275dbd..030a1ce 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -59,6 +59,7 @@ #define CRASH_TEMP_SUBDIR "/temp/" #define CRASH_PATH_SUBDIR "/dump/" +#define LIVE_PATH_SUBDIR "/livedump/" #define WAIT_FOR_OPT_TIMEOUT_SEC 60 @@ -199,16 +200,18 @@ out: return ret; } -static int prepare_paths(void) +static int prepare_paths(struct crash_info* cinfo) { int tmp_len; - tmp_len = strlen(config.crash_root_path) + strlen(CRASH_PATH_SUBDIR); + const char *crash_subdir = cinfo->livedump ? LIVE_PATH_SUBDIR : CRASH_PATH_SUBDIR; + + tmp_len = strlen(config.crash_root_path) + strlen(crash_subdir); crash_crash_path = (char*)malloc(tmp_len + 1); if (crash_crash_path == NULL) { _E("Couldn't allocate memory for crash_crash_path: %m\n"); return 0; } - snprintf(crash_crash_path, tmp_len + 1, "%s%s", config.crash_root_path, CRASH_PATH_SUBDIR); + snprintf(crash_crash_path, tmp_len + 1, "%s%s", config.crash_root_path, crash_subdir); tmp_len = strlen(config.crash_root_path) + strlen(CRASH_TEMP_SUBDIR); crash_temp_path = (char*)malloc(tmp_len + 1); @@ -434,22 +437,13 @@ static bool parse_args(struct crash_info *cinfo, int argc, char *argv[]) #undef GET_NUMBER } -static int set_crash_info(struct crash_info *cinfo, int argc, char *argv[]) +static int set_crash_info(struct crash_info *cinfo) { int ret; char *temp_dir_ret = NULL; char date[80]; struct tm loc_tm; - cinfo->livedump = false; - cinfo->kill = false; - cinfo->print_result_path = false; - cinfo->tid_info = -1; - cinfo->time_info = 0; - - if (!parse_args(cinfo, argc, argv)) - return -1; - if (cinfo->livedump) { if (cinfo->kill) cinfo->sig_info = 9; @@ -1230,15 +1224,27 @@ static void free_crash_info(struct crash_info *cinfo) #endif } +static void crash_info_init(struct crash_info *cinfo) +{ + cinfo->prstatus_fd = -1; + cinfo->livedump = false; + cinfo->kill = false; + cinfo->print_result_path = false; + cinfo->tid_info = -1; + cinfo->time_info = 0; +} + int main(int argc, char *argv[]) { - struct crash_info cinfo = {.prstatus_fd = -1}; + struct crash_info cinfo; /* Execute processes in parallel */ static pid_t dump_state_pid = 0, extra_script_pid = 0; int debug_mode = access(DEBUGMODE_PATH, F_OK) == 0; int res = 0; + crash_info_init(&cinfo); + /* * prctl(PR_SET_DUMPABLE, 0) is not neccessary. Kernel runs the * crash-manager and sets RLIMIT_CORE to 1 for the process. This is special @@ -1250,7 +1256,12 @@ int main(int argc, char *argv[]) goto exit; } - if (!prepare_paths()) { + if (!parse_args(&cinfo, argc, argv)) { + res = EXIT_FAILURE; + goto exit; + } + + if (!prepare_paths(&cinfo)) { res = EXIT_FAILURE; goto exit; } @@ -1267,7 +1278,7 @@ int main(int argc, char *argv[]) } /* Set crash info */ - if (set_crash_info(&cinfo, argc, argv) < 0) { + if (set_crash_info(&cinfo) < 0) { res = EXIT_FAILURE; goto exit; } diff --git a/tests/system/livedumper/livedumper.sh.template b/tests/system/livedumper/livedumper.sh.template index 51c9be1..d22e073 100644 --- a/tests/system/livedumper/livedumper.sh.template +++ b/tests/system/livedumper/livedumper.sh.template @@ -29,7 +29,7 @@ REPORT_PATH=`echo ${CRASH_MANAGER_OUTPUT} | cut -c 13-` wait_for_app crash-manager -pushd ${CRASH_DUMP_PATH} +pushd ${LIVE_DUMP_PATH} if [ ! -f ${REPORT_PATH} ]; then exit_with_code "Report path does not exist" ${FAIL_CODE} @@ -37,7 +37,7 @@ fi if ! unzip ${REPORT_PATH} > /dev/null; then popd - exit_with_code "FAIL: report not found in ${CRASH_DUMP_PATH}" ${FAIL_CODE} + exit_with_code "FAIL: report not found in ${LIVE_DUMP_PATH}" ${FAIL_CODE} fi COREDUMP=`find -name "kenny*coredump*"` diff --git a/tests/system/utils/minicore-utils.sh b/tests/system/utils/minicore-utils.sh index 94fa8bf..747809a 100644 --- a/tests/system/utils/minicore-utils.sh +++ b/tests/system/utils/minicore-utils.sh @@ -151,6 +151,11 @@ function __export_vars__ { exit_with_code "Couldn't get TZ_SYS_CRASH or TZ_SYS_CRASH is empty" ${FAIL_CODE} fi + export LIVE_DUMP_PATH=`tzplatform_var TZ_SYS_CRASH_ROOT`/livedump + if [ -z ${LIVE_DUMP_PATH} ]; then + exit_with_code "Couldn't get TZ_SYS_CRASH_ROOT or TZ_SYS_CRASH_ROOT is empty" ${FAIL_CODE} + fi + export SUCCESS_CODE=0 export FAIL_CODE=1 export SKIP_CODE=2 -- 2.7.4 From e9fd7ab73ff68a98b4585ed0155f41930e5748a4 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Thu, 18 Jul 2019 17:58:38 +0200 Subject: [PATCH 06/16] Pass the executable name to the minicoredumper Change-Id: Iccd4abead0a87873d05e05333630bd859d34ef46 --- src/crash-manager/crash-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 030a1ce..fa24663 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -748,7 +748,7 @@ static bool execute_minicoredump(struct crash_info *cinfo, int *exit_code) sig_str, // %s - number of signal time_str, // %t - time of dump "localhost", // %h - hostname - "core", // %e - exe name (need for result filename) + basename(cinfo->cmd_line), // %e - exe name (need for result filename) MINICOREDUMPER_CONFIG_PATH, // config file "-d", cinfo->pfx, // temp dir -- 2.7.4 From 0457a61a846230e32e130e352550de6d086b2326 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Tue, 23 Jul 2019 17:25:00 +0200 Subject: [PATCH 07/16] Release 5.5.18 This release introduces following features: - Addition of signal that caused crash and thread name to dbus signal (in sys.signal and sys.tid.comm fields, respectively) Additionally, report path is appended properly to the signal (bugfix) - Livecoredump reports are saved in livedump/ subdirectory from this release - Executable name is passed to minicoredumper allowing per-app recipes to be specified - All executables are rebuild as PIE as needed for ASLR to work Change-Id: I91b2222c603d1d75b5e3df501f3d57eed5d7ff4e --- packaging/crash-worker.spec | 2 +- packaging/crash-worker_system-tests.spec | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index 304659d..51abe84 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -14,7 +14,7 @@ Name: crash-worker Summary: Crash-manager -Version: 5.5.17 +Version: 5.5.18 Release: 1 Group: Framework/system License: Apache-2.0 and BSD diff --git a/packaging/crash-worker_system-tests.spec b/packaging/crash-worker_system-tests.spec index 2caa93e..666fd44 100644 --- a/packaging/crash-worker_system-tests.spec +++ b/packaging/crash-worker_system-tests.spec @@ -8,7 +8,7 @@ Name: crash-worker_system-tests Summary: Package with binaries and scripts for crash-worker system tests -Version: 5.5.17 +Version: 5.5.18 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From b48f63eabfcd6fc329702a2637a6112c02d9f7f5 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 25 Jul 2019 13:13:09 +0200 Subject: [PATCH 08/16] system tests: Simplify exit helper functions Change-Id: Iecafa3632ac0c81454c2692037de34b486dfa2e2 --- .../check_minicore_mem.sh.template | 2 +- .../cmp_backtraces/cmp_backtraces.sh.template | 4 +-- .../crash_root_path/crash_root_path.sh.template | 6 ++-- .../critical_process/critical_process.sh.template | 6 ++-- tests/system/dbus_notify/dbus_notify.sh.template | 4 +-- .../dump_systemstate_extras.sh.template | 4 +-- tests/system/extra_script/extra_script.sh.template | 2 +- tests/system/info_file/info_file.sh.template | 6 ++-- tests/system/livedumper/livedumper.sh.template | 10 +++--- .../log_dump_crash_root_path.sh.template | 4 +-- .../log_dump_normal/log_dump_normal.sh.template | 2 +- .../log_dump_short/log_dump_short.sh.template | 4 +-- tests/system/log_file/log_file.sh.template | 8 ++--- tests/system/report_basic/report_basic.sh.template | 16 ++++----- .../report_type_info/report_type_info.sh.template | 4 +-- tests/system/so_info_file/so_info_file.sh.template | 6 ++-- tests/system/time_test/time_test.sh.template | 4 +-- tests/system/utils/minicore-utils.sh | 40 ++++++++++++++-------- .../wait_for_opt_usr/wait_for_opt_usr.sh.template | 4 +-- tests/system/without_core/without_core.sh.template | 6 ++-- 20 files changed, 77 insertions(+), 65 deletions(-) diff --git a/tests/system/check_minicore_mem/check_minicore_mem.sh.template b/tests/system/check_minicore_mem/check_minicore_mem.sh.template index 7858da9..b1532bb 100755 --- a/tests/system/check_minicore_mem/check_minicore_mem.sh.template +++ b/tests/system/check_minicore_mem/check_minicore_mem.sh.template @@ -35,4 +35,4 @@ RESULT=`gdb ${CRASH_WORKER_SYSTEM_TESTS}/utils/kenny ${BASE_DIR}/${CORE_MINI} -- check "MAGICNAME.*id=.*kenny.cpp:31" check "run.*id=.*kenny.cpp:56" -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/cmp_backtraces/cmp_backtraces.sh.template b/tests/system/cmp_backtraces/cmp_backtraces.sh.template index 9bbb081..417cadd 100755 --- a/tests/system/cmp_backtraces/cmp_backtraces.sh.template +++ b/tests/system/cmp_backtraces/cmp_backtraces.sh.template @@ -48,8 +48,8 @@ gdb ${CRASH_WORKER_SYSTEM_TESTS}/utils/kenny ${BASE_DIR}/${CORE_ORIG} -ex "thre gdb ${CRASH_WORKER_SYSTEM_TESTS}/utils/kenny ${BASE_DIR}/${CORE_MINI} -ex "thread apply all bt" -ex "q" 2> /dev/null | grep -e '^#' > ${BASE_DIR}/${THREADS_MINI} if ! diff ${BASE_DIR}/${THREADS_ORIG} ${BASE_DIR}/${THREADS_MINI} > /dev/null; then - exit_with_code "FAIL: backtraces are different" ${FAIL_CODE} + fail "backtraces are different" fi -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/crash_root_path/crash_root_path.sh.template b/tests/system/crash_root_path/crash_root_path.sh.template index bffda78..0f59b55 100644 --- a/tests/system/crash_root_path/crash_root_path.sh.template +++ b/tests/system/crash_root_path/crash_root_path.sh.template @@ -18,7 +18,7 @@ fi function check_file_exists { if [ ! -f ${NEW_PATH}/dump/kenny*/kenny*.${1} ]; then - exit_with_code "FAIL: no ${1} file in report" ${FAIL_CODE} + fail "no ${1} file in report" fi } @@ -41,7 +41,7 @@ wait_for_app crash-manager pushd ${NEW_PATH}/dump if ! unzip kenny*zip > /dev/null; then popd - exit_with_code "FAIL: report not found in ${NEW_PATH}/dump" ${FAIL_CODE} + fail "report not found in ${NEW_PATH}/dump" fi popd @@ -50,4 +50,4 @@ check_file_exists info check_file_exists so_info check_file_exists log -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/critical_process/critical_process.sh.template b/tests/system/critical_process/critical_process.sh.template index 63568b4..7b5fdbb 100755 --- a/tests/system/critical_process/critical_process.sh.template +++ b/tests/system/critical_process/critical_process.sh.template @@ -14,7 +14,7 @@ fi . ${CRASH_WORKER_SYSTEM_TESTS}/utils/minicore-utils.sh if is_emulator; then - exit_with_code "SKIP" ${SKIP_CODE} + skip "test not applicable on emulator" fi; clean_crash_dump @@ -68,7 +68,7 @@ while true; do sleep 4 gum-utils -d --username test1 1> /dev/null 2>&1 clean_after_test - exit_with_code "SUCCESS" ${SUCCESS_CODE} + exit_ok fi if endoftime; then @@ -83,4 +83,4 @@ sleep 4 gum-utils -d --username test1 1> /dev/null 2>&1 clean_after_test -exit_with_code "FAIL" ${FAIL_CODE} +fail "expected crash not found in timeout $TIMEOUT" diff --git a/tests/system/dbus_notify/dbus_notify.sh.template b/tests/system/dbus_notify/dbus_notify.sh.template index 9e51679..d6b0bb2 100644 --- a/tests/system/dbus_notify/dbus_notify.sh.template +++ b/tests/system/dbus_notify/dbus_notify.sh.template @@ -81,11 +81,11 @@ for i in $(seq 1 10); do fi if [ $score -eq 5 ]; then - exit_with_code "SUCCESS" ${SUCCESS_CODE} + exit_ok fi fi sleep 1 done -exit_with_code "FAIL: dbus signal does not match" ${FAIL_CODE} +fail "dbus signal does not match" diff --git a/tests/system/dump_systemstate_extras/dump_systemstate_extras.sh.template b/tests/system/dump_systemstate_extras/dump_systemstate_extras.sh.template index 5b799ec..41e270f 100644 --- a/tests/system/dump_systemstate_extras/dump_systemstate_extras.sh.template +++ b/tests/system/dump_systemstate_extras/dump_systemstate_extras.sh.template @@ -13,7 +13,7 @@ function do_check { body_pat="$4" if ! grep -EA "$section_len" "$section_pat" "$f" | grep -qE "$body_pat"; then - exit_with_code "FAIL: section ${section_pat} does not contain $body_pat" ${FAIL_CODE} + fail "section ${section_pat} does not contain $body_pat" fi } @@ -61,4 +61,4 @@ do_check $tmpfile "==== $cookie1" 1 "^magic_cookie is $cookie1" do_check $tmpfile "==== /bin/env test" 999 "^MAGIC_SECRET=$cookie2" do_check $tmpfile "==== file for cookie3" 1 "^$cookie3" -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/extra_script/extra_script.sh.template b/tests/system/extra_script/extra_script.sh.template index 1c8fe5e..40f743b 100644 --- a/tests/system/extra_script/extra_script.sh.template +++ b/tests/system/extra_script/extra_script.sh.template @@ -43,4 +43,4 @@ check_zip_contains "$dumpfile" '.*log$' check_zip_contains "$dumpfile" '.*info$' check_zip_contains "$dumpfile" '.*cookie$' -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/info_file/info_file.sh.template b/tests/system/info_file/info_file.sh.template index 052f4f9..12628e0 100644 --- a/tests/system/info_file/info_file.sh.template +++ b/tests/system/info_file/info_file.sh.template @@ -24,13 +24,13 @@ REPORT_ZIP=$(ls ${CRASH_DUMP_PATH}) REPORT_DIR=$(basename ${REPORT_ZIP} .zip) if [ ! -f ${CRASH_DUMP_PATH}/${REPORT_ZIP} ]; then - exit_with_code "FAIL: no report in ${CRASH_DUMP_PATH}" ${FAIL_CODE} + fail "no report in ${CRASH_DUMP_PATH}" fi pushd ${CRASH_DUMP_PATH} if ! unzip ${REPORT_ZIP} > /dev/nulll; then popd - exit_with_code "FAIL: report ${REPORT_ZIP} does not exist" ${FAIL_CODE} + fail "report ${REPORT_ZIP} does not exist" fi popd @@ -67,4 +67,4 @@ check ': .*sleep.*' check ': .*start.*kenny' check ': main.*kenny' -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/livedumper/livedumper.sh.template b/tests/system/livedumper/livedumper.sh.template index d22e073..5657bcd 100644 --- a/tests/system/livedumper/livedumper.sh.template +++ b/tests/system/livedumper/livedumper.sh.template @@ -9,7 +9,7 @@ fi . ${CRASH_WORKER_SYSTEM_TESTS}/utils/minicore-utils.sh if [ ! -x /usr/bin/livedumper ]; then - exit_with_code "SKIP" ${SKIP_CODE} + skip "livedumper binary not installed" fi mount -o rw,remount / @@ -32,12 +32,12 @@ wait_for_app crash-manager pushd ${LIVE_DUMP_PATH} if [ ! -f ${REPORT_PATH} ]; then - exit_with_code "Report path does not exist" ${FAIL_CODE} + fail "report path does not exist" fi if ! unzip ${REPORT_PATH} > /dev/null; then popd - exit_with_code "FAIL: report not found in ${LIVE_DUMP_PATH}" ${FAIL_CODE} + fail "report not found in ${LIVE_DUMP_PATH}" fi COREDUMP=`find -name "kenny*coredump*"` @@ -55,7 +55,7 @@ check "run.*id=.*kenny.cpp:56" if [[ $(pidof kenny) ]]; then kill -9 `pidof kenny` else - exit_with_code "FAIL" ${FAIL_CODE} + fail "victim process not found" fi -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/log_dump_crash_root_path/log_dump_crash_root_path.sh.template b/tests/system/log_dump_crash_root_path/log_dump_crash_root_path.sh.template index 543690b..aabc739 100644 --- a/tests/system/log_dump_crash_root_path/log_dump_crash_root_path.sh.template +++ b/tests/system/log_dump_crash_root_path/log_dump_crash_root_path.sh.template @@ -23,9 +23,9 @@ check_file_exists "$logfile" num=`unzip -qql "$logfile" | wc -l` if [ $num -ne 2 ]; then - exit_with_code "FAIL: 'log_dump --short' report contains $num files - 2 expected" ${FAIL_CODE} + fail "'log_dump --short' report contains $num files - 2 expected" fi restore_file ${CRASH_MANAGER_CONF} -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/log_dump_normal/log_dump_normal.sh.template b/tests/system/log_dump_normal/log_dump_normal.sh.template index 90cedd5..ef70cae 100644 --- a/tests/system/log_dump_normal/log_dump_normal.sh.template +++ b/tests/system/log_dump_normal/log_dump_normal.sh.template @@ -25,4 +25,4 @@ check_file_not_exists "$dummy" clean_logdump -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/log_dump_short/log_dump_short.sh.template b/tests/system/log_dump_short/log_dump_short.sh.template index e9e3990..38d7885 100644 --- a/tests/system/log_dump_short/log_dump_short.sh.template +++ b/tests/system/log_dump_short/log_dump_short.sh.template @@ -19,7 +19,7 @@ check_file_exists "$logfile" num=`unzip -qql "$logfile" | wc -l` if [ $num -ne 2 ]; then - exit_with_code "FAIL: 'log_dump --short' report contains $num files - 2 expected" ${FAIL_CODE} + fail "'log_dump --short' report contains $num files - 2 expected" fi check_zip_contains "$logfile" 'log/dump_systemstate.*log$' @@ -29,4 +29,4 @@ check_file_exists "$dummy" clean_logdump -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/log_file/log_file.sh.template b/tests/system/log_file/log_file.sh.template index d1cdfa1..d684c2d 100644 --- a/tests/system/log_file/log_file.sh.template +++ b/tests/system/log_file/log_file.sh.template @@ -12,7 +12,7 @@ function check_section_not_empty { if [ ${RES} -eq ${COUNT} ]; then return 0 fi - exit_with_code "FAIL: section ${1} does not contain enough data" ${FAIL_CODE} + fail "section ${1} does not contain enough data" } function check_section { @@ -38,13 +38,13 @@ REPORT_ZIP=$(ls ${CRASH_DUMP_PATH}) REPORT_DIR=$(basename ${REPORT_ZIP} .zip) if [ ! -f ${CRASH_DUMP_PATH}/${REPORT_ZIP} ]; then - exit_with_code "FAIL: no report in ${CRASH_DUMP_PATH}" ${FAIL_CODE} + fail "no report in ${CRASH_DUMP_PATH}" fi pushd ${CRASH_DUMP_PATH} if ! unzip ${REPORT_ZIP} > /dev/nulll; then popd - exit_with_code "FAIL: report ${REPORT_ZIP} does not exist" ${FAIL_CODE} + fail "report ${REPORT_ZIP} does not exist" fi popd @@ -70,4 +70,4 @@ check_section "==== Kernel messages" 10 check_section "==== Log messages" 10 check_section "==== Journal messages" 10 -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/report_basic/report_basic.sh.template b/tests/system/report_basic/report_basic.sh.template index 4243ccf..18d7836 100644 --- a/tests/system/report_basic/report_basic.sh.template +++ b/tests/system/report_basic/report_basic.sh.template @@ -24,18 +24,18 @@ pushd ${CRASH_DUMP_PATH} name=$(echo kenny_${pid}_*) name=${name%.zip} -test -f "${name}.zip" || exit_with_code "FAIL: crash report not found" ${FAIL_CODE} -unzip "${name}.zip" || exit_with_code "FAIL: unable to extract archive" ${FAIL_CODE} +test -f "${name}.zip" || fail "crash report not found" +unzip "${name}.zip" || fail "unable to extract archive" # assumes default configuration - with coredump -test -s "${name}/${name}.coredump.tar" || test -f ${name}/${name}.coredump || exit_with_code "FAIL: coredump corrupt or not found" ${FAIL_CODE} +test -s "${name}/${name}.coredump.tar" || test -f ${name}/${name}.coredump || fail "coredump corrupt or not found" -test -s "${name}/${name}.log" || exit_with_code "FAIL: log corrupt or not found" ${FAIL_CODE} -test -s "${name}/${name}.so_info" || exit_with_code "FAIL: info corrupt or not found" ${FAIL_CODE} -test -s "${name}/${name}.info" || exit_with_code "FAIL: info corrupt or not found" ${FAIL_CODE} +test -s "${name}/${name}.log" || fail "log corrupt or not found" +test -s "${name}/${name}.so_info" || fail "info corrupt or not found" +test -s "${name}/${name}.info" || fail "info corrupt or not found" for i in ${CRASH_TEMP_PATH}/*; do - test -a "${i}" && exit_with_code "FAIL: temp directory not cleaned up" ${FAIL_CODE} + test -a "${i}" && fail "temp directory not cleaned up" done -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/report_type_info/report_type_info.sh.template b/tests/system/report_type_info/report_type_info.sh.template index 515e32e..d8056f8 100644 --- a/tests/system/report_type_info/report_type_info.sh.template +++ b/tests/system/report_type_info/report_type_info.sh.template @@ -29,7 +29,7 @@ restore_file ${CRASH_MANAGER_CONF} wait_for_app crash-manager if ! compgen -G ${CRASH_DUMP_PATH}/kenny*info > /dev/null; then - exit_with_code "FAIL: info file not found" ${FAIL_CODE} + fail "info file not found" fi -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/so_info_file/so_info_file.sh.template b/tests/system/so_info_file/so_info_file.sh.template index b73d589..d8437ac 100644 --- a/tests/system/so_info_file/so_info_file.sh.template +++ b/tests/system/so_info_file/so_info_file.sh.template @@ -24,12 +24,12 @@ REPORT_ZIP=$(ls ${CRASH_DUMP_PATH}) REPORT_DIR=$(basename ${REPORT_ZIP} .zip) if [ ! -f ${CRASH_DUMP_PATH}/${REPORT_ZIP} ]; then - exit_with_code "FAIL: no report in ${CRASH_DUMP_PATH}" ${FAIL_CODE} + fail "no report in ${CRASH_DUMP_PATH}" fi pushd ${CRASH_DUMP_PATH} if ! unzip ${REPORT_ZIP} > /dev/nulll; then - exit_with_code "FAIL: report ${REPORT_ZIP} does not exist" ${FAIL_CODE} + fail "report ${REPORT_ZIP} does not exist" fi popd @@ -43,4 +43,4 @@ check "/usr/lib/libstdc[^ ]+ [a-z0-9]+ libstd[^;]+;[^;]+;[^;]+;[a-z0-9]+" check "/usr/lib/libpthread[^ ]+ [a-z0-9]+ glibc;[^;]+;[^;]+;[a-z0-9]+" check "/usr/lib/ld[^ ]+ [a-z0-9]+ glibc;[^;]+;[^;]+;[a-z0-9]+" -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok diff --git a/tests/system/time_test/time_test.sh.template b/tests/system/time_test/time_test.sh.template index 2366e5a..2edb71e 100755 --- a/tests/system/time_test/time_test.sh.template +++ b/tests/system/time_test/time_test.sh.template @@ -43,7 +43,7 @@ wait_for_file ${TEMP_DIR}/${MINICORE_TIME_FILE} MINICORE_TIME=$(< ${TEMP_DIR}/${MINICORE_TIME_FILE}) if [ ${MINICORE_TIME} -le ${TIME_LIMIT} ]; then - exit_with_code "SUCCESS" ${SUCCESS_CODE} + exit_ok else - exit_with_code "FAIL: dumping time: ${MINICORE_TIME} but limit was: ${TIME_LIMIT}" ${FAIL_CODE} + fail "dumping time: ${MINICORE_TIME} but limit was: ${TIME_LIMIT}" fi diff --git a/tests/system/utils/minicore-utils.sh b/tests/system/utils/minicore-utils.sh index 747809a..dd64a99 100644 --- a/tests/system/utils/minicore-utils.sh +++ b/tests/system/utils/minicore-utils.sh @@ -1,6 +1,9 @@ #!/bin/bash __CORE_PATTERN="" +SUCCESS_CODE=0 +FAIL_CODE=1 +SKIP_CODE=2 function save_core_pattern { __CORE_PATTERN=$(< /proc/sys/kernel/core_pattern) @@ -11,6 +14,18 @@ function exit_with_code { exit ${2} } +function exit_ok { + exit_with_code "SUCCESS" ${SUCCESS_CODE} +} + +function fail { + exit_with_code "FAIL: $1" ${FAIL_CODE} +} + +function skip { + exit_with_code "SKIP: $1" ${SKIP_CODE} +} + function restore_core_pattern { echo ${__CORE_PATTERN} > /proc/sys/kernel/core_pattern } @@ -27,18 +42,18 @@ function check { if [[ ${RESULT} =~ ${1} ]]; then return 0 fi - exit_with_code "FAIL: not found ${1} in ${RESULT}" ${FAIL_CODE} + fail "not found ${1} in ${RESULT}" } function check_file_exists { if [ ! -f ${1} ]; then - exit_with_code "FAIL: file not exists $1" ${FAIL_CODE} + fail "file not exists $1" fi } function check_file_not_exists { if [ -f ${1} ]; then - exit_with_code "FAIL: file exists $1" ${FAIL_CODE} + fail "file exists $1" fi } @@ -46,7 +61,7 @@ function check_zip_contains { zipfile=${1} pattern="$2" if ! unzip -qql "$zipfile" | grep -E "$pattern"; then - exit_with_code "FAIL: zip file ${zipfile} does not contain ${pattern}" ${FAIL_CODE} + fail "zip file ${zipfile} does not contain ${pattern}" fi } @@ -67,7 +82,7 @@ function clean_crash_dump { sleep 1 if [ ! -d ${CRASH_DUMP_PATH} ]; then - exit_with_code "${CRASH_DUMP_PATH} does not exist" ${FAIL_CODE} + fail "${CRASH_DUMP_PATH} does not exist" fi rm -rf ${CRASH_DUMP_PATH}/* @@ -95,7 +110,7 @@ function wait_for_file { while [ ! -f ${1} ]; do if endoftime; then - exit_with_code "FAIL: ${1} does not exist" ${FAIL_CODE} + fail "${1} does not exist" else sleep 1 fi @@ -111,7 +126,7 @@ function wait_for_app { break; fi if endoftime; then - exit_with_code "FAIL: ${1} is still running" ${FAIL_CODE} + fail "${1} is still running" else sleep 1 fi @@ -138,27 +153,24 @@ function __export_vars__ { export CRASH_TEMP_PATH=`tzplatform_var TZ_SYS_CRASH_ROOT`/temp if [ "x${CRASH_TEMP_PATH}" = 'x/temp' ]; then - exit_with_code "Couldn't get TZ_SYS_CRASH_ROOT" ${FAIL_CODE} + fail "Couldn't get TZ_SYS_CRASH_ROOT" fi export LOGDUMP_RESULT_PATH=`tzplatform_var TZ_SYS_CRASH_ROOT`/debug if [ "x${LOGDUMP_RESULT_PATH}" = 'x/debug' ]; then - exit_with_code "Couldn't get TZ_SYS_CRASH_ROOT" ${FAIL_CODE} + fail "Couldn't get TZ_SYS_CRASH_ROOT" fi export CRASH_DUMP_PATH=`tzplatform_var TZ_SYS_CRASH` if [ -z ${CRASH_DUMP_PATH} ]; then - exit_with_code "Couldn't get TZ_SYS_CRASH or TZ_SYS_CRASH is empty" ${FAIL_CODE} + fail "Couldn't get TZ_SYS_CRASH or TZ_SYS_CRASH is empty" fi export LIVE_DUMP_PATH=`tzplatform_var TZ_SYS_CRASH_ROOT`/livedump if [ -z ${LIVE_DUMP_PATH} ]; then - exit_with_code "Couldn't get TZ_SYS_CRASH_ROOT or TZ_SYS_CRASH_ROOT is empty" ${FAIL_CODE} + fail "Couldn't get TZ_SYS_CRASH_ROOT or TZ_SYS_CRASH_ROOT is empty" fi - export SUCCESS_CODE=0 - export FAIL_CODE=1 - export SKIP_CODE=2 } function is_emulator { diff --git a/tests/system/wait_for_opt_usr/wait_for_opt_usr.sh.template b/tests/system/wait_for_opt_usr/wait_for_opt_usr.sh.template index caefaee..3120bb4 100755 --- a/tests/system/wait_for_opt_usr/wait_for_opt_usr.sh.template +++ b/tests/system/wait_for_opt_usr/wait_for_opt_usr.sh.template @@ -16,7 +16,7 @@ MTAB=`cat /etc/mtab | grep " ${OPT_MOUNTPOINT}"` clean_crash_dump if ! umount -l ${OPT_MOUNTPOINT}; then - exit_with_code "umount ${OPT_MOUNTPOINT} error" ${FAIL_CODE} + fail "umount ${OPT_MOUNTPOINT} error" fi save_core_pattern @@ -39,7 +39,7 @@ echo "${MTAB}" | while read OPT_LINE; do OPT_CUR_MOUNTPOINT=`echo ${OPT_LINE} | cut -d' ' -f2` if ! mount -t ${OPT_FS} -o ${OPT_OPTS} ${OPT_DEV} ${OPT_CUR_MOUNTPOINT}; then - exit_with_code "ERROR: mount -t ${OPT_FS} -o ${OPT_OPTS} ${OPT_DEV} ${OPT_CUR_MOUNTPOINT}" ${FAIL_CODE} + fail " mount -t ${OPT_FS} -o ${OPT_OPTS} ${OPT_DEV} ${OPT_CUR_MOUNTPOINT}" fi done diff --git a/tests/system/without_core/without_core.sh.template b/tests/system/without_core/without_core.sh.template index cfdee7a..ca13adf 100644 --- a/tests/system/without_core/without_core.sh.template +++ b/tests/system/without_core/without_core.sh.template @@ -31,12 +31,12 @@ wait_for_app crash-manager pushd ${CRASH_DUMP_PATH} if ! unzip kenny*zip > /dev/null; then popd - exit_with_code "FAIL: report does not exist" ${FAIL_CODE} + fail "report does not exist" fi popd if ls ${CRASH_DUMP_PATH}/kenny*/kenny*.coredump.tar > /dev/null; then - exit_with_code "FAIL: coredump file exists" ${FAIL_CODE} + fail "coredump file exists" fi -exit_with_code "SUCCESS" ${SUCCESS_CODE} +exit_ok -- 2.7.4 From 840b0c3d7b4cbc25cb962fc90de0b2f81ede150d Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 1 Aug 2019 16:38:12 +0200 Subject: [PATCH 09/16] Lower the number (and priority) of crash-worker sysctl config file Install sysctl file with 70 prefix instead of 99. This will allow overriding variables by creating eg. 99-local.conf file with appropriate vars. Change-Id: I88e25839a59a66896ab540d0836776b2d64225e5 --- packaging/crash-worker.spec | 2 +- .../{99-crash-manager.conf.in => 70-crash-manager.conf.in} | 0 src/crash-manager/CMakeLists.txt | 4 ++-- 3 files changed, 3 insertions(+), 3 deletions(-) rename src/crash-manager/{99-crash-manager.conf.in => 70-crash-manager.conf.in} (100%) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index 51abe84..dd328cb 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -199,7 +199,7 @@ sed -i "/${pattern}/D" %{_sysconfdir}/ld.so.preload %dir %{crash_path} %dir %{crash_temp} %{_sysconfdir}/crash-manager.conf -%attr(-,root,root) %{_prefix}/lib/sysctl.d/99-crash-manager.conf +%attr(-,root,root) %{_prefix}/lib/sysctl.d/70-crash-manager.conf %attr(0750,system_fw,system_fw) %{_bindir}/crash-manager %attr(0750,system_fw,system_fw) %{_bindir}/dump_systemstate %{_sysconfdir}/dump_systemstate.conf.d/files/files.conf.example diff --git a/src/crash-manager/99-crash-manager.conf.in b/src/crash-manager/70-crash-manager.conf.in similarity index 100% rename from src/crash-manager/99-crash-manager.conf.in rename to src/crash-manager/70-crash-manager.conf.in diff --git a/src/crash-manager/CMakeLists.txt b/src/crash-manager/CMakeLists.txt index 198a92c..ce803d8 100644 --- a/src/crash-manager/CMakeLists.txt +++ b/src/crash-manager/CMakeLists.txt @@ -48,13 +48,13 @@ INSTALL(TARGETS ${PROJECT_NAME} DESTINATION bin GROUP_READ GROUP_EXECUTE WORLD_READ WORLD_EXECUTE) CONFIGURE_FILE(500.${PROJECT_NAME}-upgrade.sh.in 500.${PROJECT_NAME}-upgrade.sh @ONLY) -CONFIGURE_FILE(99-${PROJECT_NAME}.conf.in 99-${PROJECT_NAME}.conf @ONLY) +CONFIGURE_FILE(70-${PROJECT_NAME}.conf.in 70-${PROJECT_NAME}.conf @ONLY) INSTALL(FILES ${CMAKE_SOURCE_DIR}/src/${PROJECT_NAME}/crash-manager.conf DESTINATION /etc PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ) -INSTALL(FILES ${CMAKE_SOURCE_DIR}/src/${PROJECT_NAME}/99-${PROJECT_NAME}.conf +INSTALL(FILES ${CMAKE_SOURCE_DIR}/src/${PROJECT_NAME}/70-${PROJECT_NAME}.conf DESTINATION ${CMAKE_INSTALL_PREFIX}/lib/sysctl.d PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ) -- 2.7.4 From 50f1840935232602237149f774d2d0affd2d7762 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 8 Aug 2019 10:13:04 +0200 Subject: [PATCH 10/16] crash-manager: fix potential cinfo->cmdline double free Change-Id: I1ea39c92f076834defe4dfc0e7f4a6d0f798f066 --- src/crash-manager/crash-manager.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index fa24663..fedd35f 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -273,6 +273,7 @@ static bool get_cmd_info(struct crash_info *cinfo) err: free(cinfo->cmd_line); + cinfo->cmd_line = NULL; return false; } -- 2.7.4 From 7b94ee79e3896251f74f77da7a5a516fd536d53d Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 8 Aug 2019 10:49:27 +0200 Subject: [PATCH 11/16] shared: Simplify get_exe_path Change-Id: I26839430a4836454f96c1f273a1500899dc6b607 --- src/crash-manager/crash-manager.c | 8 +++++--- src/crash-stack/crash-stack.c | 11 +++-------- src/shared/util.c | 31 ++++++++++++------------------- src/shared/util.h | 2 +- 4 files changed, 21 insertions(+), 31 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index fedd35f..b01253c 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -265,11 +265,13 @@ static bool get_cmd_info(struct crash_info *cinfo) if (!read_proc_file(cinfo->pid_info, "cmdline", cinfo->cmd_line, PATH_MAX, NULL)) goto err; - cinfo->cmd_path = get_exe_path(cinfo->pid_info); - if (!cinfo->cmd_path) + char buf[PATH_MAX]; + if (!get_exe_path(cinfo->pid_info, buf, sizeof(buf))) goto err; - return true; + cinfo->cmd_path = strdup(buf); + + return !!cinfo->cmd_path; err: free(cinfo->cmd_line); diff --git a/src/crash-stack/crash-stack.c b/src/crash-stack/crash-stack.c index c0f9604..0445d9c 100644 --- a/src/crash-stack/crash-stack.c +++ b/src/crash-stack/crash-stack.c @@ -262,15 +262,10 @@ void callstack_destructor(Callstack *callstack) */ static void __crash_stack_print_exe(FILE* outputfile, pid_t pid) { - char *file_path; + char path[PATH_MAX]; + bool has_path = get_exe_path(pid, path, sizeof(path)); - file_path = get_exe_path(pid); - if (file_path == NULL) - file_path = strdup(""); - - fprintf(outputfile, "Executable File Path: %s\n", file_path); - - free(file_path); + fprintf(outputfile, "Executable File Path: %s\n", has_path ? path : ""); } /** diff --git a/src/shared/util.c b/src/shared/util.c index 84fa25e..1860bab 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -519,34 +519,27 @@ err_close: return false; } -char* get_exe_path(pid_t pid) +bool get_exe_path(pid_t pid, char *outbuf, size_t outbuf_len) { - char exe_link[PATH_MAX]; - char buffer[PATH_MAX]; + assert(outbuf); + assert(outbuf_len > 0); - snprintf(exe_link, sizeof(exe_link), - "/proc/%d/exe", pid); + char exe_link[PATH_MAX]; - ssize_t ret = readlink(exe_link, buffer, sizeof(buffer) - 1); + snprintf(exe_link, sizeof(exe_link), "/proc/%d/exe", pid); + ssize_t ret = readlink(exe_link, outbuf, outbuf_len - 1); if (ret <= 0) { _E("Failed to read link %s: %m", exe_link); - return NULL; - } - buffer[ret] = '\0'; - - if (access(buffer, F_OK) == -1) { - _E("Invalid path %s", buffer); - return NULL; + return false; } + outbuf[ret] = '\0'; - char *result; - if (asprintf(&result, "%s", buffer) == -1) { - _E("asprintf() error: %m\n"); - return NULL; + if (access(outbuf, F_OK) == -1) { + _E("Unable to access %s: %m", outbuf); + return false; } - - return result; + return true; } /* This function is supposed to accept same data as passed to execve diff --git a/src/shared/util.h b/src/shared/util.h index 0f81ca7..f3177e8 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -58,7 +58,7 @@ int log_kmsg(char *fmt, ...); int find_crash_tid(int pid); -char* get_exe_path(pid_t pid); +bool get_exe_path(pid_t pid, char *outbuf, size_t outbuf_len); typedef bool (*charp0filter)(char *buf, int size, int datalen); bool filter_drop_trailing_whitespace(char *buf, int size, int datalen); -- 2.7.4 From 6e7cfdcd678faae94b8c7b86963ad0a9b22bc0c7 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Fri, 9 Aug 2019 13:36:49 +0200 Subject: [PATCH 12/16] Release 5.5.19 This release brings: - lowering priority of crash-worker sysctl file to allow easy overwriting of sysctls by local files - few fixes for reliability - minor system tests refactoring Change-Id: If12efeb3c59c68ed642c32d83e789592520a18af --- packaging/crash-worker.spec | 2 +- packaging/crash-worker_system-tests.spec | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index dd328cb..ecbd3de 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -14,7 +14,7 @@ Name: crash-worker Summary: Crash-manager -Version: 5.5.18 +Version: 5.5.19 Release: 1 Group: Framework/system License: Apache-2.0 and BSD diff --git a/packaging/crash-worker_system-tests.spec b/packaging/crash-worker_system-tests.spec index 666fd44..dfa12f3 100644 --- a/packaging/crash-worker_system-tests.spec +++ b/packaging/crash-worker_system-tests.spec @@ -8,7 +8,7 @@ Name: crash-worker_system-tests Summary: Package with binaries and scripts for crash-worker system tests -Version: 5.5.18 +Version: 5.5.19 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From 20ccd602619974ddd09b449beb5076e510b31949 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 21 Aug 2019 15:45:32 +0200 Subject: [PATCH 13/16] dump_scripts: move_dump.sh: set PATH exactly as security team requires Change-Id: I8750d45b43ebb090e9cf31b8c1814eb1550fdce7 --- dump_scripts/move_dump.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dump_scripts/move_dump.sh b/dump_scripts/move_dump.sh index 50f7198..94d09c4 100755 --- a/dump_scripts/move_dump.sh +++ b/dump_scripts/move_dump.sh @@ -1,6 +1,6 @@ #!/bin/sh -PATH=/bin:/usr/sbin +PATH=/bin:/usr/bin:/sbin:/usr/sbin . /etc/tizen-platform.conf -- 2.7.4 From 05912c4a395168eedf1c46975e277220f1b59978 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 21 Aug 2019 15:29:31 +0200 Subject: [PATCH 14/16] crash-manager: Rewrite make_dump_path() for readability Change-Id: I523ca95903f16118e87ca724cc035b82c879d459 --- src/crash-manager/crash-manager.c | 49 ++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index b01253c..d746ecb 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -223,35 +223,36 @@ static int prepare_paths(struct crash_info* cinfo) return 1; } -static int make_dump_dir(void) +static bool make_dump_dir(void) { - struct stat st; + const char *dirs[] = {crash_crash_path, crash_temp_path}; - if (!stat(crash_crash_path, &st)) { - if (!(st.st_mode & S_IFDIR)) { - _E("%s (not DIR) is already exist", crash_crash_path); - return -1; - } - } else { - if (mkdir(crash_crash_path, 0775) < 0) { - _E("Failed to mkdir for %s", crash_crash_path); - return -1; - } - } + for (size_t i = 0; i < ARRAY_SIZE(dirs); i++) { + const char *dirname = dirs[i]; - if (!stat(crash_temp_path, &st)) { - if (!(st.st_mode & S_IFDIR)) { - _E("%s (not DIR) is already exist", crash_temp_path); - return -1; - } - } else { - if (mkdir(crash_temp_path, 0775) < 0) { - _E("Failed to mkdir for %s", crash_temp_path); - return -1; + int r = mkdir(dirname, 0775); + if (r >= 0) + continue; + + if (errno != EEXIST) { + _E("Unable to create directory %s: %m", dirname); + return false; } + + struct stat st = {0}; + r = stat(dirname, &st); + bool isdir = !!(st.st_mode & S_IFDIR); + + if (!r && isdir) + continue; + else if (!r && !isdir) + errno = ENOTDIR; + + _E("Failure while trying to ensure %s exists and it is directory: %m", dirname); + return false; } - return 0; + return true; } static bool get_cmd_info(struct crash_info *cinfo) @@ -1275,7 +1276,7 @@ int main(int argc, char *argv[]) } /* Create crash directories */ - if (make_dump_dir() < 0) { + if (!make_dump_dir()) { res = EXIT_FAILURE; goto exit; } -- 2.7.4 From be51f6c2054edac30b31b80fa094614e018846dc Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Mon, 12 Aug 2019 13:54:37 +0200 Subject: [PATCH 15/16] crash-manager: Janitorial: rename crash_crash_path to crash_dump_path for readability Change-Id: Id62ade3ac22c27360bad000b37e3e9bc53e5b254 --- src/crash-manager/crash-manager.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index d746ecb..34e4c3f 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -83,7 +83,7 @@ struct file_info { /* Configuration variables */ config_t config; -static char* crash_crash_path; +static char* crash_dump_path; static char* crash_temp_path; /* Paths and variables */ @@ -206,12 +206,12 @@ static int prepare_paths(struct crash_info* cinfo) const char *crash_subdir = cinfo->livedump ? LIVE_PATH_SUBDIR : CRASH_PATH_SUBDIR; tmp_len = strlen(config.crash_root_path) + strlen(crash_subdir); - crash_crash_path = (char*)malloc(tmp_len + 1); - if (crash_crash_path == NULL) { - _E("Couldn't allocate memory for crash_crash_path: %m\n"); + crash_dump_path = (char*)malloc(tmp_len + 1); + if (crash_dump_path == NULL) { + _E("Couldn't allocate memory for crash_dump_path: %m\n"); return 0; } - snprintf(crash_crash_path, tmp_len + 1, "%s%s", config.crash_root_path, crash_subdir); + snprintf(crash_dump_path, tmp_len + 1, "%s%s", config.crash_root_path, crash_subdir); tmp_len = strlen(config.crash_root_path) + strlen(CRASH_TEMP_SUBDIR); crash_temp_path = (char*)malloc(tmp_len + 1); @@ -225,7 +225,7 @@ static int prepare_paths(struct crash_info* cinfo) static bool make_dump_dir(void) { - const char *dirs[] = {crash_crash_path, crash_temp_path}; + const char *dirs[] = {crash_dump_path, crash_temp_path}; for (size_t i = 0; i < ARRAY_SIZE(dirs); i++) { const char *dirname = dirs[i]; @@ -505,10 +505,10 @@ static int set_crash_info(struct crash_info *cinfo) if (config.allow_zip) ret = asprintf(&cinfo->result_path, - "%s/%s.zip", crash_crash_path, cinfo->name); + "%s/%s.zip", crash_dump_path, cinfo->name); else ret = asprintf(&cinfo->result_path, - "%s/%s", crash_crash_path, cinfo->name); + "%s/%s", crash_dump_path, cinfo->name); if (ret == -1) { _E("Failed to asprintf for result path"); cinfo->result_path = NULL; @@ -917,13 +917,13 @@ static int lock_dumpdir(void) { int fd; - if ((fd = open(crash_crash_path, O_RDONLY | O_DIRECTORY)) < 0) { - _E("Failed to open %s", crash_crash_path); + if ((fd = open(crash_dump_path, O_RDONLY | O_DIRECTORY)) < 0) { + _E("Failed to open %s: %m", crash_dump_path); return -1; } if (flock(fd, LOCK_EX) < 0) { - _E("Failed to flock (LOCK)"); + _E("Failed to lock %s for exclusive access: %m", crash_dump_path); close(fd); return -1; } @@ -965,12 +965,12 @@ static int scan_dump(struct file_info **dump_list, off_t *usage) int i, scan_num, dump_num = 0; int fd; - if ((fd = open(crash_crash_path, O_DIRECTORY)) < 0) { - _E("Failed to open %s", crash_crash_path); + if ((fd = open(crash_dump_path, O_DIRECTORY)) < 0) { + _E("Failed to open %s: %m", crash_dump_path); return -1; } - scan_num = scandir(crash_crash_path, &scan_list, &dump_filter, NULL); + scan_num = scandir(crash_dump_path, &scan_list, &dump_filter, NULL); if (scan_num < 0) { close(fd); return -1; @@ -990,7 +990,7 @@ static int scan_dump(struct file_info **dump_list, off_t *usage) } if (asprintf(&(temp_list[dump_num].path), "%s/%s", - crash_crash_path, scan_list[i]->d_name) < 0) { + crash_dump_path, scan_list[i]->d_name) < 0) { _E("Failed to asprintf"); continue; } @@ -1328,7 +1328,7 @@ int main(int argc, char *argv[]) } else { free(cinfo.result_path); if (asprintf(&cinfo.result_path, "%s/%s.info", - crash_crash_path, cinfo.name) == -1) { + crash_dump_path, cinfo.name) == -1) { cinfo.result_path = NULL; _E("asprintf() error: %m"); res = EXIT_FAILURE; @@ -1368,7 +1368,7 @@ exit: if (cinfo.prstatus_fd >= 0) close(cinfo.prstatus_fd); free(crash_temp_path); - free(crash_crash_path); + free(crash_dump_path); config_free(&config); free_crash_info(&cinfo); -- 2.7.4 From 03cf13cbe094f874d5f273743e6af21f193f0a4e Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 21 Aug 2019 15:29:01 +0200 Subject: [PATCH 16/16] crash-manager: print textual error code where possible Change-Id: Ia2536417d55d9b8275424983ce24ec6cdadc7d5a --- src/crash-manager/crash-manager.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 34e4c3f..1d3eb69 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -577,7 +577,7 @@ static int get_sysassert_cs(struct crash_info *cinfo) char move_path[PATH_MAX]; if (access(cinfo->sysassert_cs_path, F_OK)) { - _E("The sys-assert cs file not found: %s", + _E("The sys-assert cs file not found: %s: %m", cinfo->sysassert_cs_path); cinfo->have_sysassert_report = 0; return -1; @@ -934,7 +934,7 @@ static int lock_dumpdir(void) static void unlock_dumpdir(int fd) { if (flock(fd, LOCK_UN) < 0) - _E("Failed to unlink (UNLOCK)"); + _E("Failed to unlock file descriptor: %m"); close(fd); } -- 2.7.4