timesyncd: log louder when we refuse a server due to root distance

This is something people should know about, since it's caused by
misconfiguration.

Fixes: #13912

docs/PORTABILITY_AND_STABILITY.md

@@ -18,7 +18,7 @@ The stable interfaces are:
 
 * Some of the **"special" unit names** and their semantics. To be precise the ones that are necessary for normal services, and not those required only for early boot and late shutdown, with very few exceptions. To list them here: `basic.target`, `shutdown.target`, `sockets.target`, `network.target`, `getty.target`, `graphical.target`, `multi-user.target`, `rescue.target`, `emergency.target`, `poweroff.target`, `reboot.target`, `halt.target`, `runlevel[1-5].target`.
 
-* **The D-Bus interfaces of the main service daemon and other daemons**. We try to always preserve backwards compatiblity, and intentational breakage is never introduced. Nevertheless, when we find bugs that mean that the existing interface was not useful, or when the implementation did something different than stated by the documentation and the implemented behaviour is not useful, we will fix the implementation and thus introduce a change in behaviour. But the API (parameter counts and types) is never changed, and existing attributes and methods will not be removed.
+* **The D-Bus interfaces of the main service daemon and other daemons**. We try to always preserve backwards compatibility, and intentional breakage is never introduced. Nevertheless, when we find bugs that mean that the existing interface was not useful, or when the implementation did something different than stated by the documentation and the implemented behaviour is not useful, we will fix the implementation and thus introduce a change in behaviour. But the API (parameter counts and types) is never changed, and existing attributes and methods will not be removed.
 
 * For a more comprehensive and authoritative list, consult the chart below.
 
@@ -136,7 +136,7 @@ Of course, one last thing I can't make myself not ask you before we finish here,
 
 ## Independent Operation of systemd Programs
 
-Some programs in the systemd suite are indended to operate independently of the
+Some programs in the systemd suite are intended to operate independently of the
 running init process (or even without an init process, for example when
 creating system installation chroots). They can be safely called on systems with
 a different init process or for example in package installation scriptlets.

src/basic/strv.c

@@ -57,20 +57,15 @@ char *strv_find_startswith(char * const *l, const char *name) {
         return NULL;
 }
 
