time-wait-sync: use watchfile to coordinate with timesyncd

Systems that have an accurate real-time clock may have an initial
unsynchronized time that is close enough to the synchronized time that
the final adjustment doesn't trigger a waking "clock set" event.  Have
timesyncd touch a file in its runtime directory as a secondary signal
for synchronization.  Continue to support the timerfd-based trigger as a
sufficient condition when the watchfile is not present.

Closes issue #8683

1 files changed, 127 insertions, 35 deletions

diff --git a/src/time-wait-sync/time-wait-sync.c b/src/time-wait-sync/time-wait-sync.c
index 9b558532ce..198c055650 100644
--- a/src/time-wait-sync/time-wait-sync.c
+++ b/src/time-wait-sync/time-wait-sync.c
@@ -25,6 +25,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/inotify.h>
 #include <sys/timerfd.h>
 #include <sys/timex.h>
 #include <unistd.h>
@@ -32,32 +33,88 @@
 #include "sd-event.h"
 
 #include "fd-util.h"
+#include "fs-util.h"
 #include "missing.h"
 #include "signal-util.h"
 #include "time-util.h"
 
 typedef struct ClockState {
-        int fd;                        /* non-negative is descriptor from timerfd_create */
-        int adjtime_state;             /* return value from last adjtimex(2) call */
-        sd_event_source *event_source; /* non-null is the active io event source */
+        int timerfd_fd;                  /* non-negative is descriptor from timerfd_create */
+        int adjtime_state;               /* return value from last adjtimex(2) call */
+        sd_event_source *timerfd_event_source; /* non-null is the active io event source */
+        int inotify_fd;
+        sd_event_source *inotify_event_source;
+        int run_systemd_wd;
+        int run_systemd_timesync_wd;
+        bool has_watchfile;
 } ClockState;
 
+static void clock_state_release_timerfd(ClockState *sp) {
+        sp->timerfd_event_source = sd_event_source_unref(sp->timerfd_event_source);
+        sp->timerfd_fd = safe_close(sp->timerfd_fd);
+}
+
 static void clock_state_release(ClockState *sp) {
-        sp->event_source = sd_event_source_unref(sp->event_source);
-        sp->fd = safe_close(sp->fd);
+        clock_state_release_timerfd(sp);
+        sp->inotify_event_source = sd_event_source_unref(sp->inotify_event_source);
+        sp->inotify_fd = safe_close(sp->inotify_fd);
 }
 
 static int clock_state_update(ClockState *sp, sd_event *event);
 
