[systemd-commits] 4 commits - src/core src/login src/shared src/test TODO

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Sat Feb 8 10:08:11 PST 2014


 TODO                   |    3 -
 src/core/manager.c     |   48 +++++++---------------------
 src/login/pam-module.c |    2 -
 src/shared/fileio.c    |   84 ++++++++++++++++++++++++++++---------------------
 src/shared/utf8.c      |   26 +++++++++++++++
 src/shared/utf8.h      |    3 +
 src/test/test-utf8.c   |   17 +++++++++
 7 files changed, 108 insertions(+), 75 deletions(-)

New commits:
commit 2ba110900aca729f7d511c185b62149c4a28a293
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Jan 12 15:55:10 2014 -0500

    core: use automatic cleanup in two functions

diff --git a/src/core/manager.c b/src/core/manager.c
index b58b98c..dbe08f9 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1916,7 +1916,7 @@ int manager_get_job_from_dbus_path(Manager *m, const char *s, Job **_j) {
 void manager_send_unit_audit(Manager *m, Unit *u, int type, bool success) {
 
 #ifdef HAVE_AUDIT
-        char *p;
+        _cleanup_free_ char *p = NULL;
         int audit_fd;
 
         audit_fd = get_audit_fd();
@@ -1949,17 +1949,19 @@ void manager_send_unit_audit(Manager *m, Unit *u, int type, bool success) {
                 } else
                         log_warning("Failed to send audit message: %m");
         }
-
-        free(p);
 #endif
 
 }
 
 void manager_send_unit_plymouth(Manager *m, Unit *u) {
-        int fd = -1;
-        union sockaddr_union sa;
+        union sockaddr_union sa = {
+                .sa.sa_family = AF_UNIX,
+                .un.sun_path = "\0/org/freedesktop/plymouthd",
+        };
+
         int n = 0;
-        char *message = NULL;
+        _cleanup_free_ char *message = NULL;
+        _cleanup_close_ int fd = -1;
 
         /* Don't generate plymouth events if the service was already
          * started and we're just deserializing */
@@ -1985,46 +1987,22 @@ void manager_send_unit_plymouth(Manager *m, Unit *u) {
                 return;
         }
 
-        zero(sa);
-        sa.sa.sa_family = AF_UNIX;
-        strncpy(sa.un.sun_path+1, "/org/freedesktop/plymouthd", sizeof(sa.un.sun_path)-1);
         if (connect(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + 1 + strlen(sa.un.sun_path+1)) < 0) {
 
-                if (errno != EPIPE &&
-                    errno != EAGAIN &&
-                    errno != ENOENT &&
-                    errno != ECONNREFUSED &&
-                    errno != ECONNRESET &&
-                    errno != ECONNABORTED)
+                if (!IN_SET(errno, EPIPE, EAGAIN, ENOENT, ECONNREFUSED, ECONNRESET, ECONNABORTED))
                         log_error("connect() failed: %m");
-
-                goto finish;
+                return;
         }
 
         if (asprintf(&message, "U\002%c%s%n", (int) (strlen(u->id) + 1), u->id, &n) < 0) {
                 log_oom();
-                goto finish;
+                return;
         }
 
         errno = 0;
-        if (write(fd, message, n + 1) != n + 1) {
-
-                if (errno != EPIPE &&
-                    errno != EAGAIN &&
-                    errno != ENOENT &&
-                    errno != ECONNREFUSED &&
-                    errno != ECONNRESET &&
-                    errno != ECONNABORTED)
+        if (write(fd, message, n + 1) != n + 1)
+                if (!IN_SET(errno, EPIPE, EAGAIN, ENOENT, ECONNREFUSED, ECONNRESET, ECONNABORTED))
                         log_error("Failed to write Plymouth message: %m");
-
-                goto finish;
-        }
-
-finish:
-        if (fd >= 0)
-                close_nointr_nofail(fd);
-
-        free(message);
 }
 
 void manager_dispatch_bus_name_owner_changed(

commit 550a40eceb7d1917152fc9317bf2696708d52bc2
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Fri Jan 17 21:28:41 2014 -0500

    core: do not print invalid utf-8 in error messages

diff --git a/TODO b/TODO
index 57e82ff..1a1e889 100644
--- a/TODO
+++ b/TODO
@@ -16,9 +16,6 @@ Bugfixes:
 
 * properly handle .mount unit state tracking when two mount points are stacked one on top of another on the exact same mount point.
 
-* When we detect invalid UTF-8, we cannot use it in an error message:
-  log...("Path is not UTF-8 clean, ignoring assignment: %s", rvalue);
-
 * shorten the message to sane length:
 
   Cannot add dependency job for unit display-manager.service, ignoring: Unit display-manager.service failed to load: No such file or directory. See system logs and 'systemctl status display-manager.service' for details.
diff --git a/src/shared/fileio.c b/src/shared/fileio.c
index 838d128..b81eeb2 100644
--- a/src/shared/fileio.c
+++ b/src/shared/fileio.c
@@ -539,15 +539,18 @@ static int parse_env_file_push(const char *filename, unsigned line,
         va_list aq, *ap = userdata;
 
         if (!utf8_is_valid(key)) {
-                log_error("%s:%u: invalid UTF-8 for key '%s', ignoring.",
-                          filename, line, key);
+                _cleanup_free_ char *p = utf8_escape_invalid(key);
+
+                log_error("%s:%u: invalid UTF-8 in key '%s', ignoring.",
+                          filename, line, p);
                 return -EINVAL;
         }
 
         if (value && !utf8_is_valid(value)) {
-                /* FIXME: filter UTF-8 */
+                _cleanup_free_ char *p = utf8_escape_invalid(value);
+
                 log_error("%s:%u: invalid UTF-8 value for key %s: '%s', ignoring.",
-                          filename, line, key, value);
+                          filename, line, key, p);
                 return -EINVAL;
         }
 
diff --git a/src/shared/utf8.c b/src/shared/utf8.c
index 6e5ba9a..0b524d8 100644
--- a/src/shared/utf8.c
+++ b/src/shared/utf8.c
@@ -174,6 +174,32 @@ const char *utf8_is_valid(const char *str) {
         return str;
 }
 
+char *utf8_escape_invalid(const char *str) {
+        char *p, *s;
+
+        assert(str);
+
+        p = s = malloc(strlen(str) * 4 + 1);
+        if (!p)
+                return NULL;
+
+        while (*str) {
+                int len;
+
+                len = utf8_encoded_valid_unichar(str);
+                if (len > 0) {
+                        s = mempcpy(s, str, len);
+                        str += len;
+                } else {
+                        s = mempcpy(s, UTF8_REPLACEMENT_CHARACTER, strlen(UTF8_REPLACEMENT_CHARACTER));
+                        str += 1;
+                }
+        }
+        *s = '\0';
+
+        return p;
+}
+
 char *ascii_is_valid(const char *str) {
         const char *p;
 
diff --git a/src/shared/utf8.h b/src/shared/utf8.h
index f560774..c0eb73a 100644
--- a/src/shared/utf8.h
+++ b/src/shared/utf8.h
@@ -25,8 +25,11 @@
 
 #include "macro.h"
 
+#define UTF8_REPLACEMENT_CHARACTER "\xef\xbf\xbd"
+
 const char *utf8_is_valid(const char *s) _pure_;
 char *ascii_is_valid(const char *s) _pure_;
+char *utf8_escape_invalid(const char *s);
 
 bool utf8_is_printable(const char* str, size_t length) _pure_;
 
diff --git a/src/test/test-utf8.c b/src/test/test-utf8.c
index d2198fd..b7d988f 100644
--- a/src/test/test-utf8.c
+++ b/src/test/test-utf8.c
@@ -50,11 +50,28 @@ static void test_utf8_encoded_valid_unichar(void) {
 
 }
 
+static void test_utf8_escaping(void) {
+        _cleanup_free_ char *p1, *p2, *p3;
+
+        p1 = utf8_escape_invalid("goo goo goo");
+        puts(p1);
+        assert_se(utf8_is_valid(p1));
+
+        p2 = utf8_escape_invalid("\341\204\341\204");
+        puts(p2);
+        assert_se(utf8_is_valid(p2));
+
+        p3 = utf8_escape_invalid("\341\204");
+        puts(p3);
+        assert_se(utf8_is_valid(p3));
+}
+
 int main(int argc, char *argv[]) {
         test_utf8_is_valid();
         test_utf8_is_printable();
         test_ascii_is_valid();
         test_utf8_encoded_valid_unichar();
+        test_utf8_escaping();
 
         return 0;
 }

commit cda7ecb0a3d6cf839ec484a6690c12857a6e8375
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sat Feb 8 12:12:20 2014 -0500

    pam-module: avoid (null) in debug message

diff --git a/src/login/pam-module.c b/src/login/pam-module.c
index 79a9042..ae45673 100644
--- a/src/login/pam-module.c
+++ b/src/login/pam-module.c
@@ -374,7 +374,7 @@ _public_ PAM_EXTERN int pam_sm_open_session(
                            "uid=%u pid=%u service=%s type=%s class=%s desktop=%s seat=%s vtnr=%u tty=%s display=%s remote=%s remote_user=%s remote_host=%s",
                            pw->pw_uid, getpid(),
                            strempty(service),
-                           type, class, desktop,
+                           type, class, strempty(desktop),
                            strempty(seat), vtnr, strempty(tty), strempty(display),
                            yes_no(remote), strempty(remote_user), strempty(remote_host));
 

commit f27f0e2177ac0a4b96585aed7db3a080e27a2f00
Author: Goffredo Baroncelli <kreijack at libero.it>
Date:   Thu Feb 6 19:09:59 2014 +0100

    core: fix crashes if locale.conf contains invalid utf-8 string
    
    In the parse_env_file_push() and load_env_file_push() functions, there
    are two assert() call to check if the key or value parameters are utf8 valid.
    
    If the strings aren't utf8 valid, assert does abort.
    
    These function are used early by systemd to parse some files. For
    example '/etc/locale.conf'. In my case this file contained a not utf8
    sequence, which is bad, but systemd crashed during the boot, which
    is even worse!
    
    The enclosed patch removes the assert and return -EINVAL if the
    sequence is invalid. This is possible because the caller of these
    function [1] checks the errors.
    So the check of an invalid utf8 sequence is still performed, but
    systemd doesn't crash anymore and logs the error.
    
    [1] parse_env_file_internal(), invoked by load_env_file() and
    parse_env_file()

diff --git a/src/shared/fileio.c b/src/shared/fileio.c
index ede8819..838d128 100644
--- a/src/shared/fileio.c
+++ b/src/shared/fileio.c
@@ -534,35 +534,39 @@ fail:
 
 static int parse_env_file_push(const char *filename, unsigned line,
                                const char *key, char *value, void *userdata) {
-        assert(utf8_is_valid(key));
 
-        if (value && !utf8_is_valid(value))
+        const char *k;
+        va_list aq, *ap = userdata;
+
+        if (!utf8_is_valid(key)) {
+                log_error("%s:%u: invalid UTF-8 for key '%s', ignoring.",
+                          filename, line, key);
+                return -EINVAL;
+        }
+
+        if (value && !utf8_is_valid(value)) {
                 /* FIXME: filter UTF-8 */
-                log_error("%s:%u: invalid UTF-8 for key %s: '%s', ignoring.",
+                log_error("%s:%u: invalid UTF-8 value for key %s: '%s', ignoring.",
                           filename, line, key, value);
-        else {
-                const char *k;
-                va_list* ap = (va_list*) userdata;
-                va_list aq;
+                return -EINVAL;
+        }
 
-                va_copy(aq, *ap);
+        va_copy(aq, *ap);
 
-                while ((k = va_arg(aq, const char *))) {
-                        char **v;
+        while ((k = va_arg(aq, const char *))) {
+                char **v;
 
-                        v = va_arg(aq, char **);
+                v = va_arg(aq, char **);
 
-                        if (streq(key, k)) {
-                                va_end(aq);
-                                free(*v);
-                                *v = value;
-                                return 1;
-                        }
+                if (streq(key, k)) {
+                        va_end(aq);
+                        free(*v);
+                        *v = value;
+                        return 1;
                 }
-
-                va_end(aq);
         }
 
+        va_end(aq);
         free(value);
         return 0;
 }
@@ -586,26 +590,31 @@ int parse_env_file(
 
 static int load_env_file_push(const char *filename, unsigned line,
                               const char *key, char *value, void *userdata) {
-        assert(utf8_is_valid(key));
+        char ***m = userdata;
+        char *p;
+        int r;
 
-        if (value && !utf8_is_valid(value))
+        if (!utf8_is_valid(key)) {
+                log_error("%s:%u: invalid UTF-8 for key '%s', ignoring.",
+                          filename, line, key);
+                return -EINVAL;
+        }
+
+        if (value && !utf8_is_valid(value)) {
                 /* FIXME: filter UTF-8 */
-                log_error("%s:%u: invalid UTF-8 for key %s: '%s', ignoring.",
+                log_error("%s:%u: invalid UTF-8 value for key %s: '%s', ignoring.",
                           filename, line, key, value);
-        else {
-                char ***m = userdata;
-                char *p;
-                int r;
+                return -EINVAL;
+        }
 
-                p = strjoin(key, "=", strempty(value), NULL);
-                if (!p)
-                        return -ENOMEM;
+        p = strjoin(key, "=", strempty(value), NULL);
+        if (!p)
+                return -ENOMEM;
 
-                r = strv_push(m, p);
-                if (r < 0) {
-                        free(p);
-                        return r;
-                }
+        r = strv_push(m, p);
+        if (r < 0) {
+                free(p);
+                return r;
         }
 
         free(value);



More information about the systemd-commits mailing list