util: rework cunescape(), improve error handling

Change cunescape() to return a normal error code, so that we can
distuingish OOM errors from parse errors.

This also adds a flags parameter to control whether "relaxed" or normal
parsing shall be done. If set no parse failures are generated, and the
only reason why cunescape() can fail is OOM.

14 files changed, 138 insertions, 108 deletions

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 07384d3668..e61418db50 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -507,16 +507,17 @@ int config_parse_exec_oom_score_adjust(const char* unit,
         return 0;
 }
 
-int config_parse_exec(const char *unit,
-                      const char *filename,
-                      unsigned line,
-                      const char *section,
-                      unsigned section_line,
-                      const char *lvalue,
-                      int ltype,
-                      const char *rvalue,
-                      void *data,
-                      void *userdata) {
+int config_parse_exec(
+                const char *unit,
+                const char *filename,
+                unsigned line,
+                const char *section,
+                unsigned section_line,
+                const char *lvalue,
+                int ltype,
+                const char *rvalue,
+                void *data,
+                void *userdata) {
 
         ExecCommand **e = data, *nce;
         char *path, **n;
@@ -568,15 +569,13 @@ int config_parse_exec(const char *unit,
                                                 word ++;
                                         }
                                 }
-                        } else
-                                if (strneq(word, ";", MAX(l, 1U)))
+                        } else if (strneq(word, ";", MAX(l, 1U)))
                                         goto found;
 
                         k++;
                 }
                 if (!isempty(state)) {
-                        log_syntax(unit, LOG_ERR, filename, line, EINVAL,
-                                   "Trailing garbage, ignoring.");
+                        log_syntax(unit, LOG_ERR, filename, line, EINVAL, "Trailing garbage, ignoring.");
                         return 0;
                 }
 
