Remove unsupported and misleading configuration options 37/187337/4
authorKarol Lewandowski <k.lewandowsk@samsung.com>
Wed, 22 Aug 2018 06:50:42 +0000 (08:50 +0200)
committerKarol Lewandowski <k.lewandowsk@samsung.com>
Wed, 22 Aug 2018 10:55:01 +0000 (12:55 +0200)
 - Remove TIZEN_FEATURE_PTRACE_CALLSTACK option

   Crash report generation method has been reworked
   and does not depend on ptrace anymore.

 - Remove TIZEN_ENABLE_MINICOREDUMPER option

   Minicoredumper is required to dump PRSTATUS, which
   is later used by crash-manager (for D-Bus signal)
   and by crash-stack (for callstack unwinding)

 - Remove TIZEN_ENABLE_COREDUMP option

   crash-pipe does not dump PRSTATUS so can not be
   used as an option (see above).

Consequently, enabling/disabling coredumps has been
moved to configuration file (DumpCore=0/1).

Change-Id: If535acfe7e64fdde0a8b50afd8b3a1f44fda2490

CMakeLists.txt
packaging/crash-worker.spec
src/crash-manager/CMakeLists.txt
src/crash-manager/crash-manager.c
src/crash-manager/crash-manager.conf
src/crash-manager/crash-manager.h.in
src/crash-pipe/CMakeLists.txt [deleted file]
src/crash-pipe/crash-pipe.c [deleted file]

index f6e47cb..96ecea7 100644 (file)
@@ -10,13 +10,7 @@ IF("${SYS_ASSERT}" STREQUAL "ON")
        ADD_SUBDIRECTORY(src/sys-assert)
 ENDIF("${SYS_ASSERT}" STREQUAL "ON")
 
-IF(TIZEN_ENABLE_COREDUMP STREQUAL on)
-       ADD_SUBDIRECTORY(src/crash-pipe)
-ENDIF()
-
-IF(TIZEN_FEATURE_PTRACE_CALLSTACK STREQUAL on)
-       ADD_SUBDIRECTORY(src/crash-stack)
-ENDIF()
+ADD_SUBDIRECTORY(src/crash-stack)
 
 ADD_SUBDIRECTORY(src/dump_systemstate)
 ADD_SUBDIRECTORY(src/log_dump)
index 958f558..aebc135 100644 (file)
@@ -2,12 +2,12 @@
 %define on_off() %{expand:%%{?with_%{1}:ON}%%{!?with_%{1}:OFF}}
 
 %define _with_tests on
-%define TIZEN_FEATURE_PTRACE_CALLSTACK on
-%define TIZEN_FEATURE_MINICOREDUMPER on
 %bcond_with doc
 %bcond_with sys_assert
 %bcond_with tests
 
+# NOTE: To disable coredump set DumpCore=0 in configuration file
+
 Name:      crash-worker
 Summary:    Crash-manager
 Version:    1.2.1
@@ -26,11 +26,9 @@ BuildRequires:  cmake
 BuildRequires:  pkgconfig(pkgmgr-info)
 
 BuildRequires:  pkgconfig(libunwind-generic)
-%if "%{TIZEN_FEATURE_PTRACE_CALLSTACK}" == "on"
 BuildRequires:  libelf-devel libelf
 BuildRequires:  libebl-devel libebl
 BuildRequires:  libdw-devel libdw
-%endif
 
 %if %{with doc}
 BuildRequires:  doxygen
@@ -42,13 +40,7 @@ Requires(post): gzip
 Requires: zip
 Requires: libelf
 Requires: libdw
-
-%if "%{TIZEN_FEATURE_MINICOREDUMPER}" == "on"
 Requires: %{_sbindir}/minicoredumper
-%endif
-# If you need support for core dump files (see building below),
-# you should add this dependency
-# Requires: libebl
 
 %description
 crash-manager
@@ -115,21 +107,12 @@ export CFLAGS+=" -Werror"
           -DCRASH_PATH=%{crash_path} \
           -DCRASH_TEMP=%{crash_temp} \
           -DDEBUGMODE_PATH=%{debugmode_path} \
-%if "%{TIZEN_FEATURE_MINICOREDUMPER}" == "on"
-          -DTIZEN_ENABLE_MINICOREDUMP=on \
           -DMINICOREDUMPER_PATH=%{_sbindir}/minicoredumper \
           -DMINICOREDUMPER_CONF_DIR=%{_sysconfdir}/minicoredumper \
