[systemd-devel] [PATCH] journalctl: be smarter about journal error checks
Zbigniew Jędrzejewski-Szmek
zbyszek at in.waw.pl
Fri Mar 22 09:18:53 PDT 2013
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.
For an unpriviledged user:
[testuser ~]$ journalctl
Showing user generated messages only. Users in the group 'systemd-journal' can see all messages. Pass -q to turn this notice off.
No journal files were opened, due to insufficient permissions.
[testuser ~]$ echo $?
1
---
This is an improved version. Messages can certainly be tweaked.
src/journal/journal-internal.h | 3 +++
src/journal/journalctl.c | 51 ++++++++++++++++++++++++-----------
src/journal/sd-journal.c | 61 +++++++++++++++++++++++++++++++++---------
3 files changed, 86 insertions(+), 29 deletions(-)
diff --git a/src/journal/journal-internal.h b/src/journal/journal-internal.h
index 97de0e7..bc9e44d 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 ddadc21..00f5206 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -878,24 +878,43 @@ static int verify(sd_journal *j) {
return r;
}
-static int access_check(void) {
+static int access_check(sd_journal *j) {
+ uint64_t eacces = EACCES, *code;
+ Iterator it;
+ int r = 0;
-#ifdef HAVE_ACL
- 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;
+ assert(j);
+ assert(j->errors);
+
+ if (set_isempty(j->errors)) {
+ if (hashmap_isempty(j->files))
+ log_info("No journal files were found.");
+ return 0;
}
- if (!arg_quiet && geteuid() != 0 && in_group("systemd-journal") <= 0)
- log_warning("Showing user generated messages only. Users in the group 'systemd-journal' can see all messages. Pass -q to turn this notice off.");
+ if (set_contains(j->errors, &eacces)) {
+#ifdef HAVE_ACL
+ if (access("/var/log/journal", F_OK) < 0 && geteuid() != 0 && in_group("systemd-journal") <= 0)
+ log_warning("Unprivileged users can't see messages unless persistent log storage is enabled. Users in the group 'systemd-journal' can always see messages.");
+
+ if (!arg_quiet && geteuid() != 0 && in_group("systemd-journal") <= 0)
+ log_warning("Showing user generated messages only. Users in the group 'systemd-journal' can see all messages. Pass -q to turn this notice off.");
#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. Only 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)
+ if (*code != EACCES)
+ log_warning("Error was encountered while opening journal files: %s",
+ strerror((int) *code));
+
+ return r;
}
int main(int argc, char *argv[]) {
@@ -943,10 +962,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
@@ -956,6 +971,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 ef4b9b2..0c1d812 100644
--- a/src/journal/sd-journal.c
+++ b/src/journal/sd-journal.c
@@ -49,6 +49,22 @@
#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) {
+ uint64_t code = -r;
+ uint64_t* copy;
+
+ if (r >= 0)
+ return r;
+
+ copy = newdup(uint64_t, &code, 1);
+ if (!copy)
+ return log_oom();
+
+ return set_put(errors, copy);
+}
+
static void detach_location(sd_journal *j) {
Iterator i;
JournalFile *f;
@@ -1239,7 +1255,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 +1396,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 +1475,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 +1520,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 +1531,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 +1578,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(uint64_hash_func, uint64_compare_func);
+ if (!j->files || !j->directories_by_path || !j->mmap || !j->errors)
goto fail;
return j;
@@ -1607,8 +1636,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 +1681,7 @@ _public_ void sd_journal_close(sd_journal *j) {
free(j->path);
free(j->unique_field);
+ set_free_free(j->errors);
free(j);
}
@@ -1968,8 +2000,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)) {
--
1.8.1.4
More information about the systemd-devel
mailing list