update-done, condition: write the timestamp to the file as well and use it to prevent false-positives

This fixes https://bugs.freedesktop.org/show_bug.cgi?id=90192 and #4130
for real. Also, remove timestamp check in update-done.c altogether since
the whole operation is idempotent.

2 files changed, 73 insertions, 45 deletions

diff --git a/src/shared/condition.c b/src/shared/condition.c
index 6bb42c0692..f13fa6a9fd 100644
--- a/src/shared/condition.c
+++ b/src/shared/condition.c
@@ -37,6 +37,7 @@
 #include "condition.h"
 #include "extract-word.h"
 #include "fd-util.h"
+#include "fileio.h"
 #include "glob-util.h"
 #include "hostname-util.h"
 #include "ima-util.h"
@@ -309,8 +310,45 @@ static int condition_test_needs_update(Condition *c) {
         if (lstat("/usr/", &usr) < 0)
                 return true;
 
-        return usr.st_mtim.tv_sec > other.st_mtim.tv_sec ||
-                (usr.st_mtim.tv_sec == other.st_mtim.tv_sec && usr.st_mtim.tv_nsec > other.st_mtim.tv_nsec);
+        /*
+         * First, compare seconds as they are always accurate...
+         */
+        if (usr.st_mtim.tv_sec != other.st_mtim.tv_sec)
+                return usr.st_mtim.tv_sec > other.st_mtim.tv_sec;
+
+        /*
+         * ...then compare nanoseconds.
+         *
+         * A false positive is only possible when /usr's nanoseconds > 0
+         * (otherwise /usr cannot be strictly newer than the target file)
+         * AND the target file's nanoseconds == 0
+         * (otherwise the filesystem supports nsec timestamps, see stat(2)).
+         */
+        if (usr.st_mtim.tv_nsec > 0 && other.st_mtim.tv_nsec == 0) {
+                _cleanup_free_ char *timestamp_str = NULL;
+                uint64_t timestamp;
+                int r;
+
+                r = parse_env_file(p, NULL, "TimestampNSec", &timestamp_str, NULL);
+                if (r < 0) {
+                        log_error_errno(-r, "Failed to parse timestamp file '%s', using mtime: %m", p);
+                        return true;
+                } else if (r == 0) {
+                        log_debug("No data in timestamp file '%s', using mtime", p);
+                        return true;
+                }
+
+                r = safe_atou64(timestamp_str, &timestamp);
+                if (r < 0) {
+                        log_error_errno(-r, "Failed to parse timestamp value '%s' in file '%s', using mtime: %m",
+                                        timestamp_str, p);
+                        return true;
+                }
+
+                other.st_mtim.tv_nsec = timestamp % NSEC_PER_SEC;
+        }
+
+        return usr.st_mtim.tv_nsec > other.st_mtim.tv_nsec;
 }
 
 static int condition_test_first_boot(Condition *c) {
diff --git a/src/update-done/update-done.c b/src/update-done/update-done.c
index da306a4444..5cc5abfddf 100644
--- a/src/update-done/update-done.c
+++ b/src/update-done/update-done.c
@@ -23,67 +23,57 @@
 #include "util.h"
 
 #define MESSAGE                                                         \
-        "This file was created by systemd-update-done. Its only \n"     \
-        "purpose is to hold a timestamp of the time this directory\n"   \
-        "was updated. See systemd-update-done.service(8).\n"
+        "# This file was created by systemd-update-done. Its only \n"   \
+        "# purpose is to hold a timestamp of the time this directory\n" \
+        "# was updated. See systemd-update-done.service(8).\n"
 
 static int apply_timestamp(const char *path, struct timespec *ts) {
         struct timespec twice[2] = {
                 *ts,
                 *ts
         };
-        struct stat st;
+        int fd = -1;
+        _cleanup_fclose_ FILE *f = NULL;
+        int r;
 
         assert(path);
         assert(ts);
 
-        if (stat(path, &st) >= 0) {
-                /* Is the timestamp file already newer than the OS? If
-                 * so, there's nothing to do. We ignore the nanosecond
-                 * component of the timestamp, since some file systems
-                 * do not support any better accuracy than 1s and we
-                 * have no way to identify the accuracy
-                 * available. Most notably ext4 on small disks (where
-                 * 128 byte inodes are used) does not support better
-                 * accuracy than 1s. */
-                if (st.st_mtim.tv_sec > ts->tv_sec)
-                        return 0;
-
-                /* It is older? Then let's update it */
-                if (utimensat(AT_FDCWD, path, twice, AT_SYMLINK_NOFOLLOW) < 0) {
-
-                        if (errno == EROFS)
-                                return log_debug("Can't update timestamp file %s, file system is read-only.", path);
+        /*
+         * We store the timestamp both as mtime of the file and in the file itself,
+         * to support filesystems which cannot store nanosecond-precision timestamps.
+         * Hence, don't bother updating the file, let's just rewrite it.
+         */
 
-                        return log_error_errno(errno, "Failed to update timestamp on %s: %m", path);
-                }
+        r = mac_selinux_create_file_prepare(path, S_IFREG);
+        if (r < 0)
+                return log_error_errno(r, "Failed to set SELinux context for %s: %m", path);
 
-        } else if (errno == ENOENT) {
-                _cleanup_close_ int fd = -1;
-                int r;
+        fd = open(path, O_CREAT|O_WRONLY|O_TRUNC|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW, 0644);
+        mac_selinux_create_file_clear();
 
-                /* The timestamp file doesn't exist yet? Then let's create it. */
+        if (fd < 0) {
+                if (errno == EROFS)
+                        return log_debug("Can't create timestamp file %s, file system is read-only.", path);
 
-                r = mac_selinux_create_file_prepare(path, S_IFREG);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to set SELinux context for %s: %m", path);
-
-                fd = open(path, O_CREAT|O_EXCL|O_WRONLY|O_TRUNC|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW, 0644);
-                mac_selinux_create_file_clear();
+                return log_error_errno(errno, "Failed to create/open timestamp file %s: %m", path);
+        }
 
-                if (fd < 0) {
-                        if (errno == EROFS)
-                                return log_debug("Can't create timestamp file %s, file system is read-only.", path);
+        f = fdopen(fd, "w");
+        if (!f) {
+                safe_close(fd);
+                return log_error_errno(errno, "Failed to fdopen() timestamp file %s: %m", path);
+        }
 
-                        return log_error_errno(errno, "Failed to create timestamp file %s: %m", path);
-                }
+        (void) fprintf(f,
+                       "%s"
+                       "TimestampNSec=" NSEC_FMT "\n",
+                       MESSAGE, timespec_load_nsec(ts));
 
-                (void) loop_write(fd, MESSAGE, strlen(MESSAGE), false);
+        fflush(f);
 
-                if (futimens(fd, twice) < 0)
-                        return log_error_errno(errno, "Failed to update timestamp on %s: %m", path);
-        } else
-                log_error_errno(errno, "Failed to stat() timestamp file %s: %m", path);
+        if (futimens(fd, twice) < 0)
+                return log_error_errno(errno, "Failed to update timestamp on %s: %m", path);
 
         return 0;
 }