From bb69705b4327661a6d9747bc5776c72bf6c79e15 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Tue, 21 Jan 2020 12:25:41 +0100 Subject: [PATCH 01/16] Add system test for legacy signal Change-Id: If4e2d007ebac2de6a11d64137c9d93e5efc75b6d --- tests/system/CMakeLists.txt | 1 + .../dbus_notify_legacy.sh.template | 80 ++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 tests/system/dbus_notify_legacy/dbus_notify_legacy.sh.template diff --git a/tests/system/CMakeLists.txt b/tests/system/CMakeLists.txt index b5423c1..44a1b1c 100644 --- a/tests/system/CMakeLists.txt +++ b/tests/system/CMakeLists.txt @@ -32,6 +32,7 @@ configure_test("dump_systemstate_extras") configure_test("livedumper") configure_test("extra_script") configure_test("dbus_notify") +configure_test("dbus_notify_legacy") configure_test("output_param") configure_test("libcrash-service") configure_test("full_core") diff --git a/tests/system/dbus_notify_legacy/dbus_notify_legacy.sh.template b/tests/system/dbus_notify_legacy/dbus_notify_legacy.sh.template new file mode 100644 index 0000000..946e9c3 --- /dev/null +++ b/tests/system/dbus_notify_legacy/dbus_notify_legacy.sh.template @@ -0,0 +1,80 @@ +#!/bin/bash + +# Check the support for old-style DBus notification signal + +if [ -z "${CRASH_WORKER_SYSTEM_TESTS}" ]; then + CRASH_WORKER_SYSTEM_TESTS="@CRASH_SYSTEM_TESTS_PATH@" +fi + +. ${CRASH_WORKER_SYSTEM_TESTS}/utils/minicore-utils.sh + +# We are looking for following signal - example for `sleep` command: +# +# signal time=1420079847.594970 sender=:1.82 -> destination=(null destination) serial=2 path=/Org/Tizen/System/Crash/Crash; interface=org.tizen.system.crash.Crash; member=ProcessCrashed +# string "sleep" +# string "/usr/bin/sleep" +# string "sleep" +# string "sleep" +# +# Note! Regular programs (Unix sleep, tests-provided kenny) are not enough to check all fields of the signal. +# Complete test, with appid & pkgid checks, would require building proper "Tizen Application". + +CONF_DIR=/etc/crash-manager.conf.d +CONF_FILE=${CONF_DIR}/system-test-dbus-notify-legacy.conf + +mount -o rw,remount / + +mkdir -p $CONF_DIR +rm -f $CONF_FILE || : +cat <$CONF_FILE +[CrashManager] +UseLegacyNotification=true +EOF + +( ${CRASH_WORKER_SYSTEM_TESTS}/utils/kenny & + pid=$! + sleep 2 + kill -6 $pid ) & + +TMPFILE=$(mktemp /tmp/dbus_notify.XXXXXX) +dbus-monitor --system type=signal,path=/Org/Tizen/System/Crash/Crash,interface=org.tizen.system.crash.Crash,member=ProcessCrashed > $TMPFILE & +monpid=$! + +cleanup() +{ + kill $monpid + rm -f $TMPFILE + rm -f $CONF_FILE +} + +trap cleanup 0 + +sleep 3 + +PATTERN='path=/Org/Tizen/System/Crash/Crash; interface=org\.tizen\.system\.crash\.Crash; member=ProcessCrashed' +for i in $(seq 1 10); do + score=0 + if egrep "$PATTERN" $TMPFILE; then + if egrep "string \"kenny" $TMPFILE; then + score=$(($score + 1)) + fi + + # legacy signal must not have the report_path and additional metadata + + if ! egrep "string \"/opt/usr/share/crash/dump.*kenny" $TMPFILE; then + score=$(($score + 1)) + fi + + if ! egrep -A1 "string \"sys.signal" $TMPFILE; then + score=$(($score + 1)) + fi + + if [ $score -eq 3 ]; then + exit_ok + fi + fi + + sleep 1 +done + +fail "legacy dbus signal does not match" -- 2.7.4 From 4cdcd0eb0149e687153f8d2992bec8262ef2ad2d Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Tue, 21 Jan 2020 10:54:05 +0100 Subject: [PATCH 02/16] Release 6.0.7 This version adds support option to support legacy 'ProcessCrashed' signal. Change-Id: I964c7f8a58ac96f3055403d9a5c33aee223747e3 --- 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 4376521..4826205 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -13,7 +13,7 @@ Name: crash-worker Summary: Coredump handler and report generator for Tizen -Version: 6.0.6 +Version: 6.0.7 Release: 1 Group: Framework/system License: Apache-2.0 and BSD-2-Clause and MIT diff --git a/packaging/crash-worker_system-tests.spec b/packaging/crash-worker_system-tests.spec index 0afd6c9..9ef7f9c 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: 6.0.6 +Version: 6.0.7 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From 8cb62d62fc27bd97311521c3b2b69562d4bd81b9 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 22 Jan 2020 15:51:26 +0100 Subject: [PATCH 03/16] Add missing system test to spec Change-Id: I7d7b4d2a91e40938e65cb9153ebfc8d0e320876e --- packaging/crash-worker_system-tests.spec | 1 + 1 file changed, 1 insertion(+) diff --git a/packaging/crash-worker_system-tests.spec b/packaging/crash-worker_system-tests.spec index 9ef7f9c..2350dac 100644 --- a/packaging/crash-worker_system-tests.spec +++ b/packaging/crash-worker_system-tests.spec @@ -65,6 +65,7 @@ cd tests/system %{_libdir}/crash-worker_system-tests/crash_root_path/crash_root_path.sh %{_libdir}/crash-worker_system-tests/critical_process/critical_process.sh %{_libdir}/crash-worker_system-tests/dbus_notify/dbus_notify.sh +%{_libdir}/crash-worker_system-tests/dbus_notify_legacy/dbus_notify_legacy.sh %{_libdir}/crash-worker_system-tests/dump_systemstate_extras/dump_systemstate_extras.sh %{_libdir}/crash-worker_system-tests/extra_script/extra_script.sh %{_libdir}/crash-worker_system-tests/info_file/info_file.sh -- 2.7.4 From b88a8a3e7360d9fe12fd8c6e3ed3dc492aac2187 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 23 Jan 2020 13:43:46 +0100 Subject: [PATCH 04/16] Fix livedump API DBus access policy This commit partially reverts commit f24c53fb20 ("crash-service: Restrict allow livedump API via privilege") As discussed with SRPOL Security, the 'group membership' of given process does not map directly to process 'having privilege', as verified by Cynara/security-manager. For now the checking has to be done per-user for system services and via-privilege for apps. Change-Id: I2a7b76174adb22be8ffe55e41678b48938d90bbb --- src/crash-service/crash-service.conf | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/crash-service/crash-service.conf b/src/crash-service/crash-service.conf index 7aac3d0..dd38a30 100644 --- a/src/crash-service/crash-service.conf +++ b/src/crash-service/crash-service.conf @@ -13,6 +13,11 @@ send_interface="org.tizen.system.crash.livedump" send_member="livedump_pid"/> + + + Date: Thu, 23 Jan 2020 17:49:04 +0100 Subject: [PATCH 05/16] Release 6.0.8 This release brings minor fixes to dbus policy and system test. Change-Id: I2bd9cc0a7979dd92e0fe819e0eeca0ec37afb28c --- 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 4826205..a37523d 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -13,7 +13,7 @@ Name: crash-worker Summary: Coredump handler and report generator for Tizen -Version: 6.0.7 +Version: 6.0.8 Release: 1 Group: Framework/system License: Apache-2.0 and BSD-2-Clause and MIT diff --git a/packaging/crash-worker_system-tests.spec b/packaging/crash-worker_system-tests.spec index 2350dac..22d6e86 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: 6.0.7 +Version: 6.0.8 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From f3c0b21a8e98a6c297974a9503cf75ace2fbe72a Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 13 Feb 2020 09:27:41 +0100 Subject: [PATCH 06/16] Drop sys-assert leftovers Change-Id: Ia220cc73a1b1e60f165aedd58cdfe9360c8de63b --- CMakeLists.txt | 5 ----- include/defs.h.in | 1 - src/crash-manager/crash-manager.h | 16 ++++++---------- 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 630e1aa..cf0df7b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,11 +10,6 @@ ADD_SUBDIRECTORY(include) INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/include) ADD_SUBDIRECTORY(src/crash-manager) - -IF("${SYS_ASSERT}" STREQUAL "ON") - ADD_SUBDIRECTORY(src/sys-assert) -ENDIF("${SYS_ASSERT}" STREQUAL "ON") - ADD_SUBDIRECTORY(src/crash-stack) ADD_SUBDIRECTORY(src/dump_systemstate) diff --git a/include/defs.h.in b/include/defs.h.in index 3ac9871..30b61f4 100644 --- a/include/defs.h.in +++ b/include/defs.h.in @@ -6,7 +6,6 @@ #define CRASH_PATH "@CRASH_PATH@" #define CRASH_ROOT_PATH "@CRASH_ROOT_PATH@" #define CRASH_TEMP "@CRASH_TEMP@" -#define SYS_ASSERT "@SYS_ASSERT@" #define CRASH_STACK_BIN_PATH "@CRASH_STACK_BIN_PATH@" #define CRASH_POPUP_BIN_PATH "@CRASH_POPUP_BIN_PATH@" #define CRASH_NOTIFY_BIN_PATH "@CRASH_NOTIFY_BIN_PATH@" diff --git a/src/crash-manager/crash-manager.h b/src/crash-manager/crash-manager.h index 7c102bc..1e4dbe5 100644 --- a/src/crash-manager/crash-manager.h +++ b/src/crash-manager/crash-manager.h @@ -28,14 +28,17 @@ /* Paths and variables */ struct crash_info { + bool livedump; + bool kill; + bool print_result_path; pid_t pid_info; pid_t tid_info; int uid_info; int gid_info; int sig_info; + int prstatus_fd; char *cmd_line; char *cmd_path; - time_t time_info; char *temp_dir; char *name; char *result_path; @@ -43,17 +46,10 @@ struct crash_info { char *info_path; char *core_path; char *log_path; + char *output_path; char appid[APPID_MAX]; char pkgid[PKGNAME_MAX]; - char *output_path; - bool livedump; - bool kill; - bool print_result_path; -#ifdef SYS_ASSERT - char *sysassert_cs_path; - bool have_sysassert_report; -#endif - int prstatus_fd; + time_t time_info; }; bool crash_manager_direct(struct crash_info *cinfo); -- 2.7.4 From 4308eba7b0d629907ff6c403dc7a6548a23a970f Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 13 Feb 2020 15:24:27 +0100 Subject: [PATCH 07/16] crash-manager: Be a bit more verbose about consequence of missing TID Change-Id: If5b5a4c5050c9fc5afea233311de6145222c445f --- 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 753d1de..33f1820 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -334,7 +334,7 @@ bool set_crash_info(struct crash_info *cinfo) } else { cinfo->tid_info = find_crash_tid(cinfo->pid_info); if (cinfo->tid_info < 0) { - _I("TID not found"); + _W("TID not found. Assuming TID = PID (%d).", cinfo->pid_info); cinfo->tid_info = cinfo->pid_info; } } -- 2.7.4 From 512faff745cd45df3c68fd232f206ffb96328d43 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 12 Feb 2020 16:00:15 +0100 Subject: [PATCH 08/16] Use package id as prefix for report files Change-Id: I37ea45e2c0a262e9bcd2bb3bcdb97edba83cbc7d --- src/crash-manager/crash-manager.c | 89 +++++++++++++++------------------------ src/crash-manager/crash-manager.h | 1 + 2 files changed, 34 insertions(+), 56 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 33f1820..29337b7 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -363,25 +363,23 @@ bool set_crash_info(struct crash_info *cinfo) return false; } - if (asprintf(&cinfo->name, "%s_%d_%s", basename(cinfo->cmd_path), + bool is_app = false; + if (get_appid(cinfo->cmd_line, cinfo->appid, sizeof(cinfo->appid)) < 0) { + snprintf(cinfo->appid, sizeof(cinfo->appid), "%s", basename(cinfo->cmd_line)); + snprintf(cinfo->pkgid, sizeof(cinfo->pkgid), "%s", basename(cinfo->cmd_line)); + } else { + is_app = true; + if (get_pkgid(cinfo->appid, cinfo->pkgid, sizeof(cinfo->pkgid)) < 0) + snprintf(cinfo->pkgid, sizeof(cinfo->pkgid), "%s", cinfo->appid); + } + + if (asprintf(&cinfo->name, "%s_%d_%s", is_app ? cinfo->pkgid : basename(cinfo->cmd_path), cinfo->pid_info, date) == -1) { _E("Failed to snprintf for name"); cinfo->name = NULL; goto rm_temp; } - if (config.allow_zip) - ret = asprintf(&cinfo->result_path, - "%s/%s.zip", crash_dump_path, cinfo->name); - else - ret = asprintf(&cinfo->result_path, - "%s/%s", crash_dump_path, cinfo->name); - if (ret == -1) { - _E("Failed to asprintf for result path"); - cinfo->result_path = NULL; - goto rm_temp; - } - if (asprintf(&cinfo->pfx, "%s/%s", cinfo->temp_dir, cinfo->name) == -1) { _E("Failed to asprintf for pfx"); cinfo->pfx = NULL; @@ -411,17 +409,21 @@ bool set_crash_info(struct crash_info *cinfo) goto rm_temp; } - if (set_prstatus(cinfo) < 0) + if (asprintf(&cinfo->zip_path, "%s/report.zip", cinfo->temp_dir) == -1) { + _E("Out of memory"); goto rm_temp; - - if (get_appid(cinfo->cmd_line, cinfo->appid, sizeof(cinfo->appid)) < 0) { - snprintf(cinfo->appid, sizeof(cinfo->appid), "%s", basename(cinfo->cmd_line)); - snprintf(cinfo->pkgid, sizeof(cinfo->pkgid), "%s", basename(cinfo->cmd_line)); - } else { - if (get_pkgid(cinfo->appid, cinfo->pkgid, sizeof(cinfo->pkgid)) < 0) - snprintf(cinfo->pkgid, sizeof(cinfo->pkgid), "%s", cinfo->appid); } + bool is_fullreport = config.report_type >= REP_TYPE_FULL; + const char *suffix = is_fullreport ? (config.allow_zip ? ".zip" : "") : ".info"; + + ret = asprintf(&cinfo->result_path, "%s/%s%s", crash_dump_path, cinfo->name, suffix); + if (ret == -1) + goto rm_temp; + + if (set_prstatus(cinfo) < 0) + goto rm_temp; + return true; rm_temp: @@ -978,31 +980,10 @@ static void clean_dump(void) static void compress(struct crash_info *cinfo) { - int ret, lock_fd; - char zip_path[PATH_MAX]; - - ret = snprintf(zip_path, sizeof(zip_path), "%s/report.zip", - cinfo->temp_dir); - if (ret < 0) { - _E("Failed to snprintf for zip path"); - return; - } - - char *args[] = {"/bin/zip", "-qyr", zip_path, cinfo->name, NULL}; + char *args[] = {"/bin/zip", "-qyr", cinfo->zip_path, cinfo->name, NULL}; spawn_param_s param1 = { .fn = spawn_nullstdfds }; spawn_param_s param0 = { .fn = spawn_chdir, .u.char_ptr = cinfo->temp_dir, .next = ¶m1 }; (void)spawn_wait(args, NULL, ¶m0, ZIP_TIMEOUT_MS, NULL); - - if ((lock_fd = lock_dumpdir()) < 0) - return; - if (!rename(zip_path, cinfo->result_path)) - clean_dump(); - else - _E("Failed to move %s to %s", zip_path, cinfo->result_path); - unlock_dumpdir(lock_fd); - - if (remove_dir(cinfo->temp_dir, 1) < 0) - _E("Failed to delete temp directory"); } static void move_dump_data(const char *from_path, const struct crash_info *cinfo) @@ -1050,6 +1031,7 @@ static void free_crash_info(struct crash_info *cinfo) free(cinfo->info_path); free(cinfo->core_path); free(cinfo->log_path); + free(cinfo->zip_path); } void crash_info_init(struct crash_info *cinfo) @@ -1069,6 +1051,7 @@ void crash_info_init(struct crash_info *cinfo) cinfo->info_path = NULL; cinfo->core_path = NULL; cinfo->log_path = NULL; + cinfo->zip_path = NULL; } static bool run(struct crash_info *cinfo) @@ -1094,6 +1077,7 @@ static bool run(struct crash_info *cinfo) return false; } + char *temp_report; if (config.report_type >= REP_TYPE_FULL) { /* Save shared objects info (file names, bulid IDs, rpm package names) */ save_so_info(cinfo); @@ -1103,21 +1087,14 @@ static bool run(struct crash_info *cinfo) if (extra_script_pid > 0) wait_for_pid(extra_script_pid, NULL); - /* Tar compression */ if (config.allow_zip) compress(cinfo); - else - 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; - _E("asprintf() error: %m"); - return false; - } - move_dump_data(cinfo->info_path, cinfo); - } + + temp_report = config.allow_zip ? cinfo->zip_path : cinfo->pfx; + } else + temp_report = cinfo->info_path; + + move_dump_data(temp_report, cinfo); if (cinfo->print_result_path) printf("REPORT_PATH=%s\n", cinfo->result_path); diff --git a/src/crash-manager/crash-manager.h b/src/crash-manager/crash-manager.h index 1e4dbe5..275fb5a 100644 --- a/src/crash-manager/crash-manager.h +++ b/src/crash-manager/crash-manager.h @@ -46,6 +46,7 @@ struct crash_info { char *info_path; char *core_path; char *log_path; + char *zip_path; char *output_path; char appid[APPID_MAX]; char pkgid[PKGNAME_MAX]; -- 2.7.4 From 22a0cccdad107ecfae242fe8c348361421c2dcd4 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Fri, 14 Feb 2020 17:20:51 +0100 Subject: [PATCH 09/16] Simplify struct crash_info setup Change-Id: Ibbb13c4534b89d6e1ee4b951a438ae1748ae30d2 --- src/crash-manager/crash-manager.c | 226 +++++++++++++++----------------------- src/crash-manager/crash-manager.h | 7 +- 2 files changed, 88 insertions(+), 145 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 29337b7..ee717f2 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -98,76 +98,69 @@ static int appinfo_get_appid_func(pkgmgrinfo_appinfo_h handle, return ret; } -/* get application ID by pkgmgrinfo filter */ -static int get_appid(char *exepath, char *appid, int len) +static bool get_appid(char *exepath, char **appid) { pkgmgrinfo_appinfo_filter_h handle = NULL; - int count, ret; - char *aid = NULL; + bool have_appid = false; - ret = pkgmgrinfo_appinfo_filter_create(&handle); - if (ret != PMINFO_R_OK) { - ret = -1; - goto out; - } + if (pkgmgrinfo_appinfo_filter_create(&handle) != PMINFO_R_OK) + return false; - ret = pkgmgrinfo_appinfo_filter_add_string(handle, PMINFO_APPINFO_PROP_APP_EXEC, exepath); - if (ret != PMINFO_R_OK) { - ret = -1; + if (pkgmgrinfo_appinfo_filter_add_string(handle, PMINFO_APPINFO_PROP_APP_EXEC, exepath) != PMINFO_R_OK) goto out_free; - } - ret = pkgmgrinfo_appinfo_filter_count(handle, &count); - if (ret != PMINFO_R_OK) { - ret = -1; + int count = 0; + if (pkgmgrinfo_appinfo_filter_count(handle, &count) || count < 1) goto out_free; - } - if (count < 1) { - ret = -1; + char *aid = NULL; + if (pkgmgrinfo_appinfo_filter_foreach_appinfo(handle, appinfo_get_appid_func, &aid) != PMINFO_R_OK) goto out_free; - } - ret = pkgmgrinfo_appinfo_filter_foreach_appinfo(handle, appinfo_get_appid_func, &aid); - if (ret != PMINFO_R_OK) { - ret = -1; - goto out_free; - } - if (aid) { - snprintf(appid, len, "%s", aid); - ret = 0; - free(aid); - } + have_appid = aid != NULL; + if (have_appid) + *appid = aid; out_free: pkgmgrinfo_appinfo_filter_destroy(handle); -out: - return ret; + return have_appid; } -/* get package ID by appid */ -static int get_pkgid(char *appid, char *pkgid, int len) +static bool get_pkgid(char *appid, char **pkgid) { pkgmgrinfo_appinfo_h handle = NULL; - int ret; - char *pkid = NULL; - ret = pkgmgrinfo_appinfo_get_appinfo(appid, &handle); - if (ret != PMINFO_R_OK) { - ret = -1; - goto out; - } - ret = pkgmgrinfo_appinfo_get_pkgid(handle, &pkid); - if (ret != PMINFO_R_OK) { - ret = -1; + if (pkgmgrinfo_appinfo_get_appinfo(appid, &handle) != PMINFO_R_OK) + return false; + + bool have_pkgid = false; + char *p = NULL; + if (pkgmgrinfo_appinfo_get_pkgid(handle, &p) != PMINFO_R_OK) goto out_free; - } - snprintf(pkgid, len, "%s", pkid); + + have_pkgid = p != NULL; + if (have_pkgid) + *pkgid = p; out_free: pkgmgrinfo_appinfo_destroy_appinfo(handle); -out: - return ret; + return have_pkgid; +} + +static bool set_appinfo(char *exepath, char **appid, char **pkgid) +{ + char *a = NULL, *p = NULL; + + if (get_appid(exepath, &a)) { + *appid = a; + *pkgid = get_pkgid(a, &p) ? p : strdup(a); + return true; + } + + char *bn = basename(exepath); + *appid = strdup(bn); + *pkgid = strdup(bn); + return false; } static int prepare_paths(struct crash_info* cinfo) @@ -308,127 +301,76 @@ close_fd: return -1; } -bool set_crash_info(struct crash_info *cinfo) -{ - int ret; - char *temp_dir_ret = NULL; - char date[80]; - struct tm loc_tm; +static void set_crash_info_defaults(struct crash_info *cinfo) +{ if (cinfo->livedump) { - if (cinfo->kill) - cinfo->sig_info = 9; - else - cinfo->sig_info = 0; - } - - 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 false; - } + cinfo->sig_info = cinfo->kill ? 9 : 0; + } else if (cinfo->tid_info == -1) + cinfo->tid_info = find_crash_tid(cinfo->pid_info); if (cinfo->tid_info == -1) { - if (cinfo->livedump) { - cinfo->tid_info = cinfo->pid_info; - } else { - cinfo->tid_info = find_crash_tid(cinfo->pid_info); - if (cinfo->tid_info < 0) { - _W("TID not found. Assuming TID = PID (%d).", cinfo->pid_info); - cinfo->tid_info = cinfo->pid_info; - } - } + _W("TID not known. Assuming TID = PID (%d).", cinfo->pid_info); + cinfo->tid_info = cinfo->pid_info; } +} + +/* Note: caller of this function is responsible for cleaning up the cinfo on failure */ +bool set_crash_info(struct crash_info *cinfo) +{ + set_crash_info_defaults(cinfo); if (!get_cmd_info(cinfo)) { _E("Failed to get command info"); return false; } - if (cinfo->time_info == 0) - cinfo->time_info = time(NULL); - + char date[80]; + struct tm loc_tm; + cinfo->time_info = time(NULL); localtime_r(&cinfo->time_info, &loc_tm); strftime(date, sizeof(date), "%Y%m%d%H%M%S", &loc_tm); - if (asprintf(&cinfo->temp_dir, "%s/crash.XXXXXX", crash_temp_path) == -1) { - _E("Failed to asprintf for temp_dir"); - cinfo->temp_dir = NULL; - return false; - } + if (asprintf(&cinfo->temp_dir, "%s/crash.XXXXXX", crash_temp_path) == -1) + goto out_oom; - temp_dir_ret = mkdtemp(cinfo->temp_dir); - if (!temp_dir_ret || access(temp_dir_ret, F_OK)) { - _E("Failed to mkdtemp for temp_dir"); + if (mkdtemp(cinfo->temp_dir) == NULL || access(cinfo->temp_dir, F_OK)) { + _E("Failed to create temporary directory %s: %m", cinfo->temp_dir); return false; } - bool is_app = false; - if (get_appid(cinfo->cmd_line, cinfo->appid, sizeof(cinfo->appid)) < 0) { - snprintf(cinfo->appid, sizeof(cinfo->appid), "%s", basename(cinfo->cmd_line)); - snprintf(cinfo->pkgid, sizeof(cinfo->pkgid), "%s", basename(cinfo->cmd_line)); - } else { - is_app = true; - if (get_pkgid(cinfo->appid, cinfo->pkgid, sizeof(cinfo->pkgid)) < 0) - snprintf(cinfo->pkgid, sizeof(cinfo->pkgid), "%s", cinfo->appid); - } - - if (asprintf(&cinfo->name, "%s_%d_%s", is_app ? cinfo->pkgid : basename(cinfo->cmd_path), - cinfo->pid_info, date) == -1) { - _E("Failed to snprintf for name"); - cinfo->name = NULL; - goto rm_temp; - } - - if (asprintf(&cinfo->pfx, "%s/%s", cinfo->temp_dir, cinfo->name) == -1) { - _E("Failed to asprintf for pfx"); - cinfo->pfx = NULL; - goto rm_temp; - } - ret = mkdir(cinfo->pfx, 0775); - if (ret < 0) { - _E("Failed to mkdir for %s", cinfo->pfx); - goto rm_temp; - } + const char *suffix = config.report_type >= REP_TYPE_FULL ? (config.allow_zip ? ".zip" : "") : ".info"; + bool is_app = set_appinfo(cinfo->cmd_line, &cinfo->appid, &cinfo->pkgid); + if (!cinfo->appid || !cinfo->pkgid) + goto out_oom; - if (asprintf(&cinfo->info_path, "%s/%s.info", cinfo->pfx, cinfo->name) == -1) { - _E("Failed to asprintf for info path"); - cinfo->info_path = NULL; - goto rm_temp; - } - - if (asprintf(&cinfo->core_path, "%s/%s.coredump", cinfo->pfx, cinfo->name) == -1) { - _E("Failed to asprintf for core path"); - cinfo->core_path = NULL; - goto rm_temp; - } + if (-1 == asprintf(&cinfo->name, "%s_%d_%s", is_app ? cinfo->pkgid : basename(cinfo->cmd_path), cinfo->pid_info, date) + || -1 == asprintf(&cinfo->pfx, "%s/%s", cinfo->temp_dir, cinfo->name) + || -1 == asprintf(&cinfo->info_path, "%s/%s.info", cinfo->pfx, cinfo->name) + || -1 == asprintf(&cinfo->core_path, "%s/%s.coredump", cinfo->pfx, cinfo->name) + || -1 == asprintf(&cinfo->log_path, "%s/%s.log", cinfo->pfx, cinfo->name) + || -1 == asprintf(&cinfo->zip_path, "%s/report.zip", cinfo->temp_dir) + || -1 == asprintf(&cinfo->result_path, "%s/%s%s", crash_dump_path, cinfo->name, suffix)) + goto out_oom; - if (asprintf(&cinfo->log_path, "%s/%s.log", cinfo->pfx, cinfo->name) == -1) { - _E("Failed to asprintf for log path"); - cinfo->log_path = NULL; - goto rm_temp; - } - if (asprintf(&cinfo->zip_path, "%s/report.zip", cinfo->temp_dir) == -1) { - _E("Out of memory"); - goto rm_temp; + if (mkdir(cinfo->pfx, 0775) < 0) { + _E("Failed to mkdir %s: %m", cinfo->pfx); + goto out_rm_temp; } - bool is_fullreport = config.report_type >= REP_TYPE_FULL; - const char *suffix = is_fullreport ? (config.allow_zip ? ".zip" : "") : ".info"; - - ret = asprintf(&cinfo->result_path, "%s/%s%s", crash_dump_path, cinfo->name, suffix); - if (ret == -1) - goto rm_temp; - if (set_prstatus(cinfo) < 0) - goto rm_temp; + goto out_rm_temp; return true; -rm_temp: +out_rm_temp: remove_dir(cinfo->temp_dir, 1); return false; + +out_oom: + _E("Out of memory"); + return false; } static void launch_crash_popup(struct crash_info *cinfo) @@ -1032,6 +974,8 @@ static void free_crash_info(struct crash_info *cinfo) free(cinfo->core_path); free(cinfo->log_path); free(cinfo->zip_path); + free(cinfo->appid); + free(cinfo->pkgid); } void crash_info_init(struct crash_info *cinfo) @@ -1052,6 +996,8 @@ void crash_info_init(struct crash_info *cinfo) cinfo->core_path = NULL; cinfo->log_path = NULL; cinfo->zip_path = NULL; + cinfo->appid = NULL; + cinfo->pkgid = NULL; } static bool run(struct crash_info *cinfo) diff --git a/src/crash-manager/crash-manager.h b/src/crash-manager/crash-manager.h index 275fb5a..7e284e8 100644 --- a/src/crash-manager/crash-manager.h +++ b/src/crash-manager/crash-manager.h @@ -23,9 +23,6 @@ #include #include -#define APPID_MAX 128 -#define PKGNAME_MAX 128 - /* Paths and variables */ struct crash_info { bool livedump; @@ -48,8 +45,8 @@ struct crash_info { char *log_path; char *zip_path; char *output_path; - char appid[APPID_MAX]; - char pkgid[PKGNAME_MAX]; + char *appid; + char *pkgid; time_t time_info; }; -- 2.7.4 From 20a026aaf3befdd41121c4ccf3bee0e32df05286 Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Mon, 17 Feb 2020 17:07:32 +0100 Subject: [PATCH 10/16] Magic shall not prevail Change-Id: I01eded05dc377efd992a2fa175891ac6765b4114 Signed-off-by: Michal Bloch --- 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 ee717f2..d8060da 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -305,7 +305,7 @@ close_fd: static void set_crash_info_defaults(struct crash_info *cinfo) { if (cinfo->livedump) { - cinfo->sig_info = cinfo->kill ? 9 : 0; + cinfo->sig_info = cinfo->kill ? SIGKILL : 0; } else if (cinfo->tid_info == -1) cinfo->tid_info = find_crash_tid(cinfo->pid_info); @@ -325,7 +325,7 @@ bool set_crash_info(struct crash_info *cinfo) return false; } - char date[80]; + char date[16]; struct tm loc_tm; cinfo->time_info = time(NULL); localtime_r(&cinfo->time_info, &loc_tm); -- 2.7.4 From b7343f562b0d0b93df5d6a2b7728ba73f2a0d7f9 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Mon, 17 Feb 2020 11:02:07 +0100 Subject: [PATCH 11/16] Release 6.0.9 This release brings numerous fixes and one change - reports are named with pkg name prefix for apps (regular processes still are named after cmdline). Change-Id: Ia0b6345e5d3735fa7e15cd0621818e55118a11f9 --- 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 a37523d..1e5e614 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -13,7 +13,7 @@ Name: crash-worker Summary: Coredump handler and report generator for Tizen -Version: 6.0.8 +Version: 6.0.9 Release: 1 Group: Framework/system License: Apache-2.0 and BSD-2-Clause and MIT diff --git a/packaging/crash-worker_system-tests.spec b/packaging/crash-worker_system-tests.spec index 22d6e86..b3321f1 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: 6.0.8 +Version: 6.0.9 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From 1257dc8c56782827b064e7aebf512bfd369a284c Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 20 Feb 2020 19:26:59 +0100 Subject: [PATCH 12/16] config: Be more verbose about consequence of ExtraScript OOM condition Change-Id: I7add0d5df5fc2a0369c011574eff7a1eccd8343f --- src/shared/config.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/shared/config.c b/src/shared/config.c index b7f32a5..be14826 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -77,7 +77,8 @@ static bool config_load_from_dict(config_t *c, const dictionary *ini) if (extra_script) { free(c->extra_script); c->extra_script = extra_script; - } + } else + _W("Out of memory. ExtraScript will not be executed."); } str = iniparser_getstring(ini, CRASH_SECTION ":ReportType", NULL); -- 2.7.4 From 72af6be8b870eda1d3406acc3c9e04a1dd902765 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 13 Feb 2020 09:30:43 +0100 Subject: [PATCH 13/16] Complain when livecoredumper is needed and not installed This brings back functionality dropped in 22a0cccdad ("Simplify struct crash_info setup"). Change-Id: If5ddddee6b99784d7ada9a93e89d3ad975facb6b --- src/crash-manager/crash-manager.c | 5 +++++ src/crash-manager/crash-manager.h | 2 ++ src/crash-manager/main.c | 5 +++++ src/crash-service/crash-service.c | 5 +++++ 4 files changed, 17 insertions(+) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index d8060da..f075ca6 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -85,6 +85,11 @@ config_t config; static char* crash_dump_path; static char* crash_temp_path; +bool have_livecoredumper(void) +{ + return access(LIVEDUMPER_BIN_PATH, X_OK) == 0; +} + /* pkgmgrinfo filter list function for getting application ID */ static int appinfo_get_appid_func(pkgmgrinfo_appinfo_h handle, void *user_data) diff --git a/src/crash-manager/crash-manager.h b/src/crash-manager/crash-manager.h index 7e284e8..6051f82 100644 --- a/src/crash-manager/crash-manager.h +++ b/src/crash-manager/crash-manager.h @@ -54,4 +54,6 @@ bool crash_manager_direct(struct crash_info *cinfo); bool crash_manager_livedump_pid(pid_t pid, const char *dump_reason, char *report_path, size_t report_path_len); void crash_info_init(struct crash_info *cinfo); void crash_manager_free(struct crash_info *cinfo); + +bool have_livecoredumper(void); #endif diff --git a/src/crash-manager/main.c b/src/crash-manager/main.c index 0b34d3b..26b8e95 100644 --- a/src/crash-manager/main.c +++ b/src/crash-manager/main.c @@ -113,6 +113,11 @@ static bool parse_args(struct crash_info *cinfo, int argc, char *argv[]) } } + if (cinfo->livedump && !have_livecoredumper()) { + printf("livecoredumper not available - can not perform livedump.\n"); + return false; + } + if (!pid_set || (!cinfo->livedump && (!gid_set || !uid_set || !sig_set))) { printf("Not enough parameters.\n\n"); print_help(argv[0]); diff --git a/src/crash-service/crash-service.c b/src/crash-service/crash-service.c index 7b0a798..0917fcb 100644 --- a/src/crash-service/crash-service.c +++ b/src/crash-service/crash-service.c @@ -315,6 +315,11 @@ static bool dbus_init(void) int main(void) { + if (!have_livecoredumper()) { + _E("livecoredumper not available - can not provide livedump API. Terminating.\n"); + return EXIT_FAILURE; + } + loop = g_main_loop_new(NULL, false); if (!dbus_init()) { -- 2.7.4 From 59e0a9df35e66a81ceeefdfdbc6b05986770a0ae Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 20 Feb 2020 16:46:03 +0100 Subject: [PATCH 14/16] util: Add function to normalize paths produced by kernel.core_pattern %E Change-Id: I0afbb7d946d16b2fff293c8697ae2e6d7db53e43 --- src/shared/util.c | 9 +++++++++ src/shared/util.h | 2 ++ 2 files changed, 11 insertions(+) diff --git a/src/shared/util.c b/src/shared/util.c index 6afc936..bfea12b 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -578,6 +578,15 @@ bool string_ends_with(const char *string, const char *suffix) return (string_len >= suffix_len) && !strcmp(string + string_len - suffix_len, suffix); } +/* what for: kernel.core_pattern's %E replaces / with !, this function does opposite process */ +void kernel_exe_path_normalize(char *path) +{ + assert(path); + + for (size_t i = 0, len = strlen(path); i < len; i++) + path[i] = path[i] == '!' ? '/' : path[i]; +} + bool file_exists(const char *path) { struct stat buf; diff --git a/src/shared/util.h b/src/shared/util.h index f820997..ad0bd42 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -68,6 +68,8 @@ char* concatenate(char *const vec[]); bool string_ends_with(const char *string, const char *suffix); +void kernel_exe_path_normalize(char *path); + bool file_exists(const char *path); bool file_exists_in_dir(const char *dir_path, const char *file_name); -- 2.7.4 From d36495389a133eb5b03134fe84fe93a2e4f93004 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 20 Feb 2020 19:26:08 +0100 Subject: [PATCH 15/16] Janitorial: '*' -> ' *' Place pointer signature consistently with rest of codebase. Change-Id: Ie183a11724fe886036cf2929ca6acdfac989da9b --- src/crash-manager/dbus_notify.c | 4 ++-- src/crash-manager/so-info.c | 10 +++++----- src/shared/util.c | 2 +- src/shared/util.h | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/crash-manager/dbus_notify.c b/src/crash-manager/dbus_notify.c index 41126c5..ca010e5 100644 --- a/src/crash-manager/dbus_notify.c +++ b/src/crash-manager/dbus_notify.c @@ -149,14 +149,14 @@ strdup_error: goto out; } -static GVariant* build_legacy_message_data(const struct NotifyParams *notify_params) +static GVariant *build_legacy_message_data(const struct NotifyParams *notify_params) { assert(notify_params); return g_variant_new("(ssss)", notify_params->cmd_name, notify_params->cmd_path, notify_params->appid, notify_params->pkgid); } -static GVariant* build_message_data(const struct NotifyParams *notify_params) +static GVariant *build_message_data(const struct NotifyParams *notify_params) { assert(notify_params); diff --git a/src/crash-manager/so-info.c b/src/crash-manager/so-info.c index d95a927..a9ff6ef 100644 --- a/src/crash-manager/so-info.c +++ b/src/crash-manager/so-info.c @@ -204,7 +204,7 @@ static char *get_exe_filename(char *line) return NULL; } -GSList* get_filepaths(char *map_path) +GSList *get_filepaths(char *map_path) { char *line = NULL; size_t len = 0; @@ -260,7 +260,7 @@ void free_rpm(rpmts ts) } } -char* get_rpm_info_as_string_for_pkgname(Header h, const char *pkg_name) +char *get_rpm_info_as_string_for_pkgname(Header h, const char *pkg_name) { char *result = NULL; @@ -276,13 +276,13 @@ char* get_rpm_info_as_string_for_pkgname(Header h, const char *pkg_name) return result; } -char* get_rpm_info_as_string(Header h) +char *get_rpm_info_as_string(Header h) { const char *name = headerGetString(h, RPMTAG_NAME); return get_rpm_info_as_string_for_pkgname(h, name); } -char* get_rpm_info(rpmts ts, const char* filename) +char *get_rpm_info(rpmts ts, const char* filename) { char *rpm_info = NULL; @@ -355,7 +355,7 @@ void search_for_tpk(rpmts ts, GSList *pkgs) rpmdbFreeIterator(mi); } -char* get_app_name_from_path(const char *file_path) +char *get_app_name_from_path(const char *file_path) { static const char *prefix[] = {"/usr/apps/", "/opt/usr/globalapps/"}; diff --git a/src/shared/util.c b/src/shared/util.c index bfea12b..76889e5 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -546,7 +546,7 @@ bool get_exe_path(pid_t pid, char *outbuf, size_t outbuf_len) * (argv and envp), which can be arrays of strings as well as NULL * pointer. */ -char* concatenate(char *const vec[]) +char *concatenate(char *const vec[]) { size_t length = 0; for (char *const *p = vec; p && *p; p++) diff --git a/src/shared/util.h b/src/shared/util.h index ad0bd42..882ce5e 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -64,7 +64,7 @@ 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[]); +char *concatenate(char *const vec[]); bool string_ends_with(const char *string, const char *suffix); -- 2.7.4 From 56f79e0829dd2532b01203f61bed4c3fa754e4d8 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Fri, 21 Feb 2020 11:54:24 +0100 Subject: [PATCH 16/16] util: Refactor get_cmd_info Change-Id: Ie7c0584487367f7c059c49b0cc309ef5ac7e6542 --- src/crash-manager/crash-manager.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index f075ca6..f194f30 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -236,20 +236,23 @@ static bool get_cmd_info(struct crash_info *cinfo) { assert(cinfo); - cinfo->cmd_line = malloc(PATH_MAX); - if (!cinfo->cmd_line) - return false; + char buf[PATH_MAX]; - if (!read_proc_file(cinfo->pid_info, "cmdline", cinfo->cmd_line, PATH_MAX, NULL)) + if (!read_proc_file(cinfo->pid_info, "cmdline", buf, sizeof(buf), NULL)) goto err; - char buf[PATH_MAX]; + cinfo->cmd_line = strdup(buf); + if (!cinfo->cmd_line) + return false; + if (!get_exe_path(cinfo->pid_info, buf, sizeof(buf))) goto err; cinfo->cmd_path = strdup(buf); + if (!cinfo->cmd_path) + goto err; - return !!cinfo->cmd_path; + return true; err: free(cinfo->cmd_line); -- 2.7.4