-%else
-          -DTIZEN_ENABLE_COREDUMP=on \
-          -DCRASH_PIPE_PATH=%{_libexecdir}/crash-pipe \
-%endif
-%if "%{TIZEN_FEATURE_PTRACE_CALLSTACK}" == "on"
           -DCRASH_STACK_PATH=%{_libexecdir}/crash-stack \
-%endif
           -DCRASH_TESTS_PATH=%{_libdir}/crash-worker-tests \
           -DSYS_ASSERT=%{on_off sys_assert} \
           -DUPGRADE_SCRIPT_PATH=%{upgrade_script_path} \
-          -DTIZEN_FEATURE_PTRACE_CALLSTACK=%{TIZEN_FEATURE_PTRACE_CALLSTACK}
 
 make %{?jobs:-j%jobs}
 %if %{with doc}
@@ -194,19 +177,12 @@ sed -i "/${pattern}/D" %{_sysconfdir}/ld.so.preload
 %attr(-,root,root) %{_sysconfdir}/dbus-1/system.d/log_dump.conf
 %attr(-,root,root) %{_prefix}/lib/sysctl.d/99-crash-manager.conf
 %attr(-,root,root) %{_datadir}/dbus-1/system-services/org.tizen.system.crash.service
-
+%{_libexecdir}/crash-stack
 %if %{with sys_assert}
 %{_libdir}/libsys-assert.so
 %{_sysconfdir}/tmpfiles.d/sys-assert.conf
 %endif
 
-%if "%{TIZEN_FEATURE_MINICOREDUMPER}" != "on"
-%{_libexecdir}/crash-pipe
-%endif
-%if "%{TIZEN_FEATURE_PTRACE_CALLSTACK}" == "on"
-%{_libexecdir}/crash-stack
-%endif
-
 #upgrade script
 %attr(-,root,root) %{upgrade_script_path}/500.crash-manager-upgrade.sh
 
index 2b7e6e4..1ad1f93 100644 (file)
@@ -1,18 +1,6 @@
 CMAKE_MINIMUM_REQUIRED(VERSION 2.6)
 PROJECT(crash-manager C)
 
