[systemd-commits] 4 commits - Makefile.am src/journal src/shared src/udev

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Fri Mar 22 12:41:56 PDT 2013


 Makefile.am                    |    8 +
 src/journal/journal-internal.h |    3 
 src/journal/journalctl.c       |  183 +++++++++++++++++++++--------------------
 src/journal/sd-journal.c       |   54 +++++++++---
 src/shared/acl-util.c          |   60 +++++++++++++
 src/shared/acl-util.h          |    3 
 src/shared/util.c              |    4 
 src/udev/collect/collect.c     |    4 
 8 files changed, 212 insertions(+), 107 deletions(-)

New commits:
commit 7d566799022b24fef2bb8ca6d671bf8ad14d9e26
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Mar 21 20:05:42 2013 -0400

    udev/collect: avoid initalizing memory twice

diff --git a/src/udev/collect/collect.c b/src/udev/collect/collect.c
index 7850cfa..6580a09 100644
--- a/src/udev/collect/collect.c
+++ b/src/udev/collect/collect.c
@@ -139,12 +139,14 @@ static int checkout(int fd)
 
  restart:
         len = bufsize >> 1;
-        buf = calloc(1,bufsize + 1);
+        buf = malloc(bufsize + 1);
         if (!buf) {
                 fprintf(stderr, "Out of memory.\n");
                 return log_oom();
         }
         memset(buf, ' ', bufsize);