@@ -605,9 +604,10 @@ int config_parse_exec(const char *unit,
                         else
                                 skip = strneq(word, "\\;", MAX(l, 1U));
 
-                        c = cunescape_length(word + skip, l - skip);
-                        if (!c) {
-                                r = log_oom();
+                        r = cunescape_length(word + skip, l - skip, 0, &c);
+                        if (r < 0) {
+                                log_syntax(unit, LOG_ERR, filename, line, r, "Failed to unescape command line, ignoring: %s", rvalue);
+                                r = 0;
                                 goto fail;
                         }
 
@@ -637,8 +637,7 @@ int config_parse_exec(const char *unit,
                 else
                         goto ok;
 
-                log_syntax(unit, LOG_ERR, filename, line, EINVAL,
-                           "%s, ignoring: %s", reason, rvalue);
+                log_syntax(unit, LOG_ERR, filename, line, EINVAL, "%s, ignoring: %s", reason, rvalue);
                 r = 0;
                 goto fail;
 
@@ -1976,8 +1975,7 @@ int config_parse_environ(const char *unit,
         if (u) {
                 r = unit_full_printf(u, rvalue, &k);
                 if (r < 0)
-                        log_syntax(unit, LOG_ERR, filename, line, -r,
-                                   "Failed to resolve specifiers, ignoring: %s", rvalue);
+                        log_syntax(unit, LOG_ERR, filename, line, -r, "Failed to resolve specifiers, ignoring: %s", rvalue);
         }
 
         if (!k)
@@ -1989,13 +1987,14 @@ int config_parse_environ(const char *unit,
                 _cleanup_free_ char *n;
                 char **x;
 
-                n = cunescape_length(word, l);
-                if (!n)
-                        return log_oom();
+                r = cunescape_length(word, l, 0, &n);
+                if (r < 0) {
+                        log_syntax(unit, LOG_ERR, filename, line, r, "Couldn't unescape assignment, ignoring: %s", rvalue);
+                        continue;
+                }
 
                 if (!env_assignment_is_valid(n)) {
-                        log_syntax(unit, LOG_ERR, filename, line, EINVAL,
-                                   "Invalid environment assignment, ignoring: %s", rvalue);
+                        log_syntax(unit, LOG_ERR, filename, line, EINVAL, "Invalid environment assignment, ignoring: %s", rvalue);
                         continue;
                 }
 
diff --git a/src/core/manager.c b/src/core/manager.c
index 73417ab1a8..1c912fcdf7 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -2426,11 +2426,9 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                         _cleanup_free_ char *uce = NULL;
                         char **e;
 
-                        uce = cunescape(l+4);
-                        if (!uce) {
-                                r = -ENOMEM;
+                        r = cunescape(l + 4, UNESCAPE_RELAX, &uce);
+                        if (r < 0)
                                 goto finish;
-                        }
 
                         e = strv_env_set(m->environment, uce);
                         if (!e) {
diff --git a/src/core/mount.c b/src/core/mount.c
index 53594cd40b..c4aa810d05 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -1579,7 +1579,7 @@ static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) {
         r = 0;
         for (;;) {
                 const char *device, *path, *options, *fstype;
-                _cleanup_free_ const char *d = NULL, *p = NULL;
+                _cleanup_free_ char *d = NULL, *p = NULL;
                 struct libmnt_fs *fs;
                 int k;
 
@@ -1594,12 +1594,10 @@ static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) {
                 options = mnt_fs_get_options(fs);
                 fstype = mnt_fs_get_fstype(fs);
 
-                d = cunescape(device);
-                if (!d)
+                if (cunescape(device, UNESCAPE_RELAX, &d) < 0)
                         return log_oom();
 
-                p = cunescape(path);
-                if (!p)
+                if (cunescape(path, UNESCAPE_RELAX, &p) < 0)
                         return log_oom();
 
                 (void) device_found_node(m, d, true, DEVICE_FOUND_MOUNT, set_flags);
diff --git a/src/core/swap.c b/src/core/swap.c
index 6b1bdb5c1e..53f127484f 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -1099,8 +1099,7 @@ static int swap_load_proc_swaps(Manager *m, bool set_flags) {
                         continue;
                 }
 
-                d = cunescape(dev);
-                if (!d)
+                if (cunescape(dev, UNESCAPE_RELAX, &d) < 0)
                         return log_oom();
 
                 device_found_node(m, d, true, DEVICE_FOUND_SWAP, set_flags);
diff --git a/src/core/umount.c b/src/core/umount.c
index ceee70fbea..bee267a5ad 100644
--- a/src/core/umount.c
+++ b/src/core/umount.c
@@ -62,6 +62,7 @@ static void mount_points_list_free(MountPoint **head) {
 static int mount_points_list_get(MountPoint **head) {
         _cleanup_fclose_ FILE *proc_self_mountinfo = NULL;
         unsigned int i;
+        int r;
 
         assert(head);
 
@@ -97,9 +98,9 @@ static int mount_points_list_get(MountPoint **head) {
                         continue;
                 }
 
-                p = cunescape(path);
-                if (!p)
-                        return -ENOMEM;
+                r = cunescape(path, UNESCAPE_RELAX, &p);
+                if (r < 0)
+                        return r;
 
                 /* Ignore mount points we can't unmount because they
                  * are API or because we are keeping them open (like
@@ -133,10 +134,12 @@ static int mount_points_list_get(MountPoint **head) {
 static int swap_list_get(MountPoint **head) {
         _cleanup_fclose_ FILE *proc_swaps = NULL;
         unsigned int i;
+        int r;
 
         assert(head);
 
-        if (!(proc_swaps = fopen("/proc/swaps", "re")))
+        proc_swaps = fopen("/proc/swaps", "re");
+        if (!proc_swaps)
                 return (errno == ENOENT) ? 0 : -errno;
 
         (void) fscanf(proc_swaps, "%*s %*s %*s %*s %*s\n");
@@ -146,19 +149,19 @@ static int swap_list_get(MountPoint **head) {
                 char *dev = NULL, *d;
                 int k;
 
-                if ((k = fscanf(proc_swaps,
-                                "%ms " /* device/file */
-                                "%*s " /* type of swap */
-                                "%*s " /* swap size */
-                                "%*s " /* used */
-                                "%*s\n", /* priority */
-                                &dev)) != 1) {
+                k = fscanf(proc_swaps,
+                           "%ms " /* device/file */
+                           "%*s " /* type of swap */
+                           "%*s " /* swap size */
+                           "%*s " /* used */
+                           "%*s\n", /* priority */
+                           &dev);
 
+                if (k != 1) {
                         if (k == EOF)
                                 break;
 
                         log_warning("Failed to parse /proc/swaps:%u.", i);
-
                         free(dev);
                         continue;
                 }
@@ -168,14 +171,13 @@ static int swap_list_get(MountPoint **head) {
                         continue;
                 }
 
-                d = cunescape(dev);
+                r = cunescape(dev, UNESCAPE_RELAX, &d);
                 free(dev);
+                if (r < 0)
+                        return r;
 
-                if (!d) {
-                        return -ENOMEM;
-                }
-
-                if (!(swap = new0(MountPoint, 1))) {
+                swap = new0(MountPoint, 1);
+                if (!swap) {
                         free(d);
                         return -ENOMEM;
                 }
diff --git a/src/import/pull-common.c b/src/import/pull-common.c
index efd67a2937..57323531e2 100644
--- a/src/import/pull-common.c
+++ b/src/import/pull-common.c
@@ -92,9 +92,9 @@ int pull_find_old_etags(const char *url, const char *image_root, int dt, const c
                 if (a >= b)
                         continue;
 
-                u = cunescape_length(a, b - a);
-                if (!u)
-                        return -ENOMEM;
+                r = cunescape_length(a, b - a, 0, &u);
+                if (r < 0)
+                        return r;
 
                 if (!http_etag_is_valid(u)) {
                         free(u);
diff --git a/src/journal/journald-kmsg.c b/src/journal/journald-kmsg.c
index c4216c4043..80bd9cd193 100644
--- a/src/journal/journald-kmsg.c
+++ b/src/journal/journald-kmsg.c
@@ -201,8 +201,7 @@ static void dev_kmsg_record(Server *s, const char *p, size_t l) {
 
                 *e = 0;
 
-                m = cunescape_length_with_prefix(k, e - k, "_KERNEL_");
-                if (!m)
+                if (cunescape_length_with_prefix(k, e - k, "_KERNEL_", UNESCAPE_RELAX, &m) < 0)
                         break;
 
                 if (startswith(m, "_KERNEL_DEVICE="))
@@ -299,8 +298,7 @@ static void dev_kmsg_record(Server *s, const char *p, size_t l) {
                 }
         }
 
-        message = cunescape_length_with_prefix(p, pl, "MESSAGE=");
-        if (message)
+        if (cunescape_length_with_prefix(p, pl, "MESSAGE=", UNESCAPE_RELAX, &message) >= 0)
                 IOVEC_SET_STRING(iovec[n++], message);
 
         server_dispatch_message(s, iovec, n, ELEMENTSOF(iovec), NULL, NULL, NULL, 0, NULL, priority, 0);
diff --git a/src/libsystemd/sd-login/sd-login.c b/src/libsystemd/sd-login/sd-login.c
index cc0677bdf2..072191e43f 100644
--- a/src/libsystemd/sd-login/sd-login.c
+++ b/src/libsystemd/sd-login/sd-login.c
@@ -496,9 +496,9 @@ _public_ int sd_session_get_desktop(const char *session, char **desktop) {
         if (r < 0)
                 return r;
 
-        t = cunescape(escaped);
-        if (!t)
-                return -ENOMEM;
+        r = cunescape(escaped, 0, &t);
+        if (r < 0)
+                return r;
 
         *desktop = t;
         return 0;
diff --git a/src/login/logind-acl.c b/src/login/logind-acl.c
index 941fd724a5..d2b3337788 100644
--- a/src/login/logind-acl.c
+++ b/src/login/logind-acl.c
@@ -253,8 +253,7 @@ int devnode_acl_all(struct udev *udev,
                 FOREACH_DIRENT(dent, dir, return -errno) {
                         _cleanup_free_ char *unescaped_devname = NULL;
 
-                        unescaped_devname = cunescape(dent->d_name);
-                        if (!unescaped_devname)
+                        if (cunescape(dent->d_name, UNESCAPE_RELAX, &unescaped_devname) < 0)
                                 return -ENOMEM;
 
                         n = strappend("/dev/", unescaped_devname);
diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c
index 5d81693e6c..5eb1a2ea65 100644
--- a/src/login/logind-inhibit.c
+++ b/src/login/logind-inhibit.c
@@ -231,18 +231,18 @@ int inhibitor_load(Inhibitor *i) {
         }
 
         if (who) {
-                cc = cunescape(who);
-                if (!cc)
-                        return -ENOMEM;
+                r = cunescape(who, 0, &cc);
+                if (r < 0)
+                        return r;
 
                 free(i->who);
                 i->who = cc;
         }
 
         if (why) {
-                cc = cunescape(why);
-                if (!cc)
-                        return -ENOMEM;
+                r = cunescape(why, 0, &cc);
+                if (r < 0)
+                        return r;
 
                 free(i->why);
                 i->why = cc;
diff --git a/src/shared/util.c b/src/shared/util.c
index 8b011ccfa1..67f66ace21 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -1466,12 +1466,13 @@ static int cunescape_one(const char *p, size_t length, char *ret) {
         return r;
 }
 
-char *cunescape_length_with_prefix(const char *s, size_t length, const char *prefix) {
+int cunescape_length_with_prefix(const char *s, size_t length, const char *prefix, UnescapeFlags flags, char **ret) {
         char *r, *t;
         const char *f;
         size_t pl;
 
         assert(s);
+        assert(ret);
 
         /* Undoes C style string escaping, and optionally prefixes it. */
 
@@ -1479,7 +1480,7 @@ char *cunescape_length_with_prefix(const char *s, size_t length, const char *pre
 
         r = new(char, pl+length+1);
         if (!r)
-                return NULL;
+                return -ENOMEM;
 
         if (prefix)
                 memcpy(r, prefix, pl);
@@ -1491,17 +1492,31 @@ char *cunescape_length_with_prefix(const char *s, size_t length, const char *pre
                 remaining = s + length - f;
                 assert(remaining > 0);
 
-                if (*f != '\\' || remaining == 1) {
-                        /* a literal literal, or a trailing backslash, copy verbatim */
+                if (*f != '\\') {
+                        /* A literal literal, copy verbatim */
                         *(t++) = *f;
                         continue;
                 }
 
+                if (remaining == 1) {
+                        if (flags & UNESCAPE_RELAX) {
+                                /* A trailing backslash, copy verbatim */
+                                *(t++) = *f;
+                                continue;
+                        }
+
+                        return -EINVAL;
+                }
+
                 k = cunescape_one(f + 1, remaining - 1, t);
                 if (k < 0) {
-                        /* Invalid escape code, let's take it literal then */
-                        *(t++) = '\\';
-                        continue;
+                        if (flags & UNESCAPE_RELAX) {
+                                /* Invalid escape code, let's take it literal then */
+                                *(t++) = '\\';
+                                continue;
+                        }
+
+                        return k;
                 }
 
                 f += k;
@@ -1509,17 +1524,17 @@ char *cunescape_length_with_prefix(const char *s, size_t length, const char *pre
         }
 
         *t = 0;
-        return r;
-}
 
-char *cunescape_length(const char *s, size_t length) {
-        return cunescape_length_with_prefix(s, length, NULL);
+        *ret = r;
+        return t - r;
 }
 
-char *cunescape(const char *s) {
-        assert(s);
+int cunescape_length(const char *s, size_t length, UnescapeFlags flags, char **ret) {
+        return cunescape_length_with_prefix(s, length, NULL, flags, ret);
+}
 
-        return cunescape_length(s, strlen(s));
+int cunescape(const char *s, UnescapeFlags flags, char **ret) {
+        return cunescape_length(s, strlen(s), flags, ret);
 }
 
 char *xescape(const char *s, const char *bad) {
@@ -6731,9 +6746,9 @@ int umount_recursive(const char *prefix, int flags) {
                                 continue;
                         }
 
-                        p = cunescape(path);
-                        if (!p)
-                                return -ENOMEM;
+                        r = cunescape(path, UNESCAPE_RELAX, &p);
+                        if (r < 0)
+                                return r;
 
                         if (!path_startswith(p, prefix))
                                 continue;
@@ -6833,9 +6848,9 @@ int bind_remount_recursive(const char *prefix, bool ro) {
                                 continue;
                         }
 
-                        p = cunescape(path);
-                        if (!p)
-                                return -ENOMEM;
+                        r = cunescape(path, UNESCAPE_RELAX, &p);
+                        if (r < 0)
+                                return r;
 
                         /* Let's ignore autofs mounts.  If they aren't
                          * triggered yet, we want to avoid triggering
diff --git a/src/shared/util.h b/src/shared/util.h
index 882355665c..e7a5d6366e 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -311,9 +311,14 @@ char decchar(int x) _const_;
 int undecchar(char c) _const_;
 
 char *cescape(const char *s);
-char *cunescape(const char *s);
-char *cunescape_length(const char *s, size_t length);
-char *cunescape_length_with_prefix(const char *s, size_t length, const char *prefix);
+
+typedef enum UnescapeFlags {
+        UNESCAPE_RELAX = 1,
+} UnescapeFlags;
+
+int cunescape(const char *s, UnescapeFlags flags, char **ret);
+int cunescape_length(const char *s, size_t length, UnescapeFlags flags, char **ret);
+int cunescape_length_with_prefix(const char *s, size_t length, const char *prefix, UnescapeFlags flags, char **ret);
 
 char *xescape(const char *s, const char *bad);
 
@@ -1014,7 +1019,7 @@ int take_password_lock(const char *root);
 int is_symlink(const char *path);
 int is_dir(const char *path, bool follow);
 
-typedef enum UnquoteFlags{
+typedef enum UnquoteFlags {
         UNQUOTE_RELAX     = 1,
         UNQUOTE_CUNESCAPE = 2,
 } UnquoteFlags;
diff --git a/src/test/test-util.c b/src/test/test-util.c
index e9d1522a65..aaf25f88bb 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -417,23 +417,40 @@ static void test_cescape(void) {
 static void test_cunescape(void) {
         _cleanup_free_ char *unescaped;
 
-        unescaped = cunescape("abc\\\\\\\"\\b\\f\\a\\n\\r\\t\\v\\003\\177\\234\\313\\000\\x00");
-        assert_se(streq_ptr(unescaped, "abc\\\"\b\f\a\n\r\t\v\003\177\234\313\\000\\x00"));
+        assert_se(cunescape("abc\\\\\\\"\\b\\f\\a\\n\\r\\t\\v\\003\\177\\234\\313\\000\\x00", 0, &unescaped) < 0);
+        assert_se(cunescape("abc\\\\\\\"\\b\\f\\a\\n\\r\\t\\v\\003\\177\\234\\313\\000\\x00", UNESCAPE_RELAX, &unescaped) >= 0);
+        const char *x = "abc\\\"\b\f\a\n\r\t\v\003\177\234\313\\000\\x00";
+        assert_se(streq_ptr(unescaped, x));
+        free(unescaped);
+        unescaped = NULL;
 
         /* incomplete sequences */
-        unescaped = cunescape("\\x0");
+        assert_se(cunescape("\\x0", 0, &unescaped) < 0);
+        assert_se(cunescape("\\x0", UNESCAPE_RELAX, &unescaped) >= 0);
         assert_se(streq_ptr(unescaped, "\\x0"));
+        free(unescaped);
+        unescaped = NULL;
 
-        unescaped = cunescape("\\x");
+        assert_se(cunescape("\\x", 0, &unescaped) < 0);
+        assert_se(cunescape("\\x", UNESCAPE_RELAX, &unescaped) >= 0);
         assert_se(streq_ptr(unescaped, "\\x"));
+        free(unescaped);
+        unescaped = NULL;
 
-        unescaped = cunescape("\\");
+        assert_se(cunescape("\\", 0, &unescaped) < 0);
+        assert_se(cunescape("\\", UNESCAPE_RELAX, &unescaped) >= 0);
         assert_se(streq_ptr(unescaped, "\\"));
+        free(unescaped);
+        unescaped = NULL;
 
-        unescaped = cunescape("\\11");
+        assert_se(cunescape("\\11", 0, &unescaped) < 0);
+        assert_se(cunescape("\\11", UNESCAPE_RELAX, &unescaped) >= 0);
         assert_se(streq_ptr(unescaped, "\\11"));
+        free(unescaped);
+        unescaped = NULL;
 
-        unescaped = cunescape("\\1");
+        assert_se(cunescape("\\1", 0, &unescaped) < 0);
+        assert_se(cunescape("\\1", UNESCAPE_RELAX, &unescaped) >= 0);
         assert_se(streq_ptr(unescaped, "\\1"));
 }
 
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 20972f6310..8c89fb33b8 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -913,13 +913,13 @@ static int write_one_file(Item *i, const char *path) {
         }
 
         if (i->argument) {
-                _cleanup_free_ char *unescaped;
+                _cleanup_free_ char *unescaped = NULL;
 
                 log_debug("%s to \"%s\".", i->type == CREATE_FILE ? "Appending" : "Writing", path);
 
-                unescaped = cunescape(i->argument);
-                if (!unescaped)
-                        return log_oom();
+                r = cunescape(i->argument, 0, &unescaped);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to unescape parameter to write: %s", i->argument);
 
                 r = loop_write(fd, unescaped, strlen(unescaped), false);
                 if (r < 0)