-IF(TIZEN_FEATURE_PTRACE_CALLSTACK STREQUAL on)
-       ADD_DEFINITIONS(-DTIZEN_FEATURE_PTRACE_CALLSTACK)
-ENDIF()
-
-IF(TIZEN_ENABLE_COREDUMP STREQUAL on)
-        ADD_DEFINITIONS(-DTIZEN_ENABLE_COREDUMP)
-ENDIF()
-
-IF(TIZEN_ENABLE_MINICOREDUMP STREQUAL on)
-        ADD_DEFINITIONS(-DTIZEN_ENABLE_MINICOREDUMP)
-ENDIF()
-
 INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/src)
 SET(CRASH_MANAGER_SRCS
        crash-manager.c
@@ -37,14 +25,6 @@ ENDFOREACH(flag)
 
 SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${EXTRA_CFLAGS} -fPIE")
 
-IF(TIZEN_ENABLE_COREDUMP AND TIZEN_ENABLE_MINICOREDUMP)
-        message(FATAL_ERROR "You can not define TIZEN_ENABLE_COREDUMP and TIZEN_ENABLE_MINICOREDUMP at once.")
-ENDIF()
-
-IF(TIZEN_ENABLE_COREDUMP OR TIZEN_ENABLE_MINICOREDUMP)
-        OPTION(USE_COREDUMP_CONF "Use COREDUMP CONF" ON)
-ENDIF()
-
 CONFIGURE_FILE(crash-manager.h.in crash-manager.h @ONLY)
 ADD_EXECUTABLE(${PROJECT_NAME} ${CRASH_MANAGER_SRCS})
 TARGET_LINK_LIBRARIES(${PROJECT_NAME} ${crash-manager_pkgs_LDFLAGS} -pie -lrt)
index 6ee58f8..c42f261 100644 (file)
@@ -32,6 +32,7 @@
 #include <sys/prctl.h>
 #include <sys/file.h>
 #include <sys/vfs.h>
+#include <fcntl.h>
 #include <gio/gio.h>
 #include <iniparser.h>
 #include <tzplatform_config.h>
@@ -63,6 +64,7 @@
 #define SYSTEM_KEEP_FREE     0
 #define MAX_RETENTION_SEC    0
 #define MAX_CRASH_DUMP       0
+#define DUMP_CORE            1
 #define ALLOW_ZIP            true
 
 #define APPID_MAX                   128
@@ -104,6 +106,7 @@ static int system_max_use;
 static int system_keep_free;
 static int max_retention_sec;
 static int max_crash_dump;
+static int dump_core;
 static bool allow_zip;
 static char* crash_root_path;
 static char* crash_crash_path;
@@ -280,6 +283,7 @@ static int get_config(void)
        system_keep_free = SYSTEM_KEEP_FREE;
        max_retention_sec = MAX_RETENTION_SEC;
        max_crash_dump = MAX_CRASH_DUMP;
+       dump_core = DUMP_CORE;
        allow_zip = ALLOW_ZIP;
        crash_root_path = strdup(CRASH_ROOT_PATH);
        if (crash_root_path == NULL) {
@@ -335,6 +339,16 @@ static int get_config(void)
                max_crash_dump = value;
        }
 
+       snprintf(key, sizeof(key), "%s:%s", CRASH_SECTION, "DumpCore");
+       value = iniparser_getint(ini, key, -1);
+       if (value != 0 && value != 1) {
+               _D("Invalid value for DumpCore default value [ %d ]",
+                               DUMP_CORE);
+       } else {
+               _D("DumpCore [ %d ]", value);
+               dump_core = value;
+       }
+
        snprintf(key, sizeof(key), "%s:%s", CRASH_SECTION, "AllowZip");
        value = iniparser_getboolean(ini, key, -1);
        if (value < 0) {
@@ -658,7 +672,6 @@ static int get_sysassert_cs(void)
 }
 #endif
 
-#if defined(TIZEN_ENABLE_COREDUMP) || defined(TIZEN_ENABLE_MINICOREDUMP)
 static void launch_crash_popup(void)
 {
        GDBusConnection *conn;
@@ -709,7 +722,6 @@ exit:
        if (parameters)
                g_variant_unref(parameters);
 }
-#endif
 
 static int dump_system_state(void)
 {
@@ -763,29 +775,6 @@ static void save_so_info()
        get_and_save_so_info(maps_path, so_info_path);
 }
 
-#ifdef TIZEN_ENABLE_COREDUMP
-static void execute_crash_pipe(int argc, char *argv[])
-{
-       int ret;
-       char command[PATH_MAX];
-
-       /* Execute crash-pipe */
-       ret = snprintf(command, sizeof(command),
-                       "%s --save-core %s",
-                       CRASH_PIPE_PATH,
-                       crash_info.core_path);
-
-       _D("    %s", command);
-
-       if (ret < 0) {
-               _E("Failed to snprintf for crash-pipe command");
-               return;
-       }
-       system_command(command);
-}
-#endif /* TIZEN_ENABLE_COREDUMP */
-
-#ifdef TIZEN_ENABLE_MINICOREDUMP
 static void execute_minicoredump(int argc, char *argv[])
 {
        char *coredump_name = NULL;
@@ -825,12 +814,25 @@ static void execute_minicoredump(int argc, char *argv[])
 
        run_command_timeout(args[0], args, NULL, MINICOREDUMPER_TIMEOUT);
 
+       /* Minicoredumper must be executed to dump at least PRSTATUS for
+          other tools, coredump, however, might have been disabled. */
+       if (!dump_core) {
+               int ret = -1;
+               int errno_unlink = 0;
+               int dirfd = open(crash_info.pfx, O_DIRECTORY);
+               if (dirfd != -1) {
+                       ret = unlinkat(dirfd, coredump_name, 0);
+                       errno_unlink = errno;
+                       close(dirfd);
+               }
+               _D("Saving core disabled - removing coredump %s/%s: %s", crash_info.pfx, coredump_name,
+                  ret == 0 ? "success" : strerror(errno_unlink));
+       }
+
        free(coredump_name);
        free(prstatus_fd_str);
 }
-#endif /* TIZEN_ENABLE_MINICOREDUMP */
 
-#ifdef TIZEN_FEATURE_PTRACE_CALLSTACK
 static void execute_crash_stack(int argc, char *argv[])
 {
        int ret;
@@ -862,31 +864,20 @@ static void execute_crash_stack(int argc, char *argv[])
 
        system_command(command);
 }
-#endif /* TIZEN_FEATURE_PTRACE_CALLSTACK */
 
 static void execute_crash_modules(int argc, char *argv[])
 {
        _D("Execute crash module: ");
 
-#ifdef TIZEN_ENABLE_COREDUMP
-       if (report_type >= REP_TYPE_FULL)
-               execute_crash_pipe(argc, argv);
-#endif
-
-#ifdef TIZEN_ENABLE_MINICOREDUMP
        execute_minicoredump(argc, argv);
-#endif
 
-#ifdef TIZEN_FEATURE_PTRACE_CALLSTACK
-
-# ifdef SYS_ASSERT
+#ifdef SYS_ASSERT
        /* Use process_vm_readv() version as fallback if sys-assert
         * failed to generate report */
        if (crash_info.have_sysassert_report)
                return;
-# endif
+#endif
        execute_crash_stack(argc, argv);
-#endif /* TIZEN_FEATURE_PTRACE_CALLSTACK */
 }
 
 static int lock_dumpdir(void)
@@ -1217,9 +1208,7 @@ int main(int argc, char *argv[])
 {
        /* Execute dump_systemstate in parallel */
        static int dump_state_pid;
-#if defined(TIZEN_ENABLE_COREDUMP) || defined(TIZEN_ENABLE_MINICOREDUMP)
        int debug_mode = access(DEBUGMODE_PATH, F_OK) == 0;
-#endif
        int res = 0;
 
        /*
@@ -1288,11 +1277,9 @@ int main(int argc, char *argv[])
        } else {
                move_info_file();
        }
-#if defined(TIZEN_ENABLE_COREDUMP) || defined(TIZEN_ENABLE_MINICOREDUMP)
        /* launch crash-popup only if the .debugmode file is exist*/
        if (debug_mode)
                launch_crash_popup();
-#endif
 
        struct NotifyParams notify_params = {
                .prstatus_fd = crash_info.prstatus_fd,
index 98e8690..82f5b91 100644 (file)
@@ -10,3 +10,7 @@ AllowZip=yes
 
 # Report type can be FULL (.zip) or INFO (.info only)
 # ReportType=FULL
+
+# Dump coredump - 1 to enable (default), 0 to disable
+# This option applies to ReportType=FULL only!
+# DumpCore=1
index 9206737..149a056 100644 (file)
 #define CRASH_ROOT_PATH  "@CRASH_ROOT_PATH@"
 #define CRASH_TEMP       "@CRASH_TEMP@"
 #define SYS_ASSERT       "@SYS_ASSERT@"
-#ifdef TIZEN_FEATURE_PTRACE_CALLSTACK
 #define CRASH_STACK_PATH "@CRASH_STACK_PATH@"
-#endif
-#define CRASH_PIPE_PATH  "@CRASH_PIPE_PATH@"
 #define MINICOREDUMPER_PATH  "@MINICOREDUMPER_PATH@"
 #define MINICOREDUMPER_CONF_DIR  "@MINICOREDUMPER_CONF_DIR@"
 #define DEBUGMODE_PATH   "@DEBUGMODE_PATH@"
diff --git a/src/crash-pipe/CMakeLists.txt b/src/crash-pipe/CMakeLists.txt
deleted file mode 100644 (file)
index 3a63f89..0000000
+++ /dev/null
@@ -1,7 +0,0 @@
-set(CRASH_PIPE_BIN "crash-pipe")
-set(CRASH_PIPE_SRCS crash-pipe.c)
-set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -fPIE")
-set(CMAKE_EXE_LINKER_FLAGS "-Wl,--as-needed -pie")
-
-add_executable(${CRASH_PIPE_BIN} ${CRASH_PIPE_SRCS})
-install(TARGETS ${CRASH_PIPE_BIN} DESTINATION libexec)
diff --git a/src/crash-pipe/crash-pipe.c b/src/crash-pipe/crash-pipe.c
deleted file mode 100644 (file)
index 53c0ac0..0000000
+++ /dev/null
@@ -1,162 +0,0 @@
-/* crash-pipe: handle core file passed from stdin
- *
- * Copyright (c) 2016 Samsung Electronics Co., Ltd.
- *
- * Licensed under the Apache License, Version 2.0 (the License);
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- * Authors: Karol Lewandowski <k.lewandowsk@samsung.com>
- *          Ćukasz Stelmach <l.stelmach@samsung.com>
- */
-#define _GNU_SOURCE 1
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <errno.h>
-
-#include <syslog.h>
-#include <sys/prctl.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <getopt.h>
-#include <limits.h>
-
-enum {
-       OPT_HELP,
-       OPT_SAVE_CORE,
-};
-
-const struct option opts[] = {
-       { "help", no_argument, 0, OPT_HELP },
-       { "save-core", required_argument, 0, OPT_SAVE_CORE },
-       { 0, 0, 0, 0 }
-};
-
-static char *argv0 = "";
-
-static void usage(void)
-{
-       fprintf(stderr, "usage: %s [--help] --save-core FILE_NAME\n", argv0);
-}
-
-static int copy_fd_simple(int in, int out)
-{
-       static char buf[4096];
-       int readb, remaining;
-
-       while ((readb = read(in, buf, sizeof(buf))) > 0) {
-               int n;
-
-               for (n = 0, remaining = readb ; remaining > 0; remaining -= n) {
-                       n = write(out, buf, remaining);
-                       if (n == -1)
-                               return -errno;
-               }
-       }
-
-       return 1;
-}
-
-static int copy_fd_splice(int in, int out)
-{
-       ssize_t s;
-       _Bool copied_data = 0;
-
-       ssize_t max_splice = SSIZE_MAX;
-
-       do {
-               s = splice(in, NULL, out, NULL, max_splice, SPLICE_F_MOVE);
-               // output file position + len must not exceed the loff_t type limit,
-               // that's why we reduce len in every iteration
-               max_splice -= s;
-
-               // We can try to fallback to simple copy only if first splice failed
-               // (ie. we have not consumed any data yet)
-               if (!copied_data && s > 0)
-                       copied_data = 1;
-
-       } while (s > 0);
-
-       if (s < 0)
-               return copied_data ? -errno : 0;
-       else
-               return 1;
-}
-
-static int save_core(const char *core_path)
-{
-       int fd;
-       int r = 0;
-
-       fd = open(core_path, O_WRONLY | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR);
-       if (fd == -1) {
-               syslog(LOG_ERR, "crash-pipe: Unable to save core file to %s: %m\n", core_path);
-               return -errno;
-       }
-
-       r = copy_fd_splice(STDIN_FILENO, fd);
-       if (r == 0) {
-               lseek(fd, SEEK_SET, 0);
-               r = copy_fd_simple(STDIN_FILENO, fd);
-       }
-       if (r < 0) {
-               syslog(LOG_ERR, "crash-pipe: Error while saving core file %s: %m. Removing core.\n", core_path);
-               if (unlink(core_path) < 0) {
-                       // ignore errno because we're already dealing with the one from splice
-                       // amp the log level to CRIT because things are starting to look pretty grim
-                       syslog(LOG_CRIT, "crash-pipe: Error while removing core file %s: %m.\n", core_path);
-               }
-       }
-
-       close(fd);
-       return r;
-}
-
-int main(int argc, char *argv[])
-{
-       int c;
-       char *opt_save_core = NULL;
-       int ret = 1;
-
-       argv0 = argv[0];
-
-       while ((c = getopt_long_only(argc, argv, "", opts, NULL)) != -1) {
-
-               if (c == OPT_HELP) {
-                       usage();
-                       exit(EXIT_SUCCESS);
-               } else if (c == OPT_SAVE_CORE) {
-                       if (opt_save_core) {
-                               syslog(LOG_WARNING, "crash-pipe: duplicate --save-core passed\n");
-                               free(opt_save_core);
-                       }
-                       opt_save_core = strdup(optarg);
-                       if (!opt_save_core) {
-                               syslog(LOG_CRIT, "Out of memory. Exiting.");
-                               exit(EXIT_FAILURE);
-                       }
-               }
-       }
-
-       argc -= optind;
-       argv += optind;
-
-       if (opt_save_core) {
-               ret = save_core(opt_save_core);
-               free(opt_save_core);
-       }
-
-       return ret >= 0 ? EXIT_SUCCESS : EXIT_FAILURE;
-}