[systemd-commits] 5 commits - m4/attributes.m4 Makefile.am src/core src/login src/shared src/test TODO

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Mon Sep 16 07:59:40 PDT 2013


 Makefile.am                |   16 ++++++++++++----
 TODO                       |    3 ---
 m4/attributes.m4           |    6 +++---
 src/core/transaction.c     |   14 +++++++++++---
 src/login/login-shared.c   |    8 ++++++++
 src/login/login-shared.h   |    3 +++
 src/login/logind-dbus.c    |    1 +
 src/login/logind-session.c |    1 +
 src/login/logind-session.h |    1 +
 src/login/logind.c         |    6 ++++++
 src/login/sd-login.c       |   12 +++++++-----
 src/shared/cgroup-util.c   |    4 +---
 src/shared/def.h           |    5 +++++
 src/shared/env-util.c      |    5 ++---
 src/shared/replace-var.c   |    3 ++-
 src/shared/sleep-config.c  |    3 ++-
 src/shared/unit-name.c     |    5 ++---
 src/test/test-fileio.c     |    6 +++++-
 18 files changed, 72 insertions(+), 30 deletions(-)

New commits:
commit d8c8508bad097c5cc354b684866a5b7a83fce394
Author: David Herrmann <dh.herrmann at gmail.com>
Date:   Fri Sep 13 12:42:12 2013 +0200

    build: check for build/link flags harder
    
    Use AC_LINK_IFELSE instead of AC_COMPILE_IFELSE to test for flags that
    might succeed during compilation but not during linking. An example is gcc
    compiled with libssp support but gnu-ld without it. In this case
    -fstack-protector works fine during compilation but fails during linking
    as several internal helpers are missing.

