From 6fe391c56d3f4231576ccc9d62d2000f37640a92 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 11 Mar 2013 18:03:13 -0400 Subject: [PATCH] journalctl: be smarter about journal error checks There are many ways in which we can get those checks wrong, so it is better to warn and then error out on a real access failure. The error messages are wrapped to <80 lines, because their primary use is to be displayed in the terminal, and it is easier to read them this way. Reading them in the journal can be a bit trickier, but this is a bug in logs-show.c. --- src/journal/journal-internal.h | 3 + src/journal/journalctl.c | 136 ++++++++++++++++++++++----------- src/journal/sd-journal.c | 54 +++++++++---- 3 files changed, 136 insertions(+), 57 deletions(-) diff --git a/src/journal/journal-internal.h b/src/journal/journal-internal.h index 97de0e75f..bc9e44d42 100644 --- a/src/journal/journal-internal.h +++ b/src/journal/journal-internal.h @@ -30,6 +30,7 @@ #include "journal-def.h" #include "list.h" #include "hashmap.h" +#include "set.h" #include "journal-file.h" typedef struct Match Match; @@ -123,6 +124,8 @@ struct sd_journal { bool on_network; size_t data_threshold; + + Set *errors; }; char *journal_make_match_string(sd_journal *j); diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 8543adfb8..91dbde3ba 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -883,63 +883,111 @@ static int verify(sd_journal *j) { return r; } -static int access_check(void) { - #ifdef HAVE_ACL - /* If /var/log/journal doesn't even exist, unprivileged users have no access at all */ - if (access("/var/log/journal", F_OK) < 0 && geteuid() != 0 && in_group("systemd-journal") <= 0) { - log_error("Unprivileged users can't see messages unless persistent log storage is enabled. Users in the group 'systemd-journal' can always see messages."); - return -EACCES; +static int access_check_var_log_journal(sd_journal *j) { + _cleanup_strv_free_ char **g = NULL; + bool have_access; + int r; + + assert(j); + + have_access = in_group("systemd-journal") > 0; + + if (!have_access) { + /* Let's enumerate all groups from the default ACL of + * the directory, which generally should allow access + * to most journal files too */ + r = search_acl_groups(&g, "/var/log/journal/", &have_access); + if (r < 0) + return r; } - /* If /var/log/journal exists, try to pring a nice notice if the user lacks access to it */ - if (!arg_quiet && geteuid() != 0) { - _cleanup_strv_free_ char **g = NULL; - bool have_access; - int r; + if (!have_access) { - have_access = in_group("systemd-journal") > 0; + if (strv_isempty(g)) + log_notice("Hint: You are currently not seeing messages from other users and\n" + "the system. Users in the group 'systemd-journal' can see all messages.\n" + "Pass -q to turn this notice off."); + else { + _cleanup_free_ char *s = NULL; - if (!have_access) { - /* Let's enumerate all groups from the default - * ACL of the directory, which generally - * should allow access to most journal - * files too */ - r = search_acl_groups(&g, "/var/log/journal/", &have_access); + r = strv_extend(&g, "systemd-journal"); if (r < 0) - return r; + return log_oom(); + + strv_sort(g); + strv_uniq(g); + + s = strv_join(g, "', '"); + if (!s) + return log_oom(); + + log_notice("Hint: You are currently not seeing messages from other users and the system.\n" + "Users in the groups '%s' can see all messages.\n" + "Pass -q to turn this notice off.", s); } + } - if (!have_access) { + return 0; +} +#endif - if (strv_isempty(g)) - log_notice("Hint: You are currently not seeing messages from other users and the system. Users in the group 'systemd-journal' can see all messages. Pass -q to turn this notice off."); - else { - _cleanup_free_ char *s = NULL; +static int access_check(sd_journal *j) { + uint64_t eacces = EACCES, *code; + Iterator it; + int r = 0; - r = strv_extend(&g, "systemd-journal"); - if (r < 0) - return log_oom(); + assert(j); + assert(j->errors); + assert(j->files); - strv_sort(g); - strv_uniq(g); + if (set_isempty(j->errors)) { + if (hashmap_isempty(j->files)) + log_info("No journal files were found."); + return 0; + } - s = strv_join(g, "', '"); - if (!s) - return log_oom(); + if (!set_contains(j->errors, &eacces)) { +#ifdef HAVE_ACL + /* If /var/log/journal doesn't even exist, + unprivileged users have no access at all */ + if (access("/var/log/journal", F_OK) < 0 && + geteuid() != 0 && + in_group("systemd-journal") <= 0) { + log_error("Unprivileged users can't see messages unless persistent log storage\n" + "is enabled. Users in the group 'systemd-journal' can always see messages."); + return -EACCES; + } - log_notice("Hint: You are currently not seeing messages from other users and the system. Users in the groups '%s' can see all messages. Pass -q to turn this notice off.", s); - } + /* If /var/log/journal exists, try to pring a nice + notice if the user lacks access to it */ + if (!arg_quiet && geteuid() != 0) { + r = access_check_var_log_journal(j); + if (r < 0) + return r; } - } #else - if (geteuid() != 0 && in_group("systemd-journal") <= 0) { - log_error("No access to messages. Only users in the group 'systemd-journal' can see messages."); - return -EACCES; - } + if (geteuid() != 0 && in_group("systemd-journal") <= 0) + log_error("No access to messages.\n" + "Users in the group 'systemd-journal' can see messages."); #endif + if (hashmap_isempty(j->files)) { + log_error("No journal files were opened, due to insufficient permissions."); + r = -EACCES; + } + } - return 0; + SET_FOREACH(code, j->errors, it) { + int err = -PTR_TO_INT(code); + assert(err > 0); + if (err != EACCES) + log_warning("Error was encountered while opening journal files: %s", + strerror(err)); + } + + log_notice("Hint: run journalctl in debug mode: SYSTEMD_LOG_LEVEL=debug journalct ..."); + + return r; } int main(int argc, char *argv[]) { @@ -987,10 +1035,6 @@ int main(int argc, char *argv[]) { goto finish; } - r = access_check(); - if (r < 0) - return EXIT_FAILURE; - if (arg_directory) r = sd_journal_open_directory(&j, arg_directory, 0); else @@ -1000,6 +1044,10 @@ int main(int argc, char *argv[]) { return EXIT_FAILURE; } + r = access_check(j); + if (r < 0) + return EXIT_FAILURE; + if (arg_action == ACTION_VERIFY) { r = verify(j); goto finish; diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c index ef4b9b224..c62ad811d 100644 --- a/src/journal/sd-journal.c +++ b/src/journal/sd-journal.c @@ -49,6 +49,15 @@ #define DEFAULT_DATA_THRESHOLD (64*1024) +/* We return an error here only if we didn't manage to + memorize the real error. */ +static int set_put_error(Set* errors, int r) { + if (r >= 0) + return r; + + return set_put(errors, INT_TO_PTR(r)); +} + static void detach_location(sd_journal *j) { Iterator i; JournalFile *f; @@ -1239,7 +1248,7 @@ static int add_file(sd_journal *j, const char *prefix, const char *filename) { if (hashmap_size(j->files) >= JOURNAL_FILES_MAX) { log_debug("Too many open journal files, not adding %s, ignoring.", path); - return 0; + return set_put_error(j->errors, -ETOOMANYREFS); } r = journal_file_open(path, O_RDONLY, 0, false, false, NULL, j->mmap, NULL, &f); @@ -1380,8 +1389,13 @@ static int add_directory(sd_journal *j, const char *prefix, const char *dirname) if (dirent_is_file_with_suffix(de, ".journal") || dirent_is_file_with_suffix(de, ".journal~")) { r = add_file(j, m->path, de->d_name); - if (r < 0) - log_debug("Failed to add file %s/%s: %s", m->path, de->d_name, strerror(-r)); + if (r < 0) { + log_debug("Failed to add file %s/%s: %s", + m->path, de->d_name, strerror(-r)); + r = set_put_error(j->errors, r); + if (r < 0) + return r; + } } } @@ -1454,9 +1468,13 @@ static int add_root_directory(sd_journal *j, const char *p) { if (dirent_is_file_with_suffix(de, ".journal") || dirent_is_file_with_suffix(de, ".journal~")) { r = add_file(j, m->path, de->d_name); - if (r < 0) - log_debug("Failed to add file %s/%s: %s", m->path, de->d_name, strerror(-r)); - + if (r < 0) { + log_debug("Failed to add file %s/%s: %s", + m->path, de->d_name, strerror(-r)); + r = set_put_error(j->errors, r); + if (r < 0) + return r; + } } else if ((de->d_type == DT_DIR || de->d_type == DT_LNK || de->d_type == DT_UNKNOWN) && sd_id128_from_string(de->d_name, &id) >= 0) { @@ -1495,7 +1513,7 @@ static int remove_directory(sd_journal *j, Directory *d) { } static int add_search_paths(sd_journal *j) { - + int r; const char search_paths[] = "/run/log/journal\0" "/var/log/journal\0"; @@ -1506,8 +1524,11 @@ static int add_search_paths(sd_journal *j) { /* We ignore most errors here, since the idea is to only open * what's actually accessible, and ignore the rest. */ - NULSTR_FOREACH(p, search_paths) - add_root_directory(j, p); + NULSTR_FOREACH(p, search_paths) { + r = add_root_directory(j, p); + if (r < 0) + return set_put_error(j->errors, r); + } return 0; } @@ -1550,7 +1571,8 @@ static sd_journal *journal_new(int flags, const char *path) { j->files = hashmap_new(string_hash_func, string_compare_func); j->directories_by_path = hashmap_new(string_hash_func, string_compare_func); j->mmap = mmap_cache_new(); - if (!j->files || !j->directories_by_path || !j->mmap) + j->errors = set_new(trivial_hash_func, trivial_compare_func); + if (!j->files || !j->directories_by_path || !j->mmap || !j->errors) goto fail; return j; @@ -1607,8 +1629,10 @@ _public_ int sd_journal_open_directory(sd_journal **ret, const char *path, int f return -ENOMEM; r = add_root_directory(j, path); - if (r < 0) + if (r < 0) { + set_put_error(j->errors, r); goto fail; + } *ret = j; return 0; @@ -1650,6 +1674,7 @@ _public_ void sd_journal_close(sd_journal *j) { free(j->path); free(j->unique_field); + set_free(j->errors); free(j); } @@ -1968,8 +1993,11 @@ static void process_inotify_event(sd_journal *j, struct inotify_event *e) { if (e->mask & (IN_CREATE|IN_MOVED_TO|IN_MODIFY|IN_ATTRIB)) { r = add_file(j, d->path, e->name); - if (r < 0) - log_debug("Failed to add file %s/%s: %s", d->path, e->name, strerror(-r)); + if (r < 0) { + log_debug("Failed to add file %s/%s: %s", + d->path, e->name, strerror(-r)); + set_put_error(j->errors, r); + } } else if (e->mask & (IN_DELETE|IN_MOVED_FROM|IN_UNMOUNT)) { -- 2.30.2