Merge pull request #14339 from keszybz/invalid-enablement-logs

Small enhancements to logs for enabling of invalid units

TODO

@@ -1033,12 +1033,11 @@ Features:
   - allow Type=simple with PIDFile=
     https://bugzilla.redhat.com/show_bug.cgi?id=723942
   - allow writing multiple conditions in unit files on one line
-  - load-fragment: when loading a unit file via a chain of symlinks
-    verify that it is not masked via any of the names traversed.
   - introduce Type=pid-file
   - introduce mix of BindTo and Requisite
   - add a concept of RemainAfterExit= to scope units
   - Allow multiple ExecStart= for all Type= settings, so that we can cover rescue.service nicely
+  - add verification of [Install] section to systemd-analyze verify
 
 * udev-link-config:
    - Make sure ID_PATH is always exported and complete for

src/basic/random-util.c

@@ -7,6 +7,7 @@
 #include <elf.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <pthread.h>
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -28,6 +29,8 @@
 #include "siphash24.h"
 #include "time-util.h"
 
+static bool srand_called = false;
+
 int rdrand(unsigned long *ret) {
 
         /* So, you are a "security researcher", and you wonder why we bother with using raw RDRAND here,
@@ -272,8 +275,12 @@ int genuine_random_bytes(void *p, size_t n, RandomFlags flags) {
         return loop_read_exact(fd, p, n, true);
 }
 
+static void clear_srand_initialization(void) {
+        srand_called = false;
+}
+
 void initialize_srand(void) {
-        static bool srand_called = false;
+        static bool pthread_atfork_registered = false;
         unsigned x;
 #if HAVE_SYS_AUXV_H
         const void *auxv;
@@ -309,6 +316,11 @@ void initialize_srand(void) {
 
         srand(x);
         srand_called = true;
+
+        if (!pthread_atfork_registered) {
+                (void) pthread_atfork(NULL, NULL, clear_srand_initialization);
+                pthread_atfork_registered = true;
+        }
 }
 
 /* INT_MAX gives us only 31 bits, so use 24 out of that. */

src/basic/strv.c

@@ -16,8 +16,8 @@
 #include "string-util.h"
 #include "strv.h"
 
-char *strv_find(char **l, const char *name) {
-        char **i;
+char *strv_find(char * const *l, const char *name) {
+        char * const *i;
 
         assert(name);
 
@@ -28,8 +28,8 @@ char *strv_find(char **l, const char *name) {
         return NULL;
 }
 
-char *strv_find_prefix(char **l, const char *name) {
-        char **i;
+char *strv_find_prefix(char * const *l, const char *name) {
+        char * const *i;
 
         assert(name);
 
@@ -40,8 +40,8 @@ char *strv_find_prefix(char **l, const char *name) {
         return NULL;
 }
 
-char *strv_find_startswith(char **l, const char *name) {
-        char **i, *e;
+char *strv_find_startswith(char * const *l, const char *name) {
+        char * const *i, *e;
 
         assert(name);
 
@@ -181,8 +181,8 @@ char **strv_new_internal(const char *x, ...) {
         return r;
 }
 
-int strv_extend_strv(char ***a, char **b, bool filter_duplicates) {
-        char **s, **t;
+int strv_extend_strv(char ***a, char * const *b, bool filter_duplicates) {
+        char * const *s, **t;
         size_t p, q, i = 0, j;
 
         assert(a);
@@ -228,9 +228,9 @@ rollback:
         return -ENOMEM;
 }
 
-int strv_extend_strv_concat(char ***a, char **b, const char *suffix) {
+int strv_extend_strv_concat(char ***a, char * const *b, const char *suffix) {
+        char * const *s;
         int r;
-        char **s;
 
         STRV_FOREACH(s, b) {
                 char *v;
@@ -346,9 +346,9 @@ int strv_split_extract(char ***t, const char *s, const char *separators, Extract
         return (int) n;
 }
 
-char *strv_join_prefix(char **l, const char *separator, const char *prefix) {
+char *strv_join_prefix(char * const *l, const char *separator, const char *prefix) {
+        char * const *s;
         char *r, *e;
-        char **s;
         size_t n, k, m;
 
         if (!separator)
@@ -562,8 +562,8 @@ char **strv_uniq(char **l) {
         return l;
 }
 
-bool strv_is_uniq(char **l) {
-        char **i;
+bool strv_is_uniq(char * const *l) {
+        char * const *i;
 
         STRV_FOREACH(i, l)
                 if (strv_find(i+1, *i))
@@ -666,7 +666,7 @@ char **strv_split_nulstr(const char *s) {
         return r;
 }
 
-int strv_make_nulstr(char **l, char **p, size_t *q) {
+int strv_make_nulstr(char * const *l, char **ret, size_t *ret_size) {
         /* A valid nulstr with two NULs at the end will be created, but
          * q will be the length without the two trailing NULs. Thus the output
          * string is a valid nulstr and can be iterated over using NULSTR_FOREACH,
@@ -676,10 +676,10 @@ int strv_make_nulstr(char **l, char **p, size_t *q) {
 
         size_t n_allocated = 0, n = 0;
         _cleanup_free_ char *m = NULL;
-        char **i;
+        char * const *i;
 
-        assert(p);
-        assert(q);
+        assert(ret);
+        assert(ret_size);
 
         STRV_FOREACH(i, l) {
                 size_t z;
@@ -703,16 +703,16 @@ int strv_make_nulstr(char **l, char **p, size_t *q) {
                 m[n] = '\0';
 
         assert(n > 0);
-        *p = m;
-        *q = n - 1;
+        *ret = m;
+        *ret_size = n - 1;
 
         m = NULL;
 
         return 0;
 }
 
-bool strv_overlap(char **a, char **b) {
-        char **i;
+bool strv_overlap(char * const *a, char * const *b) {
+        char * const *i;
 
         STRV_FOREACH(i, a)
                 if (strv_contains(b, *i))
@@ -730,7 +730,7 @@ char **strv_sort(char **l) {
         return l;
 }
 
-bool strv_equal(char **a, char **b) {
+bool strv_equal(char * const *a, char * const *b) {
 
         if (strv_isempty(a))
                 return strv_isempty(b);
@@ -745,8 +745,8 @@ bool strv_equal(char **a, char **b) {
         return true;
 }
 
-void strv_print(char **l) {
-        char **s;
+void strv_print(char * const *l) {
+        char * const *s;
 
         STRV_FOREACH(s, l)
                 puts(*s);
@@ -874,9 +874,9 @@ rollback:
         return -ENOMEM;
 }
 
-int fputstrv(FILE *f, char **l, const char *separator, bool *space) {
+int fputstrv(FILE *f, char * const *l, const char *separator, bool *space) {
         bool b = false;
-        char **s;
+        char * const *s;
         int r;
 
         /* Like fputs(), but for strv, and with a less stupid argument order */

src/basic/strv.h

@@ -13,9 +13,9 @@
 #include "macro.h"
 #include "string-util.h"
 
-char *strv_find(char **l, const char *name) _pure_;
-char *strv_find_prefix(char **l, const char *name) _pure_;
-char *strv_find_startswith(char **l, const char *name) _pure_;
+char *strv_find(char * const *l, const char *name) _pure_;
+char *strv_find_prefix(char * const *l, const char *name) _pure_;
+char *strv_find_startswith(char * const *l, const char *name) _pure_;
 
 char **strv_free(char **l);
 DEFINE_TRIVIAL_CLEANUP_FUNC(char**, strv_free);
@@ -30,8 +30,8 @@ void strv_clear(char **l);
 char **strv_copy(char * const *l);
 size_t strv_length(char * const *l) _pure_;
 
-int strv_extend_strv(char ***a, char **b, bool filter_duplicates);
-int strv_extend_strv_concat(char ***a, char **b, const char *suffix);
+int strv_extend_strv(char ***a, char * const *b, bool filter_duplicates);
+int strv_extend_strv_concat(char ***a, char * const *b, const char *suffix);
 int strv_extend(char ***l, const char *value);
 int strv_extendf(char ***l, const char *format, ...) _printf_(2,0);
 int strv_extend_front(char ***l, const char *value);
@@ -49,9 +49,9 @@ int strv_consume_prepend(char ***l, char *value);
 
 char **strv_remove(char **l, const char *s);
 char **strv_uniq(char **l);
-bool strv_is_uniq(char **l);
+bool strv_is_uniq(char * const *l);
 
-bool strv_equal(char **a, char **b);
+bool strv_equal(char * const *a, char * const *b);
 
 #define strv_contains(l, s) (!!strv_find((l), (s)))
 
@@ -77,16 +77,16 @@ char **strv_split_newlines(const char *s);
 
 int strv_split_extract(char ***t, const char *s, const char *separators, ExtractFlags flags);
 
-char *strv_join_prefix(char **l, const char *separator, const char *prefix);
-static inline char *strv_join(char **l, const char *separator) {
+char *strv_join_prefix(char * const *l, const char *separator, const char *prefix);
+static inline char *strv_join(char * const *l, const char *separator) {
         return strv_join_prefix(l, separator, NULL);
 }
 
 char **strv_parse_nulstr(const char *s, size_t l);
 char **strv_split_nulstr(const char *s);
-int strv_make_nulstr(char **l, char **p, size_t *n);
+int strv_make_nulstr(char * const *l, char **p, size_t *n);
 
-bool strv_overlap(char **a, char **b) _pure_;
+bool strv_overlap(char * const *a, char * const *b) _pure_;
 
 #define STRV_FOREACH(s, l)                      \
         for ((s) = (l); (s) && *(s); (s)++)
@@ -103,7 +103,7 @@ bool strv_overlap(char **a, char **b) _pure_;
         for ((x) = (l), (y) = (x+1); (x) && *(x) && *(y); (x) += 2, (y) = (x + 1))
 
 char **strv_sort(char **l);
-void strv_print(char **l);
+void strv_print(char * const *l);
 
 #define STRV_MAKE(...) ((char**) ((const char*[]) { __VA_ARGS__, NULL }))
 
@@ -192,7 +192,7 @@ char **strv_skip(char **l, size_t n);
 
 int strv_extend_n(char ***l, const char *value, size_t n);
 
-int fputstrv(FILE *f, char **l, const char *separator, bool *space);
+int fputstrv(FILE *f, char * const *l, const char *separator, bool *space);
 
 #define strv_free_and_replace(a, b)             \
         ({                                      \

src/core/dbus-manager.c

@@ -1902,6 +1902,12 @@ static int install_error(
                                               "Unit %s is transient or generated.", changes[i].path);
                         goto found;
 
+                case -EUCLEAN:
+                        r = sd_bus_error_setf(error, BUS_ERROR_BAD_UNIT_SETTING,
+                                              "\"%s\" is not a valid unit name.",
+                                              changes[i].path);
+                        goto found;
+
                 case -ELOOP:
                         r = sd_bus_error_setf(error, BUS_ERROR_UNIT_LINKED,
                                               "Refusing to operate on alias name or linked unit file: %s",

src/shared/install.c

@@ -362,6 +362,12 @@ void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *chang
                                         verb, changes[i].path);
                         logged = true;
                         break;
+                case -EUCLEAN:
+                        log_error_errno(changes[i].type,
+                                        "Failed to %s unit, \"%s\" is not a valid unit name.",
+                                        verb, changes[i].path);
+                        logged = true;
+                        break;
                 case -ELOOP:
                         log_error_errno(changes[i].type, "Failed to %s unit, refusing to operate on linked unit file %s",
                                         verb, changes[i].path);
@@ -1138,9 +1144,6 @@ static int config_parse_also(
                 if (r < 0)
                         return r;
 
-                if (!unit_name_is_valid(printed, UNIT_NAME_ANY))
-                        return -EINVAL;
-
                 r = install_info_add(c, printed, NULL, true, &alsoinfo);
                 if (r < 0)
                         return r;
@@ -1194,7 +1197,8 @@ static int config_parse_default_instance(
         }
 
         if (!unit_instance_is_valid(printed))
-                return -EINVAL;
+                return log_syntax(unit, LOG_WARNING, filename, line, SYNTHETIC_ERRNO(EINVAL),
+                                  "Invalid DefaultInstance= value \"%s\".", printed);
 
         return free_and_replace(i->default_instance, printed);
 }
@@ -1797,7 +1801,8 @@ static int install_info_symlink_wants(
                         return q;
 
                 if (!unit_name_is_valid(dst, UNIT_NAME_ANY)) {
-                        r = -EINVAL;
+                        unit_file_changes_add(changes, n_changes, -EUCLEAN, dst, NULL);
+                        r = -EUCLEAN;
                         continue;
                 }