-void strv_clear(char **l) {
+char **strv_free(char **l) {
         char **k;
 
         if (!l)
-                return;
+                return NULL;
 
         for (k = l; *k; k++)
                 free(*k);
 
-        *l = NULL;
-}
-
-char **strv_free(char **l) {
-        strv_clear(l);
         return mfree(l);
 }
 

src/basic/strv.h

@@ -25,8 +25,6 @@ char **strv_free_erase(char **l);
 DEFINE_TRIVIAL_CLEANUP_FUNC(char**, strv_free_erase);
 #define _cleanup_strv_free_erase_ _cleanup_(strv_free_erasep)
 
-void strv_clear(char **l);
-
 char **strv_copy(char * const *l);
 size_t strv_length(char * const *l) _pure_;
 

src/basic/virt.c

@@ -20,6 +20,7 @@
 #include "string-util.h"
 #include "virt.h"
 
+#if defined(__i386__) || defined(__x86_64__)
 static const char *const vm_table[_VIRTUALIZATION_MAX] = {
         [VIRTUALIZATION_XEN]       = "XenVMMXenVMM",
         [VIRTUALIZATION_KVM]       = "KVMKVMKVM",
@@ -36,6 +37,7 @@ static const char *const vm_table[_VIRTUALIZATION_MAX] = {
 };
 
 DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING(vm, int);
+#endif
 
 static int detect_vm_cpuid(void) {
 

src/core/manager.c

@@ -4480,7 +4480,7 @@ static void manager_deserialize_uid_refs_one_internal(
 
         r = parse_uid(value, &uid);
         if (r < 0 || uid == 0) {
-                log_debug("Unable to parse UID reference serialization");
+                log_debug("Unable to parse UID reference serialization: " UID_FMT, uid);
                 return;
         }
 

src/core/service.c

@@ -650,24 +650,37 @@ static int service_add_default_dependencies(Service *s) {
         return unit_add_two_dependencies_by_name(UNIT(s), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, true, UNIT_DEPENDENCY_DEFAULT);
 }
 
-static void service_fix_output(Service *s) {
+static void service_fix_stdio(Service *s) {
         assert(s);
 
-        /* If nothing has been explicitly configured, patch default output in. If input is socket/tty we avoid this
+        /* Note that EXEC_INPUT_NULL and EXEC_OUTPUT_INHERIT play a special role here: they are both the
-         * however, since in that case we want output to default to the same place as we read input from. */
+         * default value that is subject to automatic overriding triggered by other settings and an explicit
-
+         * choice the user can make. We don't distuingish between these cases currently. */
-        if (s->exec_context.std_error == EXEC_OUTPUT_INHERIT &&
-            s->exec_context.std_output == EXEC_OUTPUT_INHERIT &&
-            s->exec_context.std_input == EXEC_INPUT_NULL)
-                s->exec_context.std_error = UNIT(s)->manager->default_std_error;
-
-        if (s->exec_context.std_output == EXEC_OUTPUT_INHERIT &&
-            s->exec_context.std_input == EXEC_INPUT_NULL)
-                s->exec_context.std_output = UNIT(s)->manager->default_std_output;
 
         if (s->exec_context.std_input == EXEC_INPUT_NULL &&
             s->exec_context.stdin_data_size > 0)
                 s->exec_context.std_input = EXEC_INPUT_DATA;
+
+        if (IN_SET(s->exec_context.std_input,
+                    EXEC_INPUT_TTY,
+                    EXEC_INPUT_TTY_FORCE,
+                    EXEC_INPUT_TTY_FAIL,
+                    EXEC_INPUT_SOCKET,
+                    EXEC_INPUT_NAMED_FD))
+                return;
+
+        /* We assume these listed inputs refer to bidirectional streams, and hence duplicating them from
+         * stdin to stdout/stderr makes sense and hence leaving EXEC_OUTPUT_INHERIT in place makes sense,
+         * too. Outputs such as regular files or sealed data memfds otoh don't really make sense to be
+         * duplicated for both input and output at the same time (since they then would cause a feedback
+         * loop), hence override EXEC_OUTPUT_INHERIT with the default stderr/stdout setting.  */
+
+        if (s->exec_context.std_error == EXEC_OUTPUT_INHERIT &&
+            s->exec_context.std_output == EXEC_OUTPUT_INHERIT)
+                s->exec_context.std_error = UNIT(s)->manager->default_std_error;
+
+        if (s->exec_context.std_output == EXEC_OUTPUT_INHERIT)
+                s->exec_context.std_output = UNIT(s)->manager->default_std_output;
 }
 
 static int service_setup_bus_name(Service *s) {
@@ -715,7 +728,7 @@ static int service_add_extras(Service *s) {
         if (s->type == SERVICE_ONESHOT && !s->start_timeout_defined)
                 s->timeout_start_usec = USEC_INFINITY;
 
-        service_fix_output(s);
+        service_fix_stdio(s);
 
         r = unit_patch_contexts(UNIT(s));
         if (r < 0)

src/core/unit.c

@@ -5066,14 +5066,21 @@ static void unit_unref_uid_internal(
         *ref_uid = UID_INVALID;
 }
 
-void unit_unref_uid(Unit *u, bool destroy_now) {
+static void unit_unref_uid(Unit *u, bool destroy_now) {
         unit_unref_uid_internal(u, &u->ref_uid, destroy_now, manager_unref_uid);
 }
 
-void unit_unref_gid(Unit *u, bool destroy_now) {
+static void unit_unref_gid(Unit *u, bool destroy_now) {
         unit_unref_uid_internal(u, (uid_t*) &u->ref_gid, destroy_now, manager_unref_gid);
 }
 
+void unit_unref_uid_gid(Unit *u, bool destroy_now) {
+        assert(u);
+
+        unit_unref_uid(u, destroy_now);
+        unit_unref_gid(u, destroy_now);
+}
+
 static int unit_ref_uid_internal(
                 Unit *u,
                 uid_t *ref_uid,
@@ -5112,11 +5119,11 @@ static int unit_ref_uid_internal(
         return 1;
 }
 
-int unit_ref_uid(Unit *u, uid_t uid, bool clean_ipc) {
+static int unit_ref_uid(Unit *u, uid_t uid, bool clean_ipc) {
         return unit_ref_uid_internal(u, &u->ref_uid, uid, clean_ipc, manager_ref_uid);
 }
 
-int unit_ref_gid(Unit *u, gid_t gid, bool clean_ipc) {
+static int unit_ref_gid(Unit *u, gid_t gid, bool clean_ipc) {
         return unit_ref_uid_internal(u, (uid_t*) &u->ref_gid, (uid_t) gid, clean_ipc, manager_ref_gid);
 }
 
@@ -5161,13 +5168,6 @@ int unit_ref_uid_gid(Unit *u, uid_t uid, gid_t gid) {
         return r;
 }
 
-void unit_unref_uid_gid(Unit *u, bool destroy_now) {
-        assert(u);
-
-        unit_unref_uid(u, destroy_now);
-        unit_unref_gid(u, destroy_now);
-}
-
 void unit_notify_user_lookup(Unit *u, uid_t uid, gid_t gid) {
         int r;
 

src/core/unit.h

@@ -815,12 +815,6 @@ int unit_fail_if_noncanonical(Unit *u, const char* where);
 
 int unit_test_start_limit(Unit *u);
 
-void unit_unref_uid(Unit *u, bool destroy_now);
-int unit_ref_uid(Unit *u, uid_t uid, bool clean_ipc);
-
-void unit_unref_gid(Unit *u, bool destroy_now);
-int unit_ref_gid(Unit *u, gid_t gid, bool clean_ipc);
-
 int unit_ref_uid_gid(Unit *u, uid_t uid, gid_t gid);
 void unit_unref_uid_gid(Unit *u, bool destroy_now);
 

src/journal/sd-journal.c

@@ -161,7 +161,7 @@ static int match_is_valid(const void *data, size_t size) {
         if (size < 2)
                 return false;
 
-        if (startswith(data, "__"))
+        if (((char*) data)[0] == '_' && ((char*) data)[1] == '_')
                 return false;
 
         b = data;

src/libsystemd/sd-hwdb/hwdb-util.c

@@ -446,7 +446,7 @@ static int insert_data(struct trie *trie, char **match_list, char *line, const c
 
         value = strchr(line, '=');
         if (!value)
-                return log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL,
+                return log_syntax(NULL, LOG_WARNING, filename, line_number, SYNTHETIC_ERRNO(EINVAL),
                                   "Key-value pair expected but got \"%s\", ignoring", line);
 
         value[0] = '\0';
@@ -457,7 +457,7 @@ static int insert_data(struct trie *trie, char **match_list, char *line, const c
                 line++;
 
         if (isempty(line + 1) || isempty(value))
-                return log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL,
+                return log_syntax(NULL, LOG_WARNING, filename, line_number, SYNTHETIC_ERRNO(EINVAL),
                                   "Empty %s in \"%s=%s\", ignoring",
                                   isempty(line + 1) ? "key" : "value",
                                   line, value);
@@ -477,7 +477,6 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr
         _cleanup_fclose_ FILE *f = NULL;
         _cleanup_strv_free_ char **match_list = NULL;
         uint32_t line_number = 0;
-        char *match = NULL;
         int r = 0, err;
 
         f = fopen(filename, "re");
@@ -518,20 +517,15 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr
                                 break;
 
                         if (line[0] == ' ') {
-                                log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL,
+                                r = log_syntax(NULL, LOG_WARNING, filename, line_number, SYNTHETIC_ERRNO(EINVAL),
-                                           "Match expected but got indented property \"%s\", ignoring line", line);
+                                               "Match expected but got indented property \"%s\", ignoring line", line);
-                                r = -EINVAL;
                                 break;
                         }
 
                         /* start of record, first match */
                         state = HW_MATCH;
 
-                        match = strdup(line);
+                        err = strv_extend(&match_list, line);
-                        if (!match)
-                                return -ENOMEM;
-
-                        err = strv_consume(&match_list, match);
                         if (err < 0)
                                 return err;
 
@@ -539,21 +533,16 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr
 
                 case HW_MATCH:
                         if (len == 0) {
-                                log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL,
+                                r = log_syntax(NULL, LOG_WARNING, filename, line_number, SYNTHETIC_ERRNO(EINVAL),
-                                           "Property expected, ignoring record with no properties");
+                                               "Property expected, ignoring record with no properties");
-                                r = -EINVAL;
                                 state = HW_NONE;
-                                strv_clear(match_list);
+                                match_list = strv_free(match_list);
                                 break;
                         }
 
                         if (line[0] != ' ') {
                                 /* another match */
-                                match = strdup(line);
+                                err = strv_extend(&match_list, line);
-                                if (!match)
-                                        return -ENOMEM;
-
-                                err = strv_consume(&match_list, match);
                                 if (err < 0)
                                         return err;
 
@@ -571,16 +560,15 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr
                         if (len == 0) {
                                 /* end of record */
                                 state = HW_NONE;
-                                strv_clear(match_list);
+                                match_list = strv_free(match_list);
                                 break;
                         }
 
                         if (line[0] != ' ') {
-                                log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL,
+                                r = log_syntax(NULL, LOG_WARNING, filename, line_number, SYNTHETIC_ERRNO(EINVAL),
-                                           "Property or empty line expected, got \"%s\", ignoring record", line);
+                                               "Property or empty line expected, got \"%s\", ignoring record", line);
-                                r = -EINVAL;
                                 state = HW_NONE;
-                                strv_clear(match_list);
+                                match_list = strv_free(match_list);
                                 break;
                         }
 
@@ -592,7 +580,7 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr
         }
 
         if (state == HW_MATCH)
-                log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL,
+                log_syntax(NULL, LOG_WARNING, filename, line_number, 0,
                            "Property expected, ignoring record with no properties");
 
         return r;

src/resolve/resolved-dns-trust-anchor.c

@@ -20,11 +20,6 @@
 
 static const char trust_anchor_dirs[] = CONF_PATHS_NULSTR("dnssec-trust-anchors.d");
 
-/* The first DS RR from https://data.iana.org/root-anchors/root-anchors.xml, retrieved December 2015 */
-static const uint8_t root_digest1[] =
-        { 0x49, 0xAA, 0xC1, 0x1D, 0x7B, 0x6F, 0x64, 0x46, 0x70, 0x2E, 0x54, 0xA1, 0x60, 0x73, 0x71, 0x60,
-          0x7A, 0x1A, 0x41, 0x85, 0x52, 0x00, 0xFD, 0x2C, 0xE1, 0xCD, 0xDE, 0x32, 0xF2, 0x4E, 0x8F, 0xB5 };
-
 /* The second DS RR from https://data.iana.org/root-anchors/root-anchors.xml, retrieved February 2017 */
 static const uint8_t root_digest2[] =
         { 0xE0, 0x6D, 0x44, 0xB8, 0x0B, 0x8F, 0x1D, 0x39, 0xA9, 0x5C, 0x0B, 0x0D, 0x7C, 0x65, 0xD0, 0x84,
@@ -96,11 +91,7 @@ static int dns_trust_anchor_add_builtin_positive(DnsTrustAnchor *d) {
         if (!answer)
                 return -ENOMEM;
 
-        /* Add the two RRs from https://data.iana.org/root-anchors/root-anchors.xml */
+        /* Add the currently valid RRs from https://data.iana.org/root-anchors/root-anchors.xml */
-        r = add_root_ksk(answer, key, 19036, DNSSEC_ALGORITHM_RSASHA256, DNSSEC_DIGEST_SHA256, root_digest1, sizeof(root_digest1));
-        if (r < 0)
-                return r;
-
         r = add_root_ksk(answer, key, 20326, DNSSEC_ALGORITHM_RSASHA256, DNSSEC_DIGEST_SHA256, root_digest2, sizeof(root_digest2));
         if (r < 0)
                 return r;

src/shared/json.c

@@ -26,11 +26,17 @@
 #include "user-util.h"
 #include "utf8.h"
 
-/* Refuse putting together variants with a larger depth than 4K by default (as a protection against overflowing stacks
+/* Refuse putting together variants with a larger depth than 2K by default (as a protection against overflowing stacks
  * if code processes JSON objects recursively. Note that we store the depth in an uint16_t, hence make sure this
  * remains under 2^16.
- * The value was 16k, but it was discovered to be too high on llvm/x86-64. See also the issue #10738. */
+ *
-#define DEPTH_MAX (4U*1024U)
+ * The value first was 16k, but it was discovered to be too high on llvm/x86-64. See also:
+ * https://github.com/systemd/systemd/issues/10738
+ *
+ * The value then was 4k, but it was discovered to be too high on s390x/aarch64. See also:
+ * https://github.com/systemd/systemd/issues/14396 */
+
+#define DEPTH_MAX (2U*1024U)
 assert_cc(DEPTH_MAX <= UINT16_MAX);
 
 typedef struct JsonSource {

src/test/test-execute.c

@@ -756,6 +756,7 @@ static void test_exec_specifier(Manager *m) {
 static void test_exec_standardinput(Manager *m) {
         test(__func__, m, "exec-standardinput-data.service", 0, CLD_EXITED);
         test(__func__, m, "exec-standardinput-file.service", 0, CLD_EXITED);
+        test(__func__, m, "exec-standardinput-file-cat.service", 0, CLD_EXITED);
 }
 
 static void test_exec_standardoutput(Manager *m) {
@@ -786,6 +787,7 @@ static int run_tests(UnitFileScope scope, const test_entry tests[], char **patte
         assert_se(tests);
 
         r = manager_new(scope, MANAGER_TEST_RUN_BASIC, &m);
+        m->default_std_output = EXEC_OUTPUT_NULL; /* don't rely on host journald */
         if (manager_errno_skip_test(r))
                 return log_tests_skipped_errno(r, "manager_new");
         assert_se(r >= 0);

src/timesync/timesyncd-manager.c

@@ -514,7 +514,7 @@ static int manager_receive_response(sd_event_source *source, int fd, uint32_t re
 
         root_distance = ntp_ts_short_to_d(&ntpmsg.root_delay) / 2 + ntp_ts_short_to_d(&ntpmsg.root_dispersion);
         if (root_distance > (double) m->max_root_distance_usec / (double) USEC_PER_SEC) {
-                log_debug("Server has too large root distance. Disconnecting.");
+                log_info("Server has too large root distance. Disconnecting.");
                 return manager_connect(m);
         }
 

test/meson.build

@@ -138,6 +138,7 @@ test_data_files = '''
         test-execute/exec-specifier@.service
         test-execute/exec-standardinput-data.service
         test-execute/exec-standardinput-file.service
+        test-execute/exec-standardinput-file-cat.service
         test-execute/exec-standardoutput-file.service
         test-execute/exec-standardoutput-append.service
         test-execute/exec-supplementarygroups-multiple-groups-default-group-user.service

test/test-execute/exec-standardinput-file-cat.service

@@ -0,0 +1,9 @@
+[Unit]
+Description=Test for StandardInput=file:
+
+[Service]
+ExecStart=cat
+Type=oneshot
+StandardInput=file:/etc/os-release
+# We leave StandardOutput= unset here, to verify https://github.com/systemd/systemd/issues/14560 works
+# The "cat" tool is going to write to stdout, which fails if we dup() stdin to stdout