From e9f85bed0125cfac9ac76f0ffbd6ce5325368b9c Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Thu, 16 Jun 2011 19:33:56 +0200 Subject: abrtd: remove DeleteDebugDump dbus call text data bss dec hex filename 23084 872 44 24000 5dc0 abrtd 21501 840 44 22385 5771 abrtd.new Signed-off-by: Denys Vlasenko --- src/daemon/CommLayerServerDBus.c | 50 +------ src/daemon/Makefile.am | 1 - src/daemon/MiddleWare.c | 288 --------------------------------------- src/daemon/MiddleWare.h | 61 --------- src/daemon/abrtd.c | 227 +++++++++++++++++++++++++++++- 5 files changed, 223 insertions(+), 404 deletions(-) delete mode 100644 src/daemon/MiddleWare.c delete mode 100644 src/daemon/MiddleWare.h diff --git a/src/daemon/CommLayerServerDBus.c b/src/daemon/CommLayerServerDBus.c index 28a4002d..8311dc69 100644 --- a/src/daemon/CommLayerServerDBus.c +++ b/src/daemon/CommLayerServerDBus.c @@ -20,7 +20,6 @@ #include "abrtlib.h" #include "abrt_dbus.h" #include "comm_layer_inner.h" -#include "MiddleWare.h" #include "CommLayerServerDBus.h" /* @@ -114,51 +113,6 @@ void send_dbus_sig_Warning(const char* pMessage, const char* peer) } -/* - * DBus call handlers - */ - -static long get_remote_uid(DBusMessage* call, const char** ppSender) -{ - DBusError err; - dbus_error_init(&err); - const char* sender = dbus_message_get_sender(call); - if (ppSender) - *ppSender = sender; - long uid = dbus_bus_get_unix_user(g_dbus_conn, sender, &err); - if (dbus_error_is_set(&err)) - { - dbus_error_free(&err); - error_msg("Can't determine remote uid, assuming 0"); - return 0; - } - return uid; -} - -static int handle_DeleteDebugDump(DBusMessage* call, DBusMessage* reply) -{ - int r; - DBusMessageIter in_iter; - dbus_message_iter_init(call, &in_iter); - const char* crash_id; - r = load_charp(&in_iter, &crash_id); - if (r != ABRT_DBUS_LAST_FIELD) - { - error_msg("dbus call %s: parameter type mismatch", __func__ + 7); - return -1; - } - - long unix_uid = get_remote_uid(call, NULL); - int32_t result = DeleteDebugDump(crash_id, unix_uid); - - DBusMessageIter out_iter; - dbus_message_iter_init_append(reply, &out_iter); - store_int32(&out_iter, result); - - send_flush_and_unref(reply); - return 0; -} - /* * Glib integration machinery */ @@ -173,8 +127,8 @@ static DBusHandlerResult message_received(DBusConnection* conn, DBusMessage* msg DBusMessage* reply = dbus_message_new_method_return(msg); int r = -1; - if (strcmp(member, "DeleteDebugDump") == 0) - r = handle_DeleteDebugDump(msg, reply); + /*if (strcmp(member, "DeleteDebugDump") == 0) + r = handle_DeleteDebugDump(msg, reply);*/ // NB: C++ binding also handles "Introspect" method, which returns a string. // It was sending "dummy" introspection answer whick looks like this: diff --git a/src/daemon/Makefile.am b/src/daemon/Makefile.am index 426e6dcb..9f4db016 100644 --- a/src/daemon/Makefile.am +++ b/src/daemon/Makefile.am @@ -14,7 +14,6 @@ sbin_PROGRAMS = \ abrt-server abrtd_SOURCES = \ - MiddleWare.h MiddleWare.c \ CommLayerServerDBus.h CommLayerServerDBus.c \ comm_layer_inner.h comm_layer_inner.c \ abrtd.c diff --git a/src/daemon/MiddleWare.c b/src/daemon/MiddleWare.c deleted file mode 100644 index 959d81ee..00000000 --- a/src/daemon/MiddleWare.c +++ /dev/null @@ -1,288 +0,0 @@ -/* - MiddleWare.cpp - - Copyright (C) 2009 Zdenek Prikryl (zprikryl@redhat.com) - Copyright (C) 2009 RedHat inc. - - This program is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 2 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License along - with this program; if not, write to the Free Software Foundation, Inc., - 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -*/ -#include "abrtlib.h" -#include "comm_layer_inner.h" -#include "CommLayerServerDBus.h" -#include "MiddleWare.h" - -static problem_data_t *FillCrashInfo(const char *dump_dir_name); - - -struct logging_state { - char *last_line; -}; - -static char *do_log_and_save_line(char *log_line, void *param) -{ - struct logging_state *l_state = (struct logging_state *)param; - - VERB1 log("%s", log_line); - update_client("%s", log_line); - free(l_state->last_line); - l_state->last_line = log_line; - return NULL; -} - - -/* We need to share some data between LoadDebugDump and is_crash_a_dup: */ -struct cdump_state { - char *uid; /* filled by LoadDebugDump */ - char *uuid; /* filled by is_crash_a_dup */ - char *crash_dump_dup_name; /* filled by is_crash_a_dup */ -}; - -static int is_crash_a_dup(const char *dump_dir_name, void *param) -{ - struct cdump_state *state = (struct cdump_state *)param; - - if (state->uuid) - return 0; /* we already checked it, don't do it again */ - - struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ 0); - if (!dd) - return 0; /* wtf? (error, but will be handled elsewhere later) */ - state->uuid = dd_load_text_ext(dd, FILENAME_UUID, - DD_FAIL_QUIETLY_ENOENT + DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE - ); - dd_close(dd); - if (!state->uuid) - { - return 0; /* no uuid (yet), "run_event, please continue iterating" */ - } - - /* Scan crash dumps looking for a dup */ -//TODO: explain why this is safe wrt concurrent runs - DIR *dir = opendir(DEBUG_DUMPS_DIR); - if (dir != NULL) - { - struct dirent *dent; - while ((dent = readdir(dir)) != NULL) - { - if (dot_or_dotdot(dent->d_name)) - continue; /* skip "." and ".." */ - - int different; - char *uid, *uuid; - char *dump_dir_name2 = concat_path_file(DEBUG_DUMPS_DIR, dent->d_name); - - if (strcmp(dump_dir_name, dump_dir_name2) == 0) - goto next; /* we are never a dup of ourself */ - - dd = dd_opendir(dump_dir_name2, /*flags:*/ DD_FAIL_QUIETLY_ENOENT); - if (!dd) - goto next; - uid = dd_load_text(dd, FILENAME_UID); - uuid = dd_load_text(dd, FILENAME_UUID); - dd_close(dd); - different = strcmp(state->uid, uid) || strcmp(state->uuid, uuid); - free(uid); - free(uuid); - if (different) - goto next; - - state->crash_dump_dup_name = dump_dir_name2; - /* "run_event, please stop iterating": */ - return 1; - - next: - free(dump_dir_name2); - } - closedir(dir); - } - - /* No dup found */ - return 0; /* "run_event, please continue iterating" */ -} - -static char *do_log(char *log_line, void *param) -{ - VERB1 log("%s", log_line); - //update_client("%s", log_line); - return log_line; -} - -mw_result_t LoadDebugDump(const char *dump_dir_name, problem_data_t **problem_data) -{ - mw_result_t res; - - struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ 0); - if (!dd) - return MW_ERROR; - struct cdump_state state; - state.uid = dd_load_text(dd, FILENAME_UID); - state.uuid = NULL; - state.crash_dump_dup_name = NULL; - char *analyzer = dd_load_text(dd, FILENAME_ANALYZER); - dd_close(dd); - - res = MW_ERROR; - - /* Run post-create event handler(s) */ - struct run_event_state *run_state = new_run_event_state(); - run_state->post_run_callback = is_crash_a_dup; - run_state->post_run_param = &state; - run_state->logging_callback = do_log; - int r = run_event_on_dir_name(run_state, dump_dir_name, "post-create"); - free_run_event_state(run_state); - -//TODO: consider this case: -// new dump is created, post-create detects that it is a dup, -// but then FillCrashInfo(dup_name) *FAILS*. -// In this case, we later delete damaged dup_name (right?) -// but new dump never gets its FILENAME_COUNT set! - - /* Is crash a dup? (In this case, is_crash_a_dup() should have - * aborted "post-create" event processing as soon as it saw uuid - * and determined that there is another crash with same uuid. - * In this case it sets state.crash_dump_dup_name) - */ - if (!state.crash_dump_dup_name) - { - /* No. Was there error on one of processing steps in run_event? */ - if (r != 0) - goto ret; /* yes */ - - /* Was uuid created after all? (In this case, is_crash_a_dup() - * should have fetched it and created state.uuid) - */ - if (!state.uuid) - { - /* no */ - log("Dump directory '%s' has no UUID element", dump_dir_name); - goto ret; - } - } - else - { - dump_dir_name = state.crash_dump_dup_name; - } - - /* Loads problem_data (from the *first debugdump dir* if this one is a dup) - * Returns: - * MW_OCCURRED: "crash count is != 1" (iow: it is > 1 - dup) - * MW_OK: "crash count is 1" (iow: this is a new crash, not a dup) - * else: an error code - */ - { - dd = dd_opendir(dump_dir_name, /*flags:*/ 0); - if (!dd) - { - res = MW_ERROR; - goto ret; - } - - /* Reset mode/uig/gid to correct values for all files created by event run */ - dd_sanitize_mode_and_owner(dd); - - /* Update count */ - char *count_str = dd_load_text_ext(dd, FILENAME_COUNT, DD_FAIL_QUIETLY_ENOENT); - unsigned long count = strtoul(count_str, NULL, 10); - count++; - char new_count_str[sizeof(long)*3 + 2]; - sprintf(new_count_str, "%lu", count); - dd_save_text(dd, FILENAME_COUNT, new_count_str); - dd_close(dd); - - *problem_data = FillCrashInfo(dump_dir_name); - if (*problem_data != NULL) - { - res = MW_OK; - if (count > 1) - { - log("Dump directory is a duplicate of %s", dump_dir_name); - res = MW_OCCURRED; - } - } - } - - ret: - free(state.uuid); - free(state.uid); - free(state.crash_dump_dup_name); - free(analyzer); - - return res; -} - -static problem_data_t *FillCrashInfo(const char *dump_dir_name) -{ - struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ 0); - if (!dd) - return NULL; - - problem_data_t *problem_data = create_problem_data_from_dump_dir(dd); -//Not needed anymore? -// char *events = list_possible_events(dd, NULL, ""); - dd_close(dd); -// -// add_to_problem_data_ext(problem_data, CD_EVENTS, events, -// CD_FLAG_TXT + CD_FLAG_ISNOTEDITABLE); -// free(events); - - add_to_problem_data_ext(problem_data, CD_DUMPDIR, dump_dir_name, - CD_FLAG_TXT + CD_FLAG_ISNOTEDITABLE); - - return problem_data; -} - -/* Remove dump dir */ -int DeleteDebugDump(const char *dump_dir_name, long caller_uid) -{ - /* If doesn't start with "DEBUG_DUMPS_DIR/"... */ - if (strncmp(dump_dir_name, DEBUG_DUMPS_DIR"/", strlen(DEBUG_DUMPS_DIR"/")) != 0 - /* or contains "/." anywhere (-> might contain ".." component) */ - || strstr(dump_dir_name + strlen(DEBUG_DUMPS_DIR), "/.") - ) { - /* Then refuse to operate on it (someone is attacking us??) */ - error_msg("Bad dump directory name '%s', not deleting", dump_dir_name); - return MW_ERROR; - } - - struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ 0); - if (!dd) - return MW_NOENT_ERROR; - - if (caller_uid != 0) /* not called by root */ - { - char caller_uid_str[sizeof(long) * 3 + 2]; - sprintf(caller_uid_str, "%ld", caller_uid); - - char *uid = dd_load_text_ext(dd, FILENAME_UID, DD_FAIL_QUIETLY_ENOENT | DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE); - /* we assume that the dump_dir can be handled by everyone if uid == NULL - * e.g: kerneloops - */ - if (uid != NULL) - { - bool uid_matches = (strcmp(uid, caller_uid_str) == 0); - free(uid); - if (!uid_matches) - { - dd_close(dd); - error_msg("Dump directory '%s' can't be accessed by user with uid %ld", dump_dir_name, caller_uid); - return 1; - } - } - } - - dd_delete(dd); - - return 0; /* success */ -} diff --git a/src/daemon/MiddleWare.h b/src/daemon/MiddleWare.h deleted file mode 100644 index 1c7645fe..00000000 --- a/src/daemon/MiddleWare.h +++ /dev/null @@ -1,61 +0,0 @@ -/* - MiddleWare.h - header file for MiddleWare library. It wraps plugins and - take case of them. - - Copyright (C) 2009 Zdenek Prikryl (zprikryl@redhat.com) - Copyright (C) 2009 RedHat inc. - - This program is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 2 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License along - with this program; if not, write to the Free Software Foundation, Inc., - 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -*/ -#ifndef MIDDLEWARE_H_ -#define MIDDLEWARE_H_ - -#include "abrt_types.h" - -#ifdef __cplusplus -extern "C" { -#endif - -/** - * An enum contains all return codes. - */ -typedef enum { - MW_OK, /**< No error.*/ - MW_OCCURRED, /**< No error, but thus dump is a dup.*/ - MW_ERROR, /**< Common error.*/ - MW_NOENT_ERROR, - MW_PERM_ERROR, - MW_CORRUPTED, /**< Debugdump directory is corrupted.*/ - MW_GPG_ERROR, /**< Package is not signed properly.*/ - MW_PLUGIN_ERROR, /**< plugin wasn't found or error within plugin*/ -} mw_result_t; - -/** - * Detects whether it's a duplicate crash dump. - * Fills crash info. - * Note that if it's a dup, loads _first crash_ info, not this one's. - * @param dump_dir_name A debugdump directory. - * @param problem_data A problem data. - * @return It return results of operation. See mw_result_t. - */ -mw_result_t LoadDebugDump(const char *dump_dir_name, problem_data_t **problem_data); - -int DeleteDebugDump(const char *dump_dir_name, long caller_uid); - -#ifdef __cplusplus -} -#endif - -#endif /*MIDDLEWARE_H_*/ diff --git a/src/daemon/abrtd.c b/src/daemon/abrtd.c index 5746a319..32d948a5 100644 --- a/src/daemon/abrtd.c +++ b/src/daemon/abrtd.c @@ -27,7 +27,6 @@ #include "abrtlib.h" #include "comm_layer_inner.h" #include "CommLayerServerDBus.h" -#include "MiddleWare.h" #include "parse_options.h" #define VAR_RUN_PIDFILE VAR_RUN"/abrtd.pid" @@ -44,9 +43,6 @@ * - DBus: dbus message arrived * - signal: we got SIGTERM or SIGINT * - * DBus methods we have: - * - DeleteDebugDump(crash_id): delete it from DB and delete corresponding /var/spool/abrt/DIR - * * DBus signals we emit: * - Crash(progname, crash_id, dir, uid) - a new crash occurred (new /var/spool/abrt/DIR is found) * - Warning(msg) @@ -247,7 +243,227 @@ static gboolean handle_signal_cb(GIOChannel *gio, GIOCondition condition, gpoint return TRUE; /* "please don't remove this event" */ } + /* Inotify handler */ + +/* helpers first */ + +struct logging_state { + char *last_line; +}; + +static char *do_log_and_save_line(char *log_line, void *param) +{ + struct logging_state *l_state = (struct logging_state *)param; + + VERB1 log("%s", log_line); + update_client("%s", log_line); + free(l_state->last_line); + l_state->last_line = log_line; + return NULL; +} + +/* We need to share some data between LoadDebugDump and is_crash_a_dup: */ +struct cdump_state { + char *uid; /* filled by LoadDebugDump */ + char *uuid; /* filled by is_crash_a_dup */ + char *crash_dump_dup_name; /* filled by is_crash_a_dup */ +}; + +static int is_crash_a_dup(const char *dump_dir_name, void *param) +{ + struct cdump_state *state = (struct cdump_state *)param; + + if (state->uuid) + return 0; /* we already checked it, don't do it again */ + + struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ 0); + if (!dd) + return 0; /* wtf? (error, but will be handled elsewhere later) */ + state->uuid = dd_load_text_ext(dd, FILENAME_UUID, + DD_FAIL_QUIETLY_ENOENT + DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE + ); + dd_close(dd); + if (!state->uuid) + { + return 0; /* no uuid (yet), "run_event, please continue iterating" */ + } + + /* Scan crash dumps looking for a dup */ +//TODO: explain why this is safe wrt concurrent runs + DIR *dir = opendir(DEBUG_DUMPS_DIR); + if (dir != NULL) + { + struct dirent *dent; + while ((dent = readdir(dir)) != NULL) + { + if (dot_or_dotdot(dent->d_name)) + continue; /* skip "." and ".." */ + + int different; + char *uid, *uuid; + char *dump_dir_name2 = concat_path_file(DEBUG_DUMPS_DIR, dent->d_name); + + if (strcmp(dump_dir_name, dump_dir_name2) == 0) + goto next; /* we are never a dup of ourself */ + + dd = dd_opendir(dump_dir_name2, /*flags:*/ DD_FAIL_QUIETLY_ENOENT); + if (!dd) + goto next; + uid = dd_load_text(dd, FILENAME_UID); + uuid = dd_load_text(dd, FILENAME_UUID); + dd_close(dd); + different = strcmp(state->uid, uid) || strcmp(state->uuid, uuid); + free(uid); + free(uuid); + if (different) + goto next; + + state->crash_dump_dup_name = dump_dir_name2; + /* "run_event, please stop iterating": */ + return 1; + + next: + free(dump_dir_name2); + } + closedir(dir); + } + + /* No dup found */ + return 0; /* "run_event, please continue iterating" */ +} + +static char *do_log(char *log_line, void *param) +{ + VERB1 log("%s", log_line); + //update_client("%s", log_line); + return log_line; +} + +static problem_data_t *load_crash_info(const char *dump_dir_name) +{ + struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ 0); + if (!dd) + return NULL; + + problem_data_t *problem_data = create_problem_data_from_dump_dir(dd); + dd_close(dd); + + add_to_problem_data_ext(problem_data, CD_DUMPDIR, dump_dir_name, + CD_FLAG_TXT + CD_FLAG_ISNOTEDITABLE); + + return problem_data; +} + +typedef enum { + MW_OK, /**< No error.*/ + MW_OCCURRED, /**< No error, but thus dump is a dup.*/ + MW_ERROR, /**< Common error.*/ +} mw_result_t; + +static mw_result_t LoadDebugDump(const char *dump_dir_name, problem_data_t **problem_data) +{ + mw_result_t res; + + struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ 0); + if (!dd) + return MW_ERROR; + struct cdump_state state; + state.uid = dd_load_text(dd, FILENAME_UID); + state.uuid = NULL; + state.crash_dump_dup_name = NULL; + char *analyzer = dd_load_text(dd, FILENAME_ANALYZER); + dd_close(dd); + + res = MW_ERROR; + + /* Run post-create event handler(s) */ + struct run_event_state *run_state = new_run_event_state(); + run_state->post_run_callback = is_crash_a_dup; + run_state->post_run_param = &state; + run_state->logging_callback = do_log; + int r = run_event_on_dir_name(run_state, dump_dir_name, "post-create"); + free_run_event_state(run_state); + +//TODO: consider this case: +// new dump is created, post-create detects that it is a dup, +// but then load_crash_info(dup_name) *FAILS*. +// In this case, we later delete damaged dup_name (right?) +// but new dump never gets its FILENAME_COUNT set! + + /* Is crash a dup? (In this case, is_crash_a_dup() should have + * aborted "post-create" event processing as soon as it saw uuid + * and determined that there is another crash with same uuid. + * In this case it sets state.crash_dump_dup_name) + */ + if (!state.crash_dump_dup_name) + { + /* No. Was there error on one of processing steps in run_event? */ + if (r != 0) + goto ret; /* yes */ + + /* Was uuid created after all? (In this case, is_crash_a_dup() + * should have fetched it and created state.uuid) + */ + if (!state.uuid) + { + /* no */ + log("Dump directory '%s' has no UUID element", dump_dir_name); + goto ret; + } + } + else + { + dump_dir_name = state.crash_dump_dup_name; + } + + /* Loads problem_data (from the *first debugdump dir* if this one is a dup) + * Returns: + * MW_OCCURRED: "crash count is != 1" (iow: it is > 1 - dup) + * MW_OK: "crash count is 1" (iow: this is a new crash, not a dup) + * else: an error code + */ + { + dd = dd_opendir(dump_dir_name, /*flags:*/ 0); + if (!dd) + { + res = MW_ERROR; + goto ret; + } + + /* Reset mode/uig/gid to correct values for all files created by event run */ + dd_sanitize_mode_and_owner(dd); + + /* Update count */ + char *count_str = dd_load_text_ext(dd, FILENAME_COUNT, DD_FAIL_QUIETLY_ENOENT); + unsigned long count = strtoul(count_str, NULL, 10); + count++; + char new_count_str[sizeof(long)*3 + 2]; + sprintf(new_count_str, "%lu", count); + dd_save_text(dd, FILENAME_COUNT, new_count_str); + dd_close(dd); + + *problem_data = load_crash_info(dump_dir_name); + if (*problem_data != NULL) + { + res = MW_OK; + if (count > 1) + { + log("Dump directory is a duplicate of %s", dump_dir_name); + res = MW_OCCURRED; + } + } + } + + ret: + free(state.uuid); + free(state.uid); + free(state.crash_dump_dup_name); + free(analyzer); + + return res; +} + static gboolean handle_inotify_cb(GIOChannel *gio, GIOCondition condition, gpointer ptr_unused) { /* Default size: 128 simultaneous actions (about 1/2 meg) */ @@ -373,8 +589,6 @@ static gboolean handle_inotify_cb(GIOChannel *gio, GIOCondition condition, gpoin ); break; } - case MW_CORRUPTED: - case MW_GPG_ERROR: default: log("Corrupted or bad dump %s (res:%d), deleting", fullname, (int)res); delete_dump_dir(fullname); @@ -388,6 +602,7 @@ static gboolean handle_inotify_cb(GIOChannel *gio, GIOCondition condition, gpoin return TRUE; /* "please don't remove this event" */ } + /* Run main loop with idle timeout. * Basically, almost like glib's g_main_run(loop) */ -- cgit