From b1f8fe54f2b1760f5def6bb3ba24f8b0678c9e77 Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Mon, 8 Jul 2019 16:49:34 +0200 Subject: [PATCH 01/16] Fix a bunch of minor issues found by Coverity. Change-Id: I6dc3f56f1d590a6a49800d28ba78d3185555a837 Signed-off-by: Michal Bloch --- src/livedumper/core.hpp | 4 ++++ src/livedumper/livedumper.hpp | 18 +++++++++++++----- src/livedumper/program.hpp | 1 + src/shared/spawn.c | 3 +++ 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/livedumper/core.hpp b/src/livedumper/core.hpp index 193577c..2f52efd 100644 --- a/src/livedumper/core.hpp +++ b/src/livedumper/core.hpp @@ -136,6 +136,10 @@ class Core { len = size; ssize_t cur_pos = lseek64(input, 0, SEEK_CUR); + if (cur_pos < 0) { + logger.log_error("lseek error: %s", std::system_category().default_error_condition(errno).message()); + return; + } ssize_t bytes_read; if ((bytes_read = read(input, buff, len)) == -1) { diff --git a/src/livedumper/livedumper.hpp b/src/livedumper/livedumper.hpp index 6e0869a..01a2225 100644 --- a/src/livedumper/livedumper.hpp +++ b/src/livedumper/livedumper.hpp @@ -47,9 +47,15 @@ class LiveDumper { Core m_core; std::ofstream m_core_file; int prstatus_fd = -1; + std::pair map_buff; public: - explicit LiveDumper(const pid_t pid) : m_pid(pid) {} + explicit LiveDumper(const pid_t pid) : m_pid(pid), map_buff(MAP_FAILED, 0) {} + + ~ LiveDumper () { + if (map_buff.first != MAP_FAILED) + munmap(map_buff.first, map_buff.second); + } void CollectTpids() { m_tpids.clear(); @@ -148,14 +154,16 @@ class LiveDumper { memcpy(&prstatus.pr_reg, ®isters, sizeof(prstatus.pr_reg)); prstatus.pr_pid = m_pid; - auto buff = reinterpret_cast(mmap(nullptr, sizeof(prstatus), + map_buff.first = mmap(nullptr, sizeof(prstatus), PROT_READ | PROT_WRITE, MAP_SHARED, prstatus_fd, - 0)); - if (buff == MAP_FAILED) + 0); + if (map_buff.first == MAP_FAILED) return; - memcpy(buff, &prstatus, sizeof(prstatus)); + map_buff.second = sizeof prstatus; + + memcpy(static_cast (map_buff.first), &prstatus, sizeof(prstatus)); } void AddNotes(const std::vector> &records) { diff --git a/src/livedumper/program.hpp b/src/livedumper/program.hpp index 855e7e4..f2eca7d 100644 --- a/src/livedumper/program.hpp +++ b/src/livedumper/program.hpp @@ -30,6 +30,7 @@ class ProgramTableEntryBase { memset(&header, 0, sizeof(header)); header.p_type = p_type; } + virtual ~ProgramTableEntryBase () {} }; template diff --git a/src/shared/spawn.c b/src/shared/spawn.c index baca45d..392a053 100644 --- a/src/shared/spawn.c +++ b/src/shared/spawn.c @@ -51,6 +51,9 @@ int spawn_setstderr(spawn_param_s *param) int spawn_nullstdfds(spawn_param_s *param) { int fd = open("/dev/null", O_RDWR); + if (fd < 0) + return -1; + int ret = dup2(fd, STDIN_FILENO) < 0 || dup2(fd, STDOUT_FILENO) < 0 || dup2(fd, STDERR_FILENO) < 0 ? -1 : 0; if (fd != STDIN_FILENO && fd != STDOUT_FILENO && fd != STDERR_FILENO) close(fd); -- 2.7.4 From c7c918c7c0e0bdf383e6fcdfe4f1facf195d74e5 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Tue, 9 Jul 2019 15:54:55 +0200 Subject: [PATCH 02/16] Release 5.5.16 - Fix Coverity issues Change-Id: Ic94d6570135a93575b793447c1fc5ea365c6a5f3 --- 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 4635a5b..f68f0b6 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -14,7 +14,7 @@ Name: crash-worker Summary: Crash-manager -Version: 5.5.15 +Version: 5.5.16 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 c80d9ac..92f3faa 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.15 +Version: 5.5.16 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From 5a4d1c937810a5017db5f47d73cc29f0f4f3520f Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Mon, 15 Jul 2019 11:09:11 +0200 Subject: [PATCH 03/16] Fix format specifier Change-Id: I5378d23401594aae9adf42ce595d6fad43c847ce --- src/livedumper/core.hpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/livedumper/core.hpp b/src/livedumper/core.hpp index 2f52efd..aebd3a4 100644 --- a/src/livedumper/core.hpp +++ b/src/livedumper/core.hpp @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -128,16 +129,16 @@ class Core { } void CopyData(int input, std::ofstream &output, size_t size) { - size_t len = PAGESIZE; + off64_t len = PAGESIZE; char buff[PAGESIZE]; while (size > 0) { if (size < len) len = size; - ssize_t cur_pos = lseek64(input, 0, SEEK_CUR); - if (cur_pos < 0) { - logger.log_error("lseek error: %s", std::system_category().default_error_condition(errno).message()); + off64_t cur_pos = lseek64(input, 0, SEEK_CUR); + if (cur_pos == -1) { + logger.log_error("lseek error: %s", std::system_category().default_error_condition(errno).message().c_str()); return; } @@ -145,7 +146,7 @@ class Core { if ((bytes_read = read(input, buff, len)) == -1) { // tell() returns 128bits value so I cast this to 64bit to be able to print it logger.log_error("read error at 0x%llx: %s\n", static_cast(output.tellp()), - std::system_category().default_error_condition(errno).message()); + std::system_category().default_error_condition(errno).message().c_str()); lseek64(input, cur_pos + len, SEEK_SET); output.seekp(len, std::ios_base::cur); size -= len; @@ -165,7 +166,7 @@ class Core { if (record->header.p_type != PT_LOAD) continue; if (lseek64(mem_fd, record->header.p_vaddr, SEEK_SET) == -1) - logger.log_error("lseek64 error: %s", std::system_category().default_error_condition(errno).message()); + logger.log_error("lseek64 error: %s", std::system_category().default_error_condition(errno).message().c_str()); if (!minicore || static_cast*>(record.get())->important) CopyData(mem_fd, core_file, record->header.p_filesz); else @@ -229,7 +230,11 @@ class Core { void DumpData(int fd, std::ofstream &output, typename T::Addr iaddress, typename T::Addr oaddress, size_t len, const std::string &desc) { lseek64(fd, iaddress, SEEK_SET); output.seekp(oaddress, std::ios_base::beg); - logger.log_info("dumping %s: 0x%x-0x%x to 0x%x (%d bytes)", desc.c_str(), iaddress, iaddress+len, oaddress, len); + constexpr const char * format = std::is_same::value ? + "dumping %s: 0x%" PRIx64 "-0x%" PRIx64 " to 0x%" PRIx64 " (%zu bytes)" : + "dumping %s: 0x%" PRIx32 "-0x%" PRIx32 " to 0x%" PRIx32 " (%zu bytes)"; + + logger.log_info(format, desc.c_str(), iaddress, iaddress+len, oaddress, len); CopyData(fd, output, len); } -- 2.7.4 From 34c62042fc362f40e42090b5db1c138ad883c251 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Mon, 15 Jul 2019 14:46:50 +0200 Subject: [PATCH 04/16] Skip test if livedumper doesn't exist Change-Id: Ibe22df39b4c8e572c29efec0f338cae0bef1bc9f --- tests/system/livedumper/livedumper.sh.template | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/system/livedumper/livedumper.sh.template b/tests/system/livedumper/livedumper.sh.template index 54d438b..51c9be1 100644 --- a/tests/system/livedumper/livedumper.sh.template +++ b/tests/system/livedumper/livedumper.sh.template @@ -8,6 +8,10 @@ fi . ${CRASH_WORKER_SYSTEM_TESTS}/utils/minicore-utils.sh +if [ ! -x /usr/bin/livedumper ]; then + exit_with_code "SKIP" ${SKIP_CODE} +fi + mount -o rw,remount / clean_crash_dump -- 2.7.4 From 9c54cfca4e88fae209cde90c299168273aeeb65e Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Mon, 15 Jul 2019 15:11:54 +0200 Subject: [PATCH 05/16] crash-manager executes livedumper with -m option Change-Id: If36b60500d18fe43065a4b9322446e9309e18564 --- 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 f505834..bf55763 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -785,6 +785,7 @@ static bool execute_livedumper(const struct crash_info *cinfo, int *exit_code) /* Execute livedumper */ char *args[] = { LIVEDUMPER_BIN_PATH, // livedumper filename path + "-m", // minilivecoredump "-P", prstatus_fd_str, "-f", coredump_path, pid_str, // %p - pid -- 2.7.4 From 363700b9166a71339b10e2251fff3352b2524f8a Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Mon, 15 Jul 2019 15:15:19 +0200 Subject: [PATCH 06/16] Release 5.5.17 - Fix format specifier in livedumper - crash-manager executes livedumper with -m option Change-Id: I1031fce26edd993612d1d0190fb31775488ff29e --- 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 f68f0b6..304659d 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -14,7 +14,7 @@ Name: crash-worker Summary: Crash-manager -Version: 5.5.16 +Version: 5.5.17 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 92f3faa..b082f56 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.16 +Version: 5.5.17 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From 7d7c99931e4cf5f1f8e525795c12a509c897872c Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Tue, 16 Jul 2019 13:46:29 +0200 Subject: [PATCH 07/16] Build all programs as PIE - needed for ASLR to work Change-Id: If965bb4ef9008bf280d211feeebe1a5a95de7252 --- src/crash-manager/CMakeLists.txt | 4 ++-- src/livedumper/CMakeLists.txt | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/crash-manager/CMakeLists.txt b/src/crash-manager/CMakeLists.txt index 6a20570..198a92c 100644 --- a/src/crash-manager/CMakeLists.txt +++ b/src/crash-manager/CMakeLists.txt @@ -35,12 +35,12 @@ TARGET_LINK_LIBRARIES(${PROJECT_NAME} ${crash-manager_pkgs_LDFLAGS} -pie -lrt) set(CRASH_POPUP crash-popup-launch) ADD_EXECUTABLE(${CRASH_POPUP} ${CRASH_POPUP}.c) -TARGET_LINK_LIBRARIES(${CRASH_POPUP} ${helper_pkgs_LDFLAGS}) +TARGET_LINK_LIBRARIES(${CRASH_POPUP} ${helper_pkgs_LDFLAGS} -pie) install(TARGETS ${CRASH_POPUP} DESTINATION libexec) SET(CRASH_NOTIFY crash-notify-send) ADD_EXECUTABLE(${CRASH_NOTIFY} dbus_notify.c) -TARGET_LINK_LIBRARIES(${CRASH_NOTIFY} ${helper_pkgs_LDFLAGS}) +TARGET_LINK_LIBRARIES(${CRASH_NOTIFY} ${helper_pkgs_LDFLAGS} -pie) install(TARGETS ${CRASH_NOTIFY} DESTINATION libexec) INSTALL(TARGETS ${PROJECT_NAME} DESTINATION bin diff --git a/src/livedumper/CMakeLists.txt b/src/livedumper/CMakeLists.txt index ea20fc6..efdf65a 100644 --- a/src/livedumper/CMakeLists.txt +++ b/src/livedumper/CMakeLists.txt @@ -18,7 +18,8 @@ endif(DLOG) set(LIVEDUMPER_SRCS main.cpp ${LOGGER_FILE}) add_executable(${LIVEDUMPER_BIN} ${LIVEDUMPER_SRCS}) -target_link_libraries(${LIVEDUMPER_BIN} -lelf -lthread_db) +set_property(TARGET ${LIVEDUMPER_BIN} APPEND_STRING PROPERTY COMPILE_FLAGS "-fPIE ") +target_link_libraries(${LIVEDUMPER_BIN} -lelf -lthread_db -pie) if("${LOGGER}" STREQUAL "dlog") include(FindPkgConfig) -- 2.7.4 From d53ff584fa26afd2e4d54559abaccb3c83eb0aa5 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 17 Jul 2019 13:27:08 +0200 Subject: [PATCH 08/16] defs.h: Fix crash-notify path Change-Id: I4e7cc3e516239d6642dc7214bc35472d8918cc3e --- include/defs.h.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/defs.h.in b/include/defs.h.in index f2f064c..b6eab96 100644 --- a/include/defs.h.in +++ b/include/defs.h.in @@ -7,7 +7,7 @@ #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_NOITFY_BIN_PATH@" +#define CRASH_NOTIFY_BIN_PATH "@CRASH_NOTIFY_BIN_PATH@" #define LOG_DUMP_BIN_PATH "@LOG_DUMP_BIN_PATH@" #define DUMP_SYSTEMSTATE_BIN_PATH "@DUMP_SYSTEMSTATE_BIN_PATH@" #define DUMP_SYSTEMSTATE_CONFIG_DIR_PATH "@DUMP_SYSTEMSTATE_CONFIG_DIR_PATH@" -- 2.7.4 From e4bd312b5833a4b7ca3a57b047c8617f28e717dd Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 17 Jul 2019 13:28:04 +0200 Subject: [PATCH 09/16] crash-manager: notify: Add missing report path to D-Bus signal Change-Id: I39d992d49b48f00f11103b47eec2027e655fe6c7 --- src/crash-manager/dbus_notify.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/crash-manager/dbus_notify.c b/src/crash-manager/dbus_notify.c index 68728a6..3a87808 100644 --- a/src/crash-manager/dbus_notify.c +++ b/src/crash-manager/dbus_notify.c @@ -260,6 +260,8 @@ static bool parse_cmdline(int ac, char *av[], struct NotifyParams *params) params->cmd_name = basename(optarg); else if (FLAG_CMDPATH == val) params->cmd_path = optarg; + else if (FLAG_REPORTPATH == val) + params->report_path = optarg; else if (FLAG_PID == val) params->pid = atoi(optarg); else if (FLAG_TID == val) -- 2.7.4 From 2dd4fd9e0022e4ec0b06c8248c8bf4c7a60a7ba3 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 17 Jul 2019 13:29:15 +0200 Subject: [PATCH 10/16] tests: Add crash-manager dbus notify system test Change-Id: I3e3c4669df4dd36edecd02c2fb245f455b76b4c5 --- packaging/crash-worker_system-tests.spec | 1 + tests/system/CMakeLists.txt | 1 + tests/system/dbus_notify/dbus_notify.sh.template | 75 ++++++++++++++++++++++++ 3 files changed, 77 insertions(+) create mode 100644 tests/system/dbus_notify/dbus_notify.sh.template diff --git a/packaging/crash-worker_system-tests.spec b/packaging/crash-worker_system-tests.spec index b082f56..2caa93e 100644 --- a/packaging/crash-worker_system-tests.spec +++ b/packaging/crash-worker_system-tests.spec @@ -59,6 +59,7 @@ cd tests/system %{_libdir}/crash-worker_system-tests/cmp_backtraces/cp.sh %{_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/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 diff --git a/tests/system/CMakeLists.txt b/tests/system/CMakeLists.txt index da31257..7db25e2 100644 --- a/tests/system/CMakeLists.txt +++ b/tests/system/CMakeLists.txt @@ -38,6 +38,7 @@ configure_test("log_dump_crash_root_path") configure_test("dump_systemstate_extras") configure_test("livedumper") configure_test("extra_script") +configure_test("dbus_notify") get_property(TESTS_LIST GLOBAL PROPERTY TMP_TESTS_LIST) diff --git a/tests/system/dbus_notify/dbus_notify.sh.template b/tests/system/dbus_notify/dbus_notify.sh.template new file mode 100644 index 0000000..c7e7349 --- /dev/null +++ b/tests/system/dbus_notify/dbus_notify.sh.template @@ -0,0 +1,75 @@ +#!/bin/bash + +# Check the report type change in the config file + +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" +# string "/opt/usr/share/crash/dump//sleep_2782_20150101113723.zip" +# int32 2782 +# int32 2782 +# array [ +# dict entry( +# string "arm.pc" +# variant uint32 3069275660 +# ) +# dict entry( +# string "arm.lr" +# variant uint32 3070220008 +# ) +# ] + +( ${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 +} + +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 + + if egrep "string \"/opt/usr/share/crash/dump.*kenny" $TMPFILE; then + score=$(($score + 1)) + fi + + if egrep "string \"${CRASH_WORKER_SYSTEM_TESTS}/utils/kenny" $TMPFILE; then + score=$(($score + 1)) + fi + + if [ $score -eq 3 ]; then + exit_with_code "SUCCESS" ${SUCCESS_CODE} + fi + fi + + sleep 1 +done + +exit_with_code "FAIL: dbus signal does not match" ${FAIL_CODE} -- 2.7.4 From 637e65d99a0fbe224753b73307e5e5143f257e97 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 18 Jul 2019 13:32:48 +0200 Subject: [PATCH 11/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 12/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 13/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 14/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 15/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 16/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