+        buf[bufsize] = '\0';
+
         ptr = buf + len;
         while ((read(fd, buf + len, len)) > 0) {
                 while (ptr && *ptr) {

commit 48deb058b62b7245d59344134a126a3d5bdb5b58
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Mar 21 23:24:30 2013 -0400

    util: workaround two gcc warnings
    
    gcc does not know that errno cannot be negative, and warns
    about unitialized variables later on. Kill the warnings by
    returning -errno only after checking that errno is positive.

diff --git a/src/shared/util.c b/src/shared/util.c
index bc6e035..260c100 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -290,7 +290,7 @@ int safe_atou(const char *s, unsigned *ret_u) {
         l = strtoul(s, &x, 0);
 
         if (!x || x == s || *x || errno)
-                return errno ? -errno : -EINVAL;
+                return errno > 0 ? -errno : -EINVAL;
 
         if ((unsigned long) (unsigned) l != l)
                 return -ERANGE;
@@ -310,7 +310,7 @@ int safe_atoi(const char *s, int *ret_i) {
         l = strtol(s, &x, 0);
 
         if (!x || x == s || *x || errno)
-                return errno ? -errno : -EINVAL;
+                return errno > 0 ? -errno : -EINVAL;
 
         if ((long) (int) l != l)
                 return -ERANGE;

commit 6fe391c56d3f4231576ccc9d62d2000f37640a92
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Mon Mar 11 18:03:13 2013 -0400

    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.

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 8543adf..91dbde3 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 ef4b9b2..c62ad81 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)) {
 

commit 478c82693c386e7a6e8e4b37cc99fb19b12e7186
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Fri Mar 22 14:35:26 2013 +0000

    build-sys: move acl searching code into libsystemd-acl
    
    This loop over acls is a bit too much to keep inside
    of another loop.

diff --git a/Makefile.am b/Makefile.am
index 969f85a..93583a6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -793,7 +793,8 @@ libsystemd_acl_la_CFLAGS = \
 	$(ACL_CFLAGS)
 
 libsystemd_acl_la_LIBADD = \
-	$(ACL_LIBS)
+	$(ACL_LIBS) \
+	libsystemd-shared.la
 endif
 
 # ------------------------------------------------------------------------------
@@ -2503,6 +2504,11 @@ journalctl_LDADD = \
 	libsystemd-id128-internal.la \
 	libsystemd-logs.la
 
+if HAVE_ACL
+journalctl_LDADD += \
+	libsystemd-acl.la
+endif
+
 if HAVE_QRENCODE
 journalctl_SOURCES += \
 	src/journal/journal-qrcode.c \
diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index 4c288f3..8543adf 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -37,6 +37,7 @@
 
 #ifdef HAVE_ACL
 #include <sys/acl.h>
+#include "acl-util.h"
 #endif
 
 #include <systemd/sd-journal.h>
@@ -895,62 +896,18 @@ static int access_check(void) {
         if (!arg_quiet && geteuid() != 0) {
                 _cleanup_strv_free_ char **g = NULL;
                 bool have_access;
-                acl_t acl;
                 int r;
 
                 have_access = in_group("systemd-journal") > 0;
-                if (!have_access) {
 
+                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 */
-
-                        acl = acl_get_file("/var/log/journal/", ACL_TYPE_DEFAULT);
-                        if (acl) {
-                                acl_entry_t entry;
-
-                                r = acl_get_entry(acl, ACL_FIRST_ENTRY, &entry);
-                                while (r > 0) {
-                                        acl_tag_t tag;
-                                        gid_t *gid;
-                                        char *name;
-
-                                        r = acl_get_tag_type(entry, &tag);
-                                        if (r < 0)
-                                                break;
-
-                                        if (tag != ACL_GROUP)
-                                                goto next;
-
-                                        gid = acl_get_qualifier(entry);
-                                        if (!gid)
-                                                break;
-
-                                        if (in_gid(*gid) > 0) {
-                                                have_access = true;
-                                                break;
-                                        }
-
-                                        name = gid_to_name(*gid);
-                                        if (!name) {
-                                                acl_free(acl);
-                                                return log_oom();
-                                        }
-
-                                        r = strv_push(&g, name);
-                                        if (r < 0) {
-                                                free(name);
-                                                acl_free(acl);
-                                                return log_oom();
-                                        }
-
-                                next:
-                                        r = acl_get_entry(acl, ACL_NEXT_ENTRY, &entry);
-                                }
-
-                                acl_free(acl);
-                        }
+                        r = search_acl_groups(&g, "/var/log/journal/", &have_access);
+                        if (r < 0)
+                                return r;
                 }
 
                 if (!have_access) {
diff --git a/src/shared/acl-util.c b/src/shared/acl-util.c
index d1eb6f2..48bb12f 100644
--- a/src/shared/acl-util.c
+++ b/src/shared/acl-util.c
@@ -3,7 +3,7 @@
 /***
   This file is part of systemd.
 
-  Copyright 2011 Lennart Poettering
+  Copyright 2011,2013 Lennart Poettering
 
   systemd is free software; you can redistribute it and/or modify it
   under the terms of the GNU Lesser General Public License as published by
@@ -26,6 +26,8 @@
 #include <stdbool.h>
 
 #include "acl-util.h"
+#include "util.h"
+#include "strv.h"
 
 int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *entry) {
         acl_entry_t i;
@@ -66,3 +68,59 @@ int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *entry) {
 
         return 0;
 }
+
+int search_acl_groups(char*** dst, const char* path, bool* belong) {
+        acl_t acl;
+
+        assert(path);
+        assert(belong);
+
+        acl = acl_get_file(path, ACL_TYPE_DEFAULT);
+        if (acl) {
+                acl_entry_t entry;
+                int r;
+
+                r = acl_get_entry(acl, ACL_FIRST_ENTRY, &entry);
+                while (r > 0) {
+                        acl_tag_t tag;
+                        gid_t *gid;
+                        char *name;
+
+                        r = acl_get_tag_type(entry, &tag);
+                        if (r < 0)
+                                break;
+
+                        if (tag != ACL_GROUP)
+                                goto next;
+
+                        gid = acl_get_qualifier(entry);
+                        if (!gid)
+                                break;
+
+                        if (in_gid(*gid) > 0) {
+                                *belong = true;
+                                break;
+                        }
+
+                        name = gid_to_name(*gid);
+                        if (!name) {
+                                acl_free(acl);
+                                return log_oom();
+                        }
+
+                        r = strv_push(dst, name);
+                        if (r < 0) {
+                                free(name);
+                                acl_free(acl);
+                                return log_oom();
+                        }
+
+                next:
+                        r = acl_get_entry(acl, ACL_NEXT_ENTRY, &entry);
+                }
+
+                acl_free(acl);
+        }
+
+        return 0;
+}
diff --git a/src/shared/acl-util.h b/src/shared/acl-util.h
index 31fbbcd..23090d9 100644
--- a/src/shared/acl-util.h
+++ b/src/shared/acl-util.h
@@ -21,4 +21,7 @@
   along with systemd; If not, see <http://www.gnu.org/licenses/>.
 ***/
 
+#include <stdbool.h>
+
 int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *entry);
+int search_acl_groups(char*** dst, const char* path, bool* belong);



More information about the systemd-commits mailing list