From bc1937017cad065cb53fbd5a34d5e89204535214 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Tue, 16 Jul 2019 08:16:04 +0200 Subject: [PATCH 01/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 02/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 03/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 04/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 05/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 06/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 07/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 08/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 09/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 10/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 11/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 12/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 From b3c814c8341722ddb274b18e45bac75ccdc0e2c5 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Thu, 25 Jul 2019 11:00:26 +0200 Subject: [PATCH 13/16] Move code to a separate function Change-Id: I81bd9ac6467214ce9251213dee193fd6f53c71c2 --- src/crash-manager/crash-manager.c | 167 ++++++++++++++++++++------------------ 1 file changed, 86 insertions(+), 81 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 1d3eb69..5f70610 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -441,7 +441,7 @@ static bool parse_args(struct crash_info *cinfo, int argc, char *argv[]) #undef GET_NUMBER } -static int set_crash_info(struct crash_info *cinfo) +static bool set_crash_info(struct crash_info *cinfo) { int ret; char *temp_dir_ret = NULL; @@ -458,7 +458,7 @@ static int set_crash_info(struct crash_info *cinfo) if (cinfo->livedump && !file_exists(LIVEDUMPER_BIN_PATH)) { fprintf(stderr, "Error: %s doesn't exist - can not perform livedump. Terminating.\n", LIVEDUMPER_BIN_PATH); _E("Error: %s doesn't exist - can not perform livedump. Terminating.\n", LIVEDUMPER_BIN_PATH); - return -1; + return false; } if (cinfo->tid_info == -1) { @@ -475,7 +475,7 @@ static int set_crash_info(struct crash_info *cinfo) if (!get_cmd_info(cinfo)) { _E("Failed to get command info"); - return -1; + return false; } if (cinfo->time_info == 0) @@ -487,13 +487,13 @@ static int set_crash_info(struct crash_info *cinfo) if (asprintf(&cinfo->temp_dir, "%s/crash.XXXXXX", crash_temp_path) == -1) { _E("Failed to asprintf for temp_dir"); cinfo->temp_dir = NULL; - return -1; + return false; } 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; + return false; } if (asprintf(&cinfo->name, "%s_%d_%s", basename(cinfo->cmd_line), @@ -563,11 +563,11 @@ static int set_crash_info(struct crash_info *cinfo) snprintf(cinfo->pkgid, sizeof(cinfo->pkgid), "%s", cinfo->appid); } - return 0; + return true; rm_temp: remove_dir(cinfo->temp_dir, 1); - return -1; + return false; } #ifdef SYS_ASSERT @@ -1238,82 +1238,37 @@ static void crash_info_init(struct crash_info *cinfo) cinfo->time_info = 0; } -int main(int argc, char *argv[]) +static bool run(struct crash_info *cinfo) { - 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 - * value that prevents from running crash-manager recursively. - */ - - if (!config_init(&config, CRASH_MANAGER_CONFIG_PATH)) { - res = EXIT_FAILURE; - goto exit; - } - - if (!parse_args(&cinfo, argc, argv)) { - res = EXIT_FAILURE; - goto exit; - } - - if (!prepare_paths(&cinfo)) { - res = EXIT_FAILURE; - goto exit; - } - - if (!wait_for_opt(WAIT_FOR_OPT_TIMEOUT_SEC)) { - res = EXIT_FAILURE; - goto exit; - } - - /* Create crash directories */ - if (!make_dump_dir()) { - res = EXIT_FAILURE; - goto exit; - } - - /* Set crash info */ - if (set_crash_info(&cinfo) < 0) { - res = EXIT_FAILURE; - goto exit; - } #ifdef SYS_ASSERT /* Fetch callstack of sys-assert */ - get_sysassert_cs(&cinfo); + get_sysassert_cs(cinfo); #endif if (config.report_type >= REP_TYPE_FULL) { /* Exec dump_systemstate */ - if (!dump_system_state(&cinfo, &dump_state_pid)) { - res = EXIT_FAILURE; + if (!dump_system_state(cinfo, &dump_state_pid)) { _E("Failed to get system state report"); - goto exit; + return false; } - if (config.extra_script && !extra_script(&cinfo, &extra_script_pid)) + if (config.extra_script && !extra_script(cinfo, &extra_script_pid)) _W("Failed to call extra script from config"); } /* Exec crash modules */ - if (!execute_crash_modules(&cinfo)) { - res = EXIT_FAILURE; + if (!execute_crash_modules(cinfo)) { _E("Failed to get basic crash information"); - goto exit; + return false; } if (config.report_type >= REP_TYPE_FULL) { /* Save shared objects info (file names, bulid IDs, rpm package names) */ - save_so_info(&cinfo); + save_so_info(cinfo); /* Wait misc. pids */ wait_for_pid(dump_state_pid, NULL); @@ -1322,25 +1277,24 @@ int main(int argc, char *argv[]) /* Tar compression */ if (config.allow_zip) - compress(&cinfo); + compress(cinfo); else - move_dump_data(cinfo.pfx, &cinfo); + move_dump_data(cinfo->pfx, cinfo); } else { - free(cinfo.result_path); - if (asprintf(&cinfo.result_path, "%s/%s.info", - crash_dump_path, cinfo.name) == -1) { - cinfo.result_path = NULL; + free(cinfo->result_path); + if (asprintf(&cinfo->result_path, "%s/%s.info", + crash_dump_path, cinfo->name) == -1) { + cinfo->result_path = NULL; _E("asprintf() error: %m"); - res = EXIT_FAILURE; - goto exit; + return false; } - move_dump_data(cinfo.info_path, &cinfo); + move_dump_data(cinfo->info_path, cinfo); } - if (cinfo.print_result_path) - printf("REPORT_PATH=%s\n", cinfo.result_path); + if (cinfo->print_result_path) + printf("REPORT_PATH=%s\n", cinfo->result_path); - if (!cinfo.livedump) { + if (!cinfo->livedump) { /* Release the core pipe as passed by kernel, allowing another * coredump to be handled. * @@ -1354,24 +1308,75 @@ int main(int argc, char *argv[]) */ close(STDIN_FILENO); - launch_dbus_notify(&cinfo); + launch_dbus_notify(cinfo); /* launch crash-popup only if the .debugmode file exists */ if (debug_mode) - launch_crash_popup(&cinfo); - } else if (cinfo.kill) { - kill_pid(cinfo.pid_info, SIGKILL, false); + launch_crash_popup(cinfo); + } else if (cinfo->kill) { + kill_pid(cinfo->pid_info, SIGKILL, false); } -exit: - _I("Exiting with exit code %d", res); - if (cinfo.prstatus_fd >= 0) - close(cinfo.prstatus_fd); + return true; +} + +bool crash_manager_prepare(struct crash_info *cinfo) { + if (!config_init(&config, CRASH_MANAGER_CONFIG_PATH)) + return false; + + if (!prepare_paths(cinfo)) + return false; + + if (!wait_for_opt(WAIT_FOR_OPT_TIMEOUT_SEC)) + return false; + + /* Create crash directories */ + if (!make_dump_dir()) + return false; + + if (!set_crash_info(cinfo)) + return false; + + return true; +} + +static void crash_manager_free(struct crash_info *cinfo) +{ + if (cinfo->prstatus_fd >= 0) + close(cinfo->prstatus_fd); free(crash_temp_path); free(crash_dump_path); config_free(&config); - free_crash_info(&cinfo); + free_crash_info(cinfo); +} + +int main(int argc, char *argv[]) +{ + int res = EXIT_SUCCESS; + struct crash_info 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 + * value that prevents from running crash-manager recursively. + */ + + crash_info_init(&cinfo); + + /* Parse args */ + if (!parse_args(&cinfo, argc, argv)) + return EXIT_FAILURE; + + if (!crash_manager_prepare(&cinfo)) { + res = EXIT_FAILURE; + goto exit; + } + if (!run(&cinfo)) + res = EXIT_FAILURE; +exit: + crash_manager_free(&cinfo); + _I("Exiting with exit code %d", res); return res; } -- 2.7.4 From 8aa6d1dcd52b4aa8b6629cc08071a22e8ae413f5 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 29 Aug 2019 22:56:42 +0200 Subject: [PATCH 14/16] config: Add missing free() for extra_script Change-Id: Ie17d4dd90e08f02192139acbd8edf5c5682a4f59 --- src/shared/config.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/shared/config.c b/src/shared/config.c index 2bd7c7c..f8451d9 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -98,4 +98,5 @@ void config_free(config_t *c) assert(c); free(c->crash_root_path); + free(c->extra_script); } -- 2.7.4 From c0a138fc2af42a4f109a8f79d51712f4dc9b3255 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Fri, 30 Aug 2019 08:38:50 +0200 Subject: [PATCH 15/16] config: Fix invalid default setting for MaxRetentionSec Change-Id: I809d86fdd0242c2c280b4db4eef009cadfd4652b --- src/shared/config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/config.c b/src/shared/config.c index f8451d9..3154752 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -80,7 +80,7 @@ bool config_init(config_t *c, const char *const path) c->system_max_use = GET(int, "SystemMaxUse", SYSTEM_MAX_USE); c->system_keep_free = GET(int, "SystemKeepFree", SYSTEM_KEEP_FREE); - c->max_retention_sec = GET(int, "MaxRetentionSec", SYSTEM_MAX_USE); + c->max_retention_sec = GET(int, "MaxRetentionSec", MAX_RETENTION_SEC); c->max_crash_dump = GET(int, "MaxCrashDump", MAX_CRASH_DUMP); c->dump_core = GET(boolean, "DumpCore", DUMP_CORE); c->allow_zip = GET(boolean, "AllowZip", ALLOW_ZIP); -- 2.7.4 From f1c55a20c854b26bf3b8f0f4854c3190d12c3098 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 29 Aug 2019 22:57:02 +0200 Subject: [PATCH 16/16] config: Use same type to store all bools Change-Id: Ife768dd5b71355759db7d678d7c3fd6a5399e811 --- src/shared/config.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/config.h b/src/shared/config.h index 5c3ea1f..aea42ea 100644 --- a/src/shared/config.h +++ b/src/shared/config.h @@ -39,11 +39,11 @@ enum ReportType { typedef struct config { bool allow_zip; + bool dump_core; int system_max_use; int system_keep_free; int max_retention_sec; int max_crash_dump; - int dump_core; enum ReportType report_type; char *crash_root_path; char *extra_script; -- 2.7.4