From af3e822c23ea149eb565b32c6c781ac99a705062 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Wed, 27 Nov 2019 11:16:36 +0100 Subject: [PATCH 01/16] license: add MIT license Some part of src/crash-stack/crash-stack-x86.c seems to be copied from libunwind. The part has MIT license. This adds MIT license text and an entry in the spec file. Change-Id: I8796d9ad2e61cd0671470eaaae184c2d00e6615f --- LICENSE.MIT | 9 +++++++++ packaging/crash-worker.spec | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 LICENSE.MIT diff --git a/LICENSE.MIT b/LICENSE.MIT new file mode 100644 index 0000000..5b16ef4 --- /dev/null +++ b/LICENSE.MIT @@ -0,0 +1,9 @@ +The MIT License + +Copyright (c) 2019 Samsung Electronics + +Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index 35853f0..6087233 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -154,7 +154,7 @@ mkdir -p %{buildroot}%{crash_temp} /usr/bin/chsmack -a "System" -t %{crash_temp} %files -%license LICENSE LICENSE.BSD +%license LICENSE LICENSE.BSD LICENSE.MIT %manifest crash-worker.manifest %defattr(-,crash_worker,crash_worker,-) %dir %{crash_root_path} -- 2.7.4 From 5b8d40f36c640d1f525a3c266578ecdcb835cf8a Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Thu, 28 Nov 2019 08:53:56 +0100 Subject: [PATCH 02/16] spec: add MIT to License field This adds MIT License to licenses specified in spec's License field. Change-Id: I40635546c35d11c777767740dea7bac2993bedf3 --- packaging/crash-worker.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index 6087233..08bcafb 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -16,7 +16,7 @@ Summary: Coredump handler and report generator for Tizen Version: 6.0.4 Release: 1 Group: Framework/system -License: Apache-2.0 and BSD +License: Apache-2.0 and BSD-2-Clause and MIT Source0: %{name}-%{version}.tar.gz Source1001: crash-worker.manifest BuildRequires: pkgconfig(dlog) -- 2.7.4 From 0d6a0e9136d7f1aff1050f1aeca88fe3d2e6b843 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Fri, 13 Dec 2019 11:46:13 +0100 Subject: [PATCH 03/16] Fix file permissions Change-Id: I078c745cc56e1cf108997f2e76897dbe4cb194da --- LICENSE | 0 NOTICE | 0 src/dump_systemstate/CMakeLists.txt | 0 3 files changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 LICENSE mode change 100755 => 100644 NOTICE mode change 100755 => 100644 src/dump_systemstate/CMakeLists.txt diff --git a/LICENSE b/LICENSE old mode 100755 new mode 100644 diff --git a/NOTICE b/NOTICE old mode 100755 new mode 100644 diff --git a/src/dump_systemstate/CMakeLists.txt b/src/dump_systemstate/CMakeLists.txt old mode 100755 new mode 100644 -- 2.7.4 From 928ff71c8ed1b7c7be6ff9f8a53bd12d0ae5e069 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Tue, 17 Dec 2019 14:56:06 +0100 Subject: [PATCH 04/16] crash-service: Stability monitor does not need to own crash-worker's dbus name Change-Id: Ida73ce27ac6ea040dcffb81f3a61e4096dc81940 --- src/crash-service/crash-service.conf | 1 - 1 file changed, 1 deletion(-) diff --git a/src/crash-service/crash-service.conf b/src/crash-service/crash-service.conf index 1bdcfb0..02bd574 100644 --- a/src/crash-service/crash-service.conf +++ b/src/crash-service/crash-service.conf @@ -14,7 +14,6 @@ send_member="livedump_pid"/> - -- 2.7.4 From 83045734d9c70a6b7d13ff504dccd3cacafffdbf Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Fri, 13 Dec 2019 13:11:08 +0100 Subject: [PATCH 05/16] config: Separate config loading from processing Change-Id: Ief38894e03904f9947ee72773f07044280f62b41 --- src/shared/config.c | 123 +++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 98 insertions(+), 25 deletions(-) diff --git a/src/shared/config.c b/src/shared/config.c index 3154752..1e728c4 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -49,9 +49,55 @@ enum ReportType report_type_from_str(const char *report_type_str) return REP_TYPE_INVALID; } -bool config_init(config_t *c, const char *const path) +static bool config_load_from_dict(config_t *c, dictionary *ini) { assert(c); + + char *str = iniparser_getstring(ini, CRASH_SECTION ":CrashRootPath", NULL); + if (str) { + char *crash_root_path = strdup(str); + if (!crash_root_path) { + _E("config: Unable to set CrashRootPath - aborting"); + return false; + } + + free(c->crash_root_path); + c->crash_root_path = crash_root_path; + } + + /* strdup() can technically fail, but we don't mind. It is better to + * create a report without the extra script than to abort completely. */ + str = iniparser_getstring(ini, CRASH_SECTION ":ExtraScript", NULL); + if (str) { + char *extra_script = strdup(str); + if (extra_script) { + free(c->extra_script); + c->extra_script = extra_script; + } + } + + str = iniparser_getstring(ini, CRASH_SECTION ":ReportType", NULL); + if (str) { + int type = report_type_from_str(str); + if (report_type_to_str(type)) + c->report_type = type; + } + +#define UPDATE(where, type, key) where = iniparser_get##type(ini, CRASH_SECTION ":" key, where) + + UPDATE(c->system_max_use, int, "SystemMaxUse"); + UPDATE(c->max_retention_sec, int, "MaxRetentionSec"); + UPDATE(c->max_crash_dump, int, "MaxCrashDump"); + UPDATE(c->dump_core, boolean, "DumpCore"); + UPDATE(c->allow_zip, boolean, "AllowZip"); + +#undef UPDATE + + return true; +} + +static bool config_load_from_path(config_t *c, const char *const path) +{ assert(path); dictionary *ini = iniparser_load(path); @@ -60,37 +106,64 @@ bool config_init(config_t *c, const char *const path) return false; } - bool ret = false; + bool ret = config_load_from_dict(c, ini); + iniparser_freedict(ini); -#define GET(type, key, defval) iniparser_get##type(ini, CRASH_SECTION ":" key, defval) + return ret; +} - c->crash_root_path = strdup(GET(string, "CrashRootPath", CRASH_ROOT_PATH)); - if (!c->crash_root_path) - goto out; +static bool config_apply_defaults(config_t *c) +{ + assert(c); - /* strdup() can technically fail, but we don't mind. It is better to - * create a report without the extra script than to abort completely. */ - char *extrascript = GET(string, "ExtraScript", NULL); - c->extra_script = extrascript ? strdup(extrascript) : NULL; + memset(c, 0, sizeof(*c)); - char *reptype = GET(string, "ReportType", (char *)report_type_strmap[REP_TYPE_FULL]); - c->report_type = report_type_from_str(reptype); - if (!report_type_to_str(c->report_type)) - goto out; + c->crash_root_path = strdup(CRASH_ROOT_PATH); + c->report_type = REP_TYPE_FULL; + c->system_max_use = SYSTEM_MAX_USE; + c->system_keep_free = SYSTEM_KEEP_FREE; + c->max_retention_sec = MAX_RETENTION_SEC; + c->max_crash_dump = MAX_CRASH_DUMP; + c->dump_core = DUMP_CORE; + c->allow_zip = ALLOW_ZIP; - 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", 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); + return c->crash_root_path != NULL; +} -#undef GET +static void config_dump(config_t *c) +{ + assert(c); - ret = true; -out: - iniparser_freedict(ini); - return ret; + _D("config: crash_root_path = %s\n" + "config: extra_script = %s\n" + "config: report_type = %s\n" + "config: system_max_use = %d\n" + "config: system_keep_free = %d\n" + "config: max_retention_sec = %d\n" + "config: max_crash_dump = %d\n" + "config: dump_core = %d\n" + "config: allow_zip = %d\n", + c->crash_root_path, + c->extra_script, + report_type_to_str(c->report_type), + c->system_max_use, + c->system_keep_free, + c->max_retention_sec, + c->max_crash_dump, + c->dump_core, + c->allow_zip); +} + +bool config_init(config_t *c, const char *const path) +{ + if (!config_apply_defaults(c) || !config_load_from_path(c, path)) { + _E("config: Unable to initialize configuration"); + return false; + } + + config_dump(c); + + return true; } void config_free(config_t *c) -- 2.7.4 From cdca0baf4ce0cda8a866db7e25d544636e4e23ec Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 18 Dec 2019 10:41:09 +0100 Subject: [PATCH 06/16] config: Search for configuration also under crash-manager.conf.d directory Change-Id: I9c5680eafb467a0514feec5225cb794609ba1823 --- packaging/crash-worker.spec | 1 + src/crash-manager/CMakeLists.txt | 5 ++ src/crash-manager/crash-manager.conf.d.example | 3 ++ src/shared/config.c | 63 ++++++++++++++++++++++ .../crash_root_path/crash_root_path.sh.template | 17 ++++-- 5 files changed, 84 insertions(+), 5 deletions(-) create mode 100644 src/crash-manager/crash-manager.conf.d.example diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index 08bcafb..1557f20 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -161,6 +161,7 @@ mkdir -p %{buildroot}%{crash_temp} %dir %{crash_path} %dir %{crash_temp} %{_sysconfdir}/crash-manager.conf +%{_sysconfdir}/crash-manager.conf.d/crash-manager.conf.example %attr(-,root,root) %{_prefix}/lib/sysctl.d/70-crash-manager.conf %attr(0750,crash_worker,crash_worker) %{_bindir}/crash-manager %attr(0750,crash_worker,crash_worker) %{_bindir}/dump_systemstate diff --git a/src/crash-manager/CMakeLists.txt b/src/crash-manager/CMakeLists.txt index 591a3ad..2fdf2a2 100644 --- a/src/crash-manager/CMakeLists.txt +++ b/src/crash-manager/CMakeLists.txt @@ -69,6 +69,11 @@ 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}/crash-manager.conf.d.example + DESTINATION /etc/crash-manager.conf.d/ + PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ + RENAME crash-manager.conf.example) + INSTALL(FILES ${CMAKE_SOURCE_DIR}/src/${PROJECT_NAME}/crash-manager.pc DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ) diff --git a/src/crash-manager/crash-manager.conf.d.example b/src/crash-manager/crash-manager.conf.d.example new file mode 100644 index 0000000..b50dbff --- /dev/null +++ b/src/crash-manager/crash-manager.conf.d.example @@ -0,0 +1,3 @@ +# Rename this file to end with .conf for it to be read by crash-manager +[CrashManager] +DumpCore=0 diff --git a/src/shared/config.c b/src/shared/config.c index 1e728c4..901bab0 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -15,10 +15,14 @@ * limitations under the License. */ #include +#include +#include #include #include #include #include +#include +#include #include "config.h" #include "defs.h" @@ -112,6 +116,63 @@ static bool config_load_from_path(config_t *c, const char *const path) return ret; } +static int entry_filter(const struct dirent *e) +{ + assert(e); + + const char *const conf_suffix = ".conf"; + const char *const name = e->d_name; + int len = strlen(name); + + if (e->d_type != DT_REG || len <= strlen(conf_suffix)) + return 0; + + // accept only files ending with predefined suffix + return strcmp(name + len - strlen(conf_suffix), conf_suffix) == 0; +} + +static bool config_load_from_dir_prefix(config_t *c, const char *const dir_prefix) +{ + assert(dir_prefix); + + char dir_path[PATH_MAX]; + int ret = snprintf(dir_path, sizeof(dir_path), "%s.d", dir_prefix); + if (ret < 0 || ret >= PATH_MAX) { + _W("config: internal error while trying to prepare config dir path"); + return false; + } + + int fd = open(dir_path, O_RDONLY | O_DIRECTORY); + if (fd < 0 && errno == ENOENT) + return true; + if (fd < 0) { + _E("config: Unable to access config directory at %s: %m", dir_path); + return false; + } + + struct dirent **entries = NULL; + int n = scandirat(fd, ".", &entries, entry_filter, alphasort); + if (n < 0) + goto out; + + for (int i = 0; i < n; ++i) { + char file_path[PATH_MAX]; + const char *fname = entries[i]->d_name; + ret = snprintf(file_path, sizeof(file_path), "%s/%s", dir_path, fname); + if (ret < 0 || ret >= PATH_MAX) { + _W("config: internal error while trying to prepare for reading %s config file", fname); + continue; + } + _D("config: reading additional configuration file from %s", file_path); + (void)config_load_from_path(c, file_path); + } + +out: + free(entries); + close(fd); + return true; +} + static bool config_apply_defaults(config_t *c) { assert(c); @@ -161,6 +222,8 @@ bool config_init(config_t *c, const char *const path) return false; } + (void)config_load_from_dir_prefix(c, path); + config_dump(c); return true; 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 0f59b55..7fe5e96 100644 --- a/tests/system/crash_root_path/crash_root_path.sh.template +++ b/tests/system/crash_root_path/crash_root_path.sh.template @@ -8,7 +8,9 @@ fi . ${CRASH_WORKER_SYSTEM_TESTS}/utils/minicore-utils.sh -CRASH_MANAGER_CONF=/etc/crash-manager.conf +CONF_DIR=/etc/crash-manager.conf.d +CONF_FILE=${CONF_DIR}/crash-root-path-test.conf + NEW_PATH=/tmp/crash_path_test/ if [ ! -d ${NEW_PATH} ]; then mkdir ${NEW_PATH} @@ -23,8 +25,13 @@ function check_file_exists { } mount -o rw,remount / -backup_file ${CRASH_MANAGER_CONF} -cat ${CRASH_MANAGER_CONF}.backup | sed "s|#[ ]\+CrashRootPath=.*|CrashRootPath=${NEW_PATH}|" > ${CRASH_MANAGER_CONF} + +mkdir -p $CONF_DIR +rm -f $CONF_FILE || : +cat <$CONF_FILE +[CrashManager] +CrashRootPath=${NEW_PATH} +EOF { ${CRASH_WORKER_SYSTEM_TESTS}/utils/kenny & @@ -34,10 +41,10 @@ cat ${CRASH_MANAGER_CONF}.backup | sed "s|#[ ]\+CrashRootPath=.*|CrashRootPath=$ sleep 2 -restore_file ${CRASH_MANAGER_CONF} - wait_for_app crash-manager +rm -f ${CONF_FILE} || : + pushd ${NEW_PATH}/dump if ! unzip kenny*zip > /dev/null; then popd -- 2.7.4 From 584b89152d0e30db496529eb7da83cb7424882c8 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Fri, 20 Dec 2019 20:32:46 +0100 Subject: [PATCH 07/16] config: Ensure freed pointers are set to null Change-Id: I779cff27a6873187e5f17415469864d6ccd0d1b7 --- src/shared/config.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/shared/config.c b/src/shared/config.c index 901bab0..d389187 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -235,4 +235,6 @@ void config_free(config_t *c) free(c->crash_root_path); free(c->extra_script); + + memset(c, 0, sizeof(*c)); } -- 2.7.4 From 615efb3d6e5051cdc79eec3b30cec88dacbdf4f9 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 18 Dec 2019 15:36:27 +0100 Subject: [PATCH 08/16] Release 6.0.5 This release brings ability to extend configuration via snippets in /etc/crash-manager.conf.d/ directory. Change-Id: Ic01f780a2bb3868e48f02bbe95ac725adcb384cd --- 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 1557f20..9b9a62f 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.4 +Version: 6.0.5 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 e1dc4a0..f0be398 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.4 +Version: 6.0.5 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From 82a4e1e87e0631904804439ff9afa8ece2ef329d Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Fri, 3 Jan 2020 09:16:48 +0100 Subject: [PATCH 09/16] crash-manager: Make permission handling in maps more abstract This also fixes gcc9.2 build error. Change-Id: Iae099a366063cd69cca840bd83121a8930d89088 --- src/crash-manager/so-info.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/crash-manager/so-info.c b/src/crash-manager/so-info.c index a0ae2dc..d95a927 100644 --- a/src/crash-manager/so-info.c +++ b/src/crash-manager/so-info.c @@ -169,12 +169,20 @@ static int get_build_id(char *filename, char **build_id) return ret; } -static int get_perms(char *line, char *perms) +enum Perms { + PERM_READ = 1, + PERM_WRITE = 2, + PERM_EXEC = 4 +}; + +/* Parses beginning of the maps file in the form: `addr1-addr2 r-xp size ...` */ +static int get_perms(char *line, enum Perms *perms) { char *first_space = strchr(line, ' '); - if (first_space > 0) { - strncpy(perms, first_space+1, 4); + if (first_space != 0 && strlen(first_space) > 4 /* eg. " r-xp" from example above */) { + char *p = first_space + 1; + *perms = (p[0] == 'r' ? PERM_READ : 0) | (p[1] == 'w' ? PERM_WRITE : 0) | (p[2] == 'x' ? PERM_EXEC : 0); return 0; } else return -1; @@ -186,9 +194,9 @@ static int get_perms(char *line, char *perms) */ static char *get_exe_filename(char *line) { - char perms[4]; + enum Perms perms; - if (get_perms(line, perms) == 0 && perms[2] == 'x') { + if (get_perms(line, &perms) == 0 && (perms & PERM_EXEC)) { char *p = strstr(line, " /"); if (p > 0) return p+1; -- 2.7.4 From f24c53fb208eca0307ab55d1ae9157118b15ab2a Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Tue, 17 Dec 2019 14:58:15 +0100 Subject: [PATCH 10/16] crash-service: Restrict allow livedump API via privilege Use newly created http://tizen.org/privilege/internal/livecoredump privilege to restrict access to crash-service's livedump dbus interface. Change-Id: I9b2f560c061426d561674e3560b60c10e36c384e --- src/crash-service/crash-service.conf | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/crash-service/crash-service.conf b/src/crash-service/crash-service.conf index 02bd574..7aac3d0 100644 --- a/src/crash-service/crash-service.conf +++ b/src/crash-service/crash-service.conf @@ -13,13 +13,12 @@ send_interface="org.tizen.system.crash.livedump" send_member="livedump_pid"/> - - - + -- 2.7.4 From deb008ea555989885a91b19ecec4829b8301be02 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Tue, 7 Jan 2020 10:24:04 +0100 Subject: [PATCH 11/16] Release 6.0.6 This release brings following changes: - Use designated privilege to limit livedump API. This replaces temporary use of per-used privilege check in dbus policy. - Gcc 9.x build fix Change-Id: I2bad2792af9aefbd210cd1537293823a664d1a8f --- 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 9b9a62f..4376521 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.5 +Version: 6.0.6 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 f0be398..0afd6c9 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.5 +Version: 6.0.6 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From 11f31feaebfab49492c93d7ea0236bd8686030a0 Mon Sep 17 00:00:00 2001 From: Kunhoon Baik Date: Tue, 21 Jan 2020 16:47:59 +0900 Subject: [PATCH 12/16] Add option to support legacy DBus 'ProcessCrashed' signal Original author: Kun-Hoon Baik Reworked by: Karol Lewandowski Change-Id: I64b950b99d81b2a0a4ba4e67938d865431c1a25e --- src/crash-manager/crash-manager.c | 3 +++ src/crash-manager/crash-manager.conf | 14 ++++++++++++++ src/crash-manager/dbus_notify.c | 21 +++++++++++++++++---- src/shared/config.c | 8 ++++++-- src/shared/config.h | 2 ++ 5 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index bd402a1..753d1de 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -533,6 +533,8 @@ static void launch_dbus_notify(struct crash_info *cinfo) SNPRINTF_OR_EXIT(tid, "%d"); SNPRINTF_OR_EXIT(sig, "%d"); + char *legacy_notification_str = config.legacy_notification ? "--legacy-sig" : ""; + char *av[] = { CRASH_NOTIFY_BIN_PATH, "--cmdline", cinfo->cmd_line, "--cmdpath", cinfo->cmd_path, @@ -544,6 +546,7 @@ static void launch_dbus_notify(struct crash_info *cinfo) "--prstatus_fd", prstatus_fd_str, "--signal", sig_str, "--tid-comm", tid_comm_str, + legacy_notification_str, NULL }; spawn(av, NULL, NULL, NULL, NULL); diff --git a/src/crash-manager/crash-manager.conf b/src/crash-manager/crash-manager.conf index c485527..acaca35 100644 --- a/src/crash-manager/crash-manager.conf +++ b/src/crash-manager/crash-manager.conf @@ -15,6 +15,20 @@ AllowZip=yes # This option applies to ReportType=FULL only! # DumpCore=1 +# Use Legacy ProcessCrashed DBus signal +# When '0' (default), crash worker emits new-style signal with extensive data: +# 1. cmd name, +# 2. cmd path, +# 3. appid, +# 4. pkgid, +# 5. report path, +# 6. pid, +# 7. tid, +# 8. array of str->variant with additional data +# When '1' (legacy), signal signature is limited to fields 1-4 (cmd name, path, app/pkgid). +# The purpose of this is option is to support software still depending on legacy style. +# UseLegacyNotification=0 + # Extra script to run whose output will be attached to generated report # Script will be called with two arguments, in pseudo-code: # sh -c "$ExtraScript /path/to/report/temp/directory PID" diff --git a/src/crash-manager/dbus_notify.c b/src/crash-manager/dbus_notify.c index 0254092..41126c5 100644 --- a/src/crash-manager/dbus_notify.c +++ b/src/crash-manager/dbus_notify.c @@ -51,6 +51,7 @@ struct RegInfo { }; struct NotifyParams { + bool legacy_notification; int prstatus_fd; pid_t pid; pid_t tid; @@ -148,10 +149,16 @@ strdup_error: goto out; } +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) { - if (notify_params == NULL) - return NULL; + assert(notify_params); GVariantBuilder md_builder; g_variant_builder_init(&md_builder, G_VARIANT_TYPE("(sssssiia{sv})")); @@ -209,7 +216,9 @@ static bool send_notify(GDBusConnection *conn, const struct NotifyParams *notify int result = false; GError *error = NULL; - GVariant *data = build_message_data(notify_params); + GVariant *data = notify_params->legacy_notification + ? build_legacy_message_data(notify_params) + : build_message_data(notify_params); if (data == NULL) { _E("Error while preparing parameters"); goto out; @@ -256,6 +265,7 @@ static bool parse_cmdline(int ac, char *av[], struct NotifyParams *params) FLAG_PRSTATUS_FD, FLAG_SIGNAL, FLAG_TID_COMM, + FLAG_LEGACY_NOTIFICATION, }; static const struct option options[] = { { .name = "cmdline", .has_arg = required_argument, .flag = NULL, .val = FLAG_CMDLINE }, @@ -268,6 +278,7 @@ static bool parse_cmdline(int ac, char *av[], struct NotifyParams *params) { .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 }, + { .name = "legacy-sig", .has_arg = no_argument, .flag = NULL, .val = FLAG_LEGACY_NOTIFICATION }, { NULL }, }; @@ -295,6 +306,8 @@ static bool parse_cmdline(int ac, char *av[], struct NotifyParams *params) params->signal = atoi(optarg); else if (FLAG_TID_COMM == val) params->tid_comm = optarg; + else if (FLAG_LEGACY_NOTIFICATION == val) + params->legacy_notification = true; } while (val != -1); return params->cmd_name && params->cmd_path && params->appid && params->pkgid && params->prstatus_fd > 0; @@ -304,7 +317,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 --signal SIGNALNR\n", progname); + printf("%s --prstatus_fd N --pid PID --tid TID --cmdline CMDLINE --cmdpath CMDPATH --appid APPID --pkgid PKGID --signal SIGNALNR [--legacy-sig]\n", progname); } int main(int ac, char *av[]) diff --git a/src/shared/config.c b/src/shared/config.c index d389187..cbf796c 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -94,6 +94,7 @@ static bool config_load_from_dict(config_t *c, dictionary *ini) UPDATE(c->max_crash_dump, int, "MaxCrashDump"); UPDATE(c->dump_core, boolean, "DumpCore"); UPDATE(c->allow_zip, boolean, "AllowZip"); + UPDATE(c->legacy_notification, boolean, "UseLegacyNotification"); #undef UPDATE @@ -187,6 +188,7 @@ static bool config_apply_defaults(config_t *c) c->max_crash_dump = MAX_CRASH_DUMP; c->dump_core = DUMP_CORE; c->allow_zip = ALLOW_ZIP; + c->legacy_notification = LEGACY_NOTIFICATION; return c->crash_root_path != NULL; } @@ -203,7 +205,8 @@ static void config_dump(config_t *c) "config: max_retention_sec = %d\n" "config: max_crash_dump = %d\n" "config: dump_core = %d\n" - "config: allow_zip = %d\n", + "config: allow_zip = %d\n" + "config: legacy_notification = %d\n", c->crash_root_path, c->extra_script, report_type_to_str(c->report_type), @@ -212,7 +215,8 @@ static void config_dump(config_t *c) c->max_retention_sec, c->max_crash_dump, c->dump_core, - c->allow_zip); + c->allow_zip, + c->legacy_notification); } bool config_init(config_t *c, const char *const path) diff --git a/src/shared/config.h b/src/shared/config.h index aea42ea..9343300 100644 --- a/src/shared/config.h +++ b/src/shared/config.h @@ -27,6 +27,7 @@ #define MAX_CRASH_DUMP 0 #define DUMP_CORE 1 #define ALLOW_ZIP 1 +#define LEGACY_NOTIFICATION 0 #define CRASH_SECTION "CrashManager" @@ -40,6 +41,7 @@ enum ReportType { typedef struct config { bool allow_zip; bool dump_core; + bool legacy_notification; int system_max_use; int system_keep_free; int max_retention_sec; -- 2.7.4 From bb69705b4327661a6d9747bc5776c72bf6c79e15 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Tue, 21 Jan 2020 12:25:41 +0100 Subject: [PATCH 13/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 14/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 15/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 16/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"/> + + +