diff --git a/m4/attributes.m4 b/m4/attributes.m4
index 7e080da..aa53ef2 100644
--- a/m4/attributes.m4
+++ b/m4/attributes.m4
@@ -42,9 +42,9 @@ AC_DEFUN([CC_CHECK_FLAG_APPEND], [
                  AS_TR_SH([cc_cv_$2_$3]),
           [eval "AS_TR_SH([cc_save_$2])='${$2}'"
            eval "AS_TR_SH([$2])='-Werror $3'"
-           AC_COMPILE_IFELSE([AC_LANG_SOURCE([int a = 0; int main(void) { return a; } ])],
-                                    [eval "AS_TR_SH([cc_cv_$2_$3])='yes'"],
-                                    [eval "AS_TR_SH([cc_cv_$2_$3])='no'"])
+           AC_LINK_IFELSE([AC_LANG_SOURCE([int a = 0; int main(void) { return a; } ])],
+                          [eval "AS_TR_SH([cc_cv_$2_$3])='yes'"],
+                          [eval "AS_TR_SH([cc_cv_$2_$3])='no'"])
            eval "AS_TR_SH([$2])='$cc_save_$2'"])
 
   AS_IF([eval test x$]AS_TR_SH([cc_cv_$2_$3])[ = xyes],

commit 4b549144d82ea0f368321d149215f577049fffa6
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Sep 15 22:26:56 2013 -0400

    Verify validity of session name when received from outside
    
    Only ASCII letters and digits are allowed.

diff --git a/Makefile.am b/Makefile.am
index 7318913..f7bc5f3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2324,7 +2324,10 @@ if HAVE_ACL
 libudev_core_la_SOURCES += \
 	src/udev/udev-builtin-uaccess.c \
 	src/login/logind-acl.c \
-	src/login/sd-login.c
+	src/login/sd-login.c \
+	src/systemd/sd-login.h \
+	src/login/login-shared.c \
+	src/login/login-shared.h
 
 libudev_core_la_LIBADD += \
 	libsystemd-acl.la
@@ -3759,7 +3762,9 @@ libsystemd_logind_core_la_SOURCES = \
 	src/login/logind-session-dbus.c \
 	src/login/logind-seat-dbus.c \
 	src/login/logind-user-dbus.c \
-	src/login/logind-acl.h
+	src/login/logind-acl.h \
+	src/login/login-shared.c \
+	src/login/login-shared.h
 
 libsystemd_logind_core_la_CFLAGS = \
 	$(AM_CFLAGS) \
@@ -3860,7 +3865,10 @@ tests += \
 	test-login-tables
 
 libsystemd_login_la_SOURCES = \
-	src/login/sd-login.c
+	src/login/sd-login.c \
+	src/systemd/sd-login.h \
+	src/login/login-shared.c \
+	src/login/login-shared.h
 
 libsystemd_login_la_CFLAGS = \
 	$(AM_CFLAGS) \
diff --git a/TODO b/TODO
index 9943b3e..bfeaa81 100644
--- a/TODO
+++ b/TODO
@@ -142,9 +142,6 @@ Features:
 
 * journald: make sure ratelimit is actually really per-service with the new cgroup changes
 
-* libsystemd-logind: sd_session_is_active() and friends: verify
-  validity of session name before appending it to a path
-
 * gparted needs to disable auto-activation of mount units somehow, or
   maybe we should stop doing auto-activation of this after boot
   entirely. https://bugzilla.gnome.org/show_bug.cgi?id=701676
diff --git a/src/login/login-shared.c b/src/login/login-shared.c
new file mode 100644
index 0000000..ff13c28
--- /dev/null
+++ b/src/login/login-shared.c
@@ -0,0 +1,8 @@
+#include "login-shared.h"
+#include "def.h"
+
+bool session_id_valid(const char *id) {
+        assert(id);
+
+        return id + strspn(id, LETTERS DIGITS) == '\0';
+}
diff --git a/src/login/login-shared.h b/src/login/login-shared.h
new file mode 100644
index 0000000..728ef00
--- /dev/null
+++ b/src/login/login-shared.h
@@ -0,0 +1,3 @@
+#include <stdbool.h>
+
+bool session_id_valid(const char *id);
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 345df9f..d052e74 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -554,6 +554,7 @@ static int bus_manager_create_session(Manager *m, DBusMessage *message) {
                  * the audit data and let's better register a new
                  * ID */
                 if (hashmap_get(m->sessions, id)) {
+                        log_warning("Existing logind session ID %s used by new audit session, ignoring", id);
                         audit_id = 0;
 
                         free(id);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index a726fb1..2d22a68 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -41,6 +41,7 @@ Session* session_new(Manager *m, const char *id) {
 
         assert(m);
         assert(id);
+        assert(session_id_valid(id));
 
         s = new0(Session, 1);
         if (!s)
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index edaae8d..9cf6485 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -29,6 +29,7 @@ typedef enum KillWho KillWho;
 #include "logind.h"
 #include "logind-seat.h"
 #include "logind-user.h"
+#include "login-shared.h"
 
 typedef enum SessionState {
         SESSION_OPENING,  /* Session scope is being created */
diff --git a/src/login/logind.c b/src/login/logind.c
index 9094567..4ef92b8 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -684,6 +684,12 @@ int manager_enumerate_sessions(Manager *m) {
                 if (!dirent_is_file(de))
                         continue;
 
+                if (!session_id_valid(de->d_name)) {
+                        log_warning("Invalid session file name '%s', ignoring.", de->d_name);
+                        r = -EINVAL;
+                        continue;
+                }
+
                 k = manager_add_session(m, de->d_name, &s);
                 if (k < 0) {
                         log_error("Failed to add session by file name %s: %s", de->d_name, strerror(-k));
diff --git a/src/login/sd-login.c b/src/login/sd-login.c
index 8a7838d..71d8c29 100644
--- a/src/login/sd-login.c
+++ b/src/login/sd-login.c
@@ -31,6 +31,7 @@
 #include "sd-login.h"
 #include "strv.h"
 #include "fileio.h"
+#include "login-shared.h"
 
 _public_ int sd_pid_get_session(pid_t pid, char **session) {
         if (pid < 0)
@@ -226,17 +227,19 @@ static int file_of_session(const char *session, char **_p) {
 
         assert(_p);
 
-        if (session)
+        if (session) {
+                if (!session_id_valid(session))
+                        return -EINVAL;
+
                 p = strappend("/run/systemd/sessions/", session);
-        else {
-                char *buf;
+        } else {
+                _cleanup_free_ char *buf = NULL;
 
                 r = sd_pid_get_session(0, &buf);
                 if (r < 0)
                         return r;
 
                 p = strappend("/run/systemd/sessions/", buf);
-                free(buf);
         }
 
         if (!p)
@@ -255,7 +258,6 @@ _public_ int sd_session_is_active(const char *session) {
                 return r;
 
         r = parse_env_file(p, NEWLINE, "ACTIVE", &s, NULL);
-
         if (r < 0)
                 return r;
 
diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index 1d545e0..0bffebd 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -1511,9 +1511,7 @@ char *cg_unescape(const char *p) {
 }
 
 #define CONTROLLER_VALID                        \
-        "0123456789"                            \
-        "abcdefghijklmnopqrstuvwxyz"            \
-        "ABCDEFGHIJKLMNOPQRSTUVWXYZ"            \
+        DIGITS LETTERS                          \
         "_"
 
 bool cg_controller_is_valid(const char *p, bool allow_named) {
diff --git a/src/shared/def.h b/src/shared/def.h
index 5abb544..edd0bcf 100644
--- a/src/shared/def.h
+++ b/src/shared/def.h
@@ -33,3 +33,8 @@
 
 #define SIGNALS_CRASH_HANDLER SIGSEGV,SIGILL,SIGFPE,SIGBUS,SIGQUIT,SIGABRT
 #define SIGNALS_IGNORE SIGPIPE
+
+#define DIGITS            "0123456789"
+#define LOWERCASE_LETTERS "abcdefghijklmnopqrstuvwxyz"
+#define UPPERCASE_LETTERS "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+#define LETTERS LOWERCASE_LETTERS UPPERCASE_LETTERS
diff --git a/src/shared/env-util.c b/src/shared/env-util.c
index 6a52fb9..5e29629 100644
--- a/src/shared/env-util.c
+++ b/src/shared/env-util.c
@@ -27,11 +27,10 @@
 #include "utf8.h"
 #include "util.h"
 #include "env-util.h"
+#include "def.h"
 
 #define VALID_CHARS_ENV_NAME                    \
-        "0123456789"                            \
-        "abcdefghijklmnopqrstuvwxyz"            \
-        "ABCDEFGHIJKLMNOPQRSTUVWXYZ"            \
+        DIGITS LETTERS                          \
         "_"
 
 #ifndef ARG_MAX
diff --git a/src/shared/replace-var.c b/src/shared/replace-var.c
index e11c57a..478fc43 100644
--- a/src/shared/replace-var.c
+++ b/src/shared/replace-var.c
@@ -24,6 +24,7 @@
 #include "macro.h"
 #include "util.h"
 #include "replace-var.h"
+#include "def.h"
 
 /*
  * Generic infrastructure for replacing @FOO@ style variables in
@@ -40,7 +41,7 @@ static int get_variable(const char *b, char **r) {
         if (*b != '@')
                 return 0;
 
-        k = strspn(b + 1, "ABCDEFGHIJKLMNOPQRSTUVWXYZ_");
+        k = strspn(b + 1, UPPERCASE_LETTERS "_");
         if (k <= 0 || b[k+1] != '@')
                 return 0;
 
diff --git a/src/shared/unit-name.c b/src/shared/unit-name.c
index 1baa6eb..8f6c28e 100644
--- a/src/shared/unit-name.c
+++ b/src/shared/unit-name.c
@@ -26,11 +26,10 @@
 #include "path-util.h"
 #include "util.h"
 #include "unit-name.h"
+#include "def.h"
 
 #define VALID_CHARS                             \
-        "0123456789"                            \
-        "abcdefghijklmnopqrstuvwxyz"            \
-        "ABCDEFGHIJKLMNOPQRSTUVWXYZ"            \
+        DIGITS LETTERS                          \
         ":-_.\\"
 
 static const char* const unit_type_table[_UNIT_TYPE_MAX] = {

commit 1244d8d640a2644aa8dc8e588cd9c414b3d39163
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Sep 15 12:15:38 2013 -0400

    transaction.c: do not point users to logs when unit not found
    
    The logs are unlikely to contain any useful information in this case.
    
    Also, change "walked on cycle path" to "found dependency on", which
    is less technical and indicates the direction. With the old message,
    I was never sure if prior units depended on later ones, or vice versa.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=996133
    https://bugzilla.redhat.com/show_bug.cgi?id=997082

diff --git a/src/core/transaction.c b/src/core/transaction.c
index 27efef7..203070f 100644
--- a/src/core/transaction.c
+++ b/src/core/transaction.c
@@ -344,7 +344,7 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi
         assert(!j->transaction_prev);
 
         /* Does a recursive sweep through the ordering graph, looking
-         * for a cycle. If we find cycle we try to break it. */
+         * for a cycle. If we find a cycle we try to break it. */
 
         /* Have we seen this before? */
         if (j->generation == generation) {
@@ -371,7 +371,7 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi
 
                         /* logging for j not k here here to provide consistent narrative */
                         log_info_unit(j->unit->id,
-                                      "Walked on cycle path to %s/%s",
+                                      "Found dependency on %s/%s",
                                       k->unit->id, job_type_to_string(k->type));
 
                         if (!delete &&
@@ -860,7 +860,7 @@ int transaction_add_job_and_dependencies(
                 return -EINVAL;
         }
 
-        if (type != JOB_STOP && (unit->load_state == UNIT_ERROR || unit->load_state == UNIT_NOT_FOUND)) {
+        if (type != JOB_STOP && unit->load_state == UNIT_ERROR) {
                 dbus_set_error(e, BUS_ERROR_LOAD_FAILED,
                                "Unit %s failed to load: %s. "
                                "See system logs and 'systemctl status %s' for details.",
@@ -870,6 +870,14 @@ int transaction_add_job_and_dependencies(
                 return -EINVAL;
         }
 
+        if (type != JOB_STOP && unit->load_state == UNIT_NOT_FOUND) {
+                dbus_set_error(e, BUS_ERROR_LOAD_FAILED,
+                               "Unit %s failed to load: %s.",
+                               unit->id,
+                               strerror(-unit->load_error));
+                return -EINVAL;
+        }
+
         if (type != JOB_STOP && unit->load_state == UNIT_MASKED) {
                 dbus_set_error(e, BUS_ERROR_MASKED, "Unit %s is masked.", unit->id);
                 return -EADDRNOTAVAIL;

commit c32fc72f371535d29e0fce8c92694054c15427db
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Sep 15 10:35:51 2013 -0400

    Remove duplicate entries from syscall list
    
    ARM syscall list includes SYS_OABI_SYSCALL_BASE and SYS_SYSCALL_BASE
    which were obsuring real syscall names.

diff --git a/Makefile.am b/Makefile.am
index 7b7539a..7318913 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1027,7 +1027,7 @@ BUILT_SOURCES += \
 
 src/core/syscall-list.txt: Makefile
 	$(AM_V_at)$(MKDIR_P) $(dir $@)
-	$(AM_V_GEN)$(CPP) $(CFLAGS) $(AM_CPPFLAGS) $(CPPFLAGS) -dM -include sys/syscall.h - < /dev/null | $(AWK) '/^#define[ \t]+__NR_[^ ]+[ \t]+[0-9(]/ { sub(/__NR_/, "", $$2); print $$2; }' > $@
+	$(AM_V_GEN)$(CPP) $(CFLAGS) $(AM_CPPFLAGS) $(CPPFLAGS) -dM -include sys/syscall.h - < /dev/null | $(AWK) '/^#define[ \t]+__NR_[^ ]+[ \t]+[0-9(]/ { sub(/__NR_/, "", $$2); if ($$2 !~ /SYSCALL_BASE/) print $$2; }' > $@
 
 src/core/syscall-from-name.gperf: src/core/syscall-list.txt Makefile
 	$(AM_V_at)$(MKDIR_P) $(dir $@)

commit 442e00839e4fc3c11506f5c8a9477b465865aecc
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Sep 15 08:40:16 2013 -0400

    Assume that /proc/meminfo can be missing
    
    Travis tests are failing, probably because /proc/meminfo is not available
    in the test environment. The same might be true in some virtualized systems,
    so just treat missing /proc/meminfo as a sign that hibernation is not
    possible.

diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c
index 5ec7cce..148c4dc 100644
--- a/src/shared/sleep-config.c
+++ b/src/shared/sleep-config.c
@@ -172,7 +172,8 @@ static bool enough_memory_for_hibernation(void) {
 
         r = get_status_field("/proc/meminfo", "\nSwapFree:", &swapfree);
         if (r < 0) {
-                log_error("Failed to retrieve SwapFree from /proc/meminfo: %s", strerror(-r));
+                log_full(r == -ENOENT ? LOG_DEBUG : LOG_WARNING,
+                         "Failed to retrieve SwapFree from /proc/meminfo: %s", strerror(-r));
                 return false;
         }
 
diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c
index 4a4ed79..3511f3a 100644
--- a/src/test/test-fileio.c
+++ b/src/test/test-fileio.c
@@ -232,12 +232,16 @@ static void test_executable_is_script(void) {
 static void test_status_field(void) {
         _cleanup_free_ char *t = NULL, *p = NULL, *s = NULL;
         unsigned long long total, buffers;
+        int r;
 
         assert_se(get_status_field("/proc/self/status", "\nThreads:", &t) == 0);
         puts(t);
         assert_se(streq(t, "1"));
 
-        assert_se(get_status_field("/proc/meminfo", "MemTotal:", &p) == 0);
+        r = get_status_field("/proc/meminfo", "MemTotal:", &p);
+        if (r == -ENOENT)
+                return;
+        assert(r == 0);
         puts(p);
         assert_se(safe_atollu(p, &total) == 0);
 



More information about the systemd-commits mailing list