[systemd-devel] [RFC 3/4] monitor for utab changes with inotify

[Chris Leech]

Parsing the mount table with libmount races against the mount command,
which will handle the actual mounting before updating utab.  This means
the poll event on /proc/self/mountinfo can kick of a reparse in systemd
before the utab information is available.

This change adds in an additional event source using inotify to watch
for changes to utab.  It only watches for IN_MOVED_TO events, matching
libmount behavior of always overwriting this file using rename(2).

This does add a second pass through the mount table parsing when utab is
updated.
---
 src/core/manager.c |  2 +-
 src/core/manager.h |  2 ++
 src/core/mount.c   | 57 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index 129f6dd..7362149 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -554,7 +554,7 @@ int manager_new(SystemdRunningAs running_as, bool test_run, Manager **_m) {
 
         m->idle_pipe[0] = m->idle_pipe[1] = m->idle_pipe[2] = m->idle_pipe[3] = -1;
 
-        m->pin_cgroupfs_fd = m->notify_fd = m->signal_fd = m->time_change_fd = m->dev_autofs_fd = m->private_listen_fd = m->kdbus_fd = -1;
+        m->pin_cgroupfs_fd = m->notify_fd = m->signal_fd = m->time_change_fd = m->dev_autofs_fd = m->private_listen_fd = m->kdbus_fd = m->utab_inotify_fd = -1;
         m->current_job_id = 1; /* start as id #1, so that we can leave #0 around as "null-like" value */
 
         m->ask_password_inotify_fd = -1;
diff --git a/src/core/manager.h b/src/core/manager.h
index ab72548..bdeea14 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -182,6 +182,8 @@ struct Manager {
         /* Data specific to the mount subsystem */
         FILE *proc_self_mountinfo;
         sd_event_source *mount_event_source;
+        int utab_inotify_fd;
+        sd_event_source *mount_utab_event_source;
 
         /* Data specific to the swap filesystem */
         FILE *proc_swaps;
diff --git a/src/core/mount.c b/src/core/mount.c
index 2bc7b14..7d77021 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -26,9 +26,8 @@
 #include <signal.h>
 
 #ifdef HAVE_LIBMOUNT
+#include <sys/inotify.h>
 #include <libmount.h>
-#else
-#define mnt_init_debug(m) do {} while (0)
 #endif
 
 #include "manager.h"
@@ -1615,11 +1614,13 @@ static void mount_shutdown(Manager *m) {
         assert(m);
 
         m->mount_event_source = sd_event_source_unref(m->mount_event_source);
+        m->mount_utab_event_source = sd_event_source_unref(m->mount_utab_event_source);
 
         if (m->proc_self_mountinfo) {
                 fclose(m->proc_self_mountinfo);
                 m->proc_self_mountinfo = NULL;
         }
+        m->utab_inotify_fd = safe_close(m->utab_inotify_fd);
 }
 
 static int mount_get_timeout(Unit *u, uint64_t *timeout) {
@@ -1640,8 +1641,6 @@ static int mount_enumerate(Manager *m) {
         int r;
         assert(m);
 
-        mnt_init_debug(0);
-
         if (!m->proc_self_mountinfo) {
                 m->proc_self_mountinfo = fopen("/proc/self/mountinfo", "re");
                 if (!m->proc_self_mountinfo)
@@ -1658,6 +1657,27 @@ static int mount_enumerate(Manager *m) {
                 if (r < 0)
                         goto fail;
         }
+#ifdef HAVE_LIBMOUNT
+        mnt_init_debug(0);
+
+        if (m->utab_inotify_fd < 0) {
+                m->utab_inotify_fd = inotify_init1(IN_NONBLOCK|IN_CLOEXEC);
+                if (m->utab_inotify_fd < 0)
+                        goto fail_with_errno;
+
+                r = inotify_add_watch(m->utab_inotify_fd, "/run/mount", IN_MOVED_TO);
+                if (r < 0)
+                        goto fail_with_errno;
+
+                r = sd_event_add_io(m->event, &m->mount_utab_event_source, m->utab_inotify_fd, EPOLLIN, mount_dispatch_io, m);
+                if (r < 0)
+                        goto fail;
+
+                r = sd_event_source_set_priority(m->mount_utab_event_source, -10);
+                if (r < 0)
+                        goto fail;
+        }
+#endif
 
         r = mount_load_proc_self_mountinfo(m, false);
         if (r < 0)
@@ -1665,6 +1685,8 @@ static int mount_enumerate(Manager *m) {
 
         return 0;
 
+fail_with_errno:
+        r = -errno;
 fail:
         mount_shutdown(m);
         return r;
@@ -1676,12 +1698,37 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents,
         int r;
 
         assert(m);
-        assert(revents & EPOLLPRI);
+        assert(revents & (EPOLLPRI | EPOLLIN));
 
         /* The manager calls this for every fd event happening on the
          * /proc/self/mountinfo file, which informs us about mounting
          * table changes */
 
+#ifdef HAVE_LIBMOUNT
+        /* This may also be called for /run/mount events */
+        if (fd == m->utab_inotify_fd) {
+                char inotify_buffer[sizeof(struct inotify_event) + NAME_MAX + 1];
+                struct inotify_event *event;
+                char *p;
+                int rescan = 0;
+
+                while ((r = read(fd, inotify_buffer, sizeof(inotify_buffer))) > 0) {
+                        for (p = inotify_buffer; p < inotify_buffer + r; ) {
+                                event = (struct inotify_event *) p;
+                                /* only care about changes to utab, but we have
+                                 * to monitor the directory to reliably get
+                                 * notifications about when utab is replaced
+                                 * using rename(2) */
+                                if (strcmp(event->name, "utab") == 0)
+                                        rescan = 1;
+                                p += sizeof(struct inotify_event) + event->len;
+                        }
+                }
+                if (!rescan)
+                        return 0;
+        }
+#endif
+
         r = mount_load_proc_self_mountinfo(m, true);
         if (r < 0) {
                 log_error("Failed to reread /proc/self/mountinfo: %s", strerror(-r));
-- 
1.9.3