-static int io_handler(sd_event_source * s,
-                      int fd,
-                      uint32_t revents,
-                      void *userdata) {
+static int update_notify_run_systemd_timesync(ClockState *sp) {
+        sp->run_systemd_timesync_wd = inotify_add_watch(sp->inotify_fd, "/run/systemd/timesync", IN_CREATE|IN_DELETE_SELF);
+        return sp->run_systemd_timesync_wd;
+}
+
+static int timerfd_handler(sd_event_source *s,
+                           int fd,
+                           uint32_t revents,
+                           void *userdata) {
         ClockState *sp = userdata;
 
         return clock_state_update(sp, sd_event_source_get_event(s));
 }
 
+static void process_inotify_event(sd_event *event, ClockState *sp, struct inotify_event *e) {
+        if (e->wd == sp->run_systemd_wd) {
+                /* Only thing we care about is seeing if we can start watching /run/systemd/timesync. */
+                if (sp->run_systemd_timesync_wd < 0)
+                        update_notify_run_systemd_timesync(sp);
+        } else if (e->wd == sp->run_systemd_timesync_wd) {
+                if (e->mask & IN_DELETE_SELF) {
+                        /* Somebody removed /run/systemd/timesync. */
+                        (void) inotify_rm_watch(sp->inotify_fd, sp->run_systemd_timesync_wd);
+                        sp->run_systemd_timesync_wd = -1;
+                } else
+                        /* Somebody might have created /run/systemd/timesync/synchronized. */
+                        clock_state_update(sp, event);
+        }
+}
+
+static int inotify_handler(sd_event_source *s,
+                           int fd,
+                           uint32_t revents,
+                           void *userdata) {
+        sd_event *event = sd_event_source_get_event(s);
+        ClockState *sp = userdata;
+        union inotify_event_buffer buffer;
+        struct inotify_event *e;
+        ssize_t l;
+
+        l = read(fd, &buffer, sizeof(buffer));
+        if (l < 0) {
+                if (IN_SET(errno, EAGAIN, EINTR))
+                        return 0;
+
+                return log_warning_errno(errno, "Lost access to inotify: %m");
+        }
+        FOREACH_INOTIFY_EVENT(e, buffer, l)
+                process_inotify_event(event, sp, e);
+
+        return 0;
+}
+
 static int clock_state_update(ClockState *sp,
                               sd_event *event) {
         static const struct itimerspec its = {
@@ -69,34 +126,39 @@ static int clock_state_update(ClockState *sp,
         usec_t t;
         const char * ts;
 
-        clock_state_release(sp);
+        clock_state_release_timerfd(sp);
 
         /* The kernel supports cancelling timers whenever its realtime clock is "set" (which can happen in a variety of
-         * ways, generally adjustments of at least 500 ms). The way this module works is we set up a timer that will
+         * ways, generally adjustments of at least 500 ms). The way this module works is we set up a timerfd that will
          * wake when the clock is set, and when that happens we read the clock synchronization state from the return
          * value of adjtimex(2), which supports the NTP time adjustment protocol.
          *
          * The kernel determines whether the clock is synchronized using driver-specific tests, based on time
-         * information passed by an application, generally through adjtimex(2). If the application asserts the clock
-         * is synchronized, but does not also do something that "sets the clock", the timer will not be cancelled and
-         * synchronization will not be detected. Should this behavior be observed with a time synchronization provider
-         * this code might be reworked to do a periodic check as well.
+         * information passed by an application, generally through adjtimex(2). If the application asserts the clock is
+         * synchronized, but does not also do something that "sets the clock", the timer will not be cancelled and
+         * synchronization will not be detected.
          *
          * Similarly, this service will never complete if the application sets the time without also providing
-         * information that adjtimex(2) can use to determine that the clock is synchronized.
+         * information that adjtimex(2) can use to determine that the clock is synchronized. This generally doesn't
+         * happen, but can if the system has a hardware clock that is accurate enough that the adjustment is too small
+         * to be a "set".
+         *
+         * Both these failure-to-detect situations are covered by having the presence/creation of
+         * /run/systemd/timesync/synchronized, which is considered sufficient to indicate a synchronized clock even if
+         * the kernel has not been updated.
          *
-         * Well-behaved implementations including systemd-timesyncd should not produce either situation. For timesyncd
-         * the initial setting of the time uses settimeofday(2), which sets the clock but does not mark it
-         * synchronized. When an NTP source is selected it sets the clock again with clock_adjtime(2) which does mark
-         * it synchronized. */
+         * For timesyncd the initial setting of the time uses settimeofday(2), which sets the clock but does not mark
+         * it synchronized. When an NTP source is selected it sets the clock again with clock_adjtime(2) which marks it
+         * synchronized and also touches /run/systemd/timesync/synchronized which covers the case when the clock wasn't
+         * "set". */
         r = timerfd_create(CLOCK_REALTIME, TFD_NONBLOCK | TFD_CLOEXEC);
         if (r < 0) {
                 log_error_errno(errno, "Failed to create timerfd: %m");
                 goto finish;
         }
-        sp->fd = r;
+        sp->timerfd_fd = r;
 
-        r = timerfd_settime(sp->fd, TFD_TIMER_ABSTIME | TFD_TIMER_CANCEL_ON_SET, &its, NULL);
+        r = timerfd_settime(sp->timerfd_fd, TFD_TIMER_ABSTIME | TFD_TIMER_CANCEL_ON_SET, &its, NULL);
         if (r < 0) {
                 log_error_errno(errno, "Failed to set timerfd conditions: %m");
                 goto finish;
@@ -117,24 +179,26 @@ static int clock_state_update(ClockState *sp,
                 strcpy(buf, "unrepresentable");
         log_info("adjtime state %d status %x time %s", sp->adjtime_state, tx.status, ts);
 
-        if (sp->adjtime_state == TIME_ERROR) {
-                /* Not synchronized. Do a one-shot wait on the descriptor and inform the caller we need to keep
+        sp->has_watchfile = access("/run/systemd/timesync/synchronized", F_OK) >= 0;
+        if (sp->has_watchfile)
+                /* Presence of watch file overrides adjtime_state */
+                r = 0;
+        else if (sp->adjtime_state == TIME_ERROR) {
+                /* Not synchronized.  Do a one-shot wait on the descriptor and inform the caller we need to keep
                  * running. */
-                r = sd_event_add_io(event, &sp->event_source, sp->fd,
-                                    EPOLLIN, io_handler, sp);
+                r = sd_event_add_io(event, &sp->timerfd_event_source, sp->timerfd_fd,
+                                    EPOLLIN, timerfd_handler, sp);
                 if (r < 0) {
                         log_error_errno(r, "Failed to create time change monitor source: %m");
                         goto finish;
                 }
                 r = 1;
-        } else {
+        } else
                 /* Synchronized; we can exit. */
-                (void) sd_event_exit(event, 0);
                 r = 0;
-        }
 
  finish:
-        if (r < 0)
+        if (r <= 0)
                 (void) sd_event_exit(event, r);
         return r;
 }
@@ -143,7 +207,10 @@ int main(int argc, char * argv[]) {
         int r;
         _cleanup_(sd_event_unrefp) sd_event *event;
         ClockState state = {
-                .fd = -1,
+                .timerfd_fd = -1,
+                .inotify_fd = -1,
+                .run_systemd_wd = -1,
+                .run_systemd_timesync_wd = -1,
         };
 
         assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGTERM, SIGINT, -1) >= 0);
@@ -172,17 +239,42 @@ int main(int argc, char * argv[]) {
                 goto finish;
         }
 
+        r = inotify_init1(IN_NONBLOCK|IN_CLOEXEC);
+        if (r < 0) {
+                log_error_errno(errno, "Failed to create inotify descriptor: %m");
+                goto finish;
+        }
+        state.inotify_fd = r;
+
+        r = sd_event_add_io(event, &state.inotify_event_source, state.inotify_fd,
+                            EPOLLIN, inotify_handler, &state);
+        if (r < 0) {
+                log_error_errno(r, "Failed to create notify event source: %m");
+                goto finish;
+        }
+
+        r = inotify_add_watch(state.inotify_fd, "/run/systemd/", IN_CREATE);
+        if (r < 0) {
+                log_error_errno(errno, "Failed to watch /run/systemd/: %m");
+                goto finish;
+        }
+        state.run_systemd_wd = r;
+
+        (void) update_notify_run_systemd_timesync(&state);
+
         r = clock_state_update(&state, event);
         if (r > 0) {
                 r = sd_event_loop(event);
                 if (r < 0)
                         log_error_errno(r, "Failed in event loop: %m");
-                else if (state.adjtime_state == TIME_ERROR) {
-                        log_error("Event loop terminated without synchronizing");
-                        r = -ECANCELED;
-                }
         }
 
+        if (state.has_watchfile)
+                log_debug("Exit enabled by: /run/systemd/timesync/synchonized");
+
+        if (state.adjtime_state == TIME_ERROR)
+                log_info("Exit without adjtimex synchronized.");
+
  finish:
         clock_state_release(&state);
         return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;