shutdown: use libmount to enumerate /proc/self/mountinfo

This is analogous to 8d3ae2bd4c9bf9fc2e57f7b3776325a1c750ca30, except that now
src/core/umount.c not src/core/mount.c is converted.

Might help with https://bugzilla.redhat.com/show_bug.cgi?id=1554943, or not.

In the patch, mnt_free_tablep and mnt_free_iterp are declared twice. It'd
be nicer to define them just once in mount-setup.h, but then libmount.h would
have to be included there. libmount.h seems to be buggy, and declares some
defines which break other headers, and working around this is more pain than
the two duplicate lines. So let's live with the duplication for now.

This fixes memleak of MountPoint in mount_points_list_get() on error, not that
it matters any.

diff --git a/meson.build b/meson.build
index 56e3d0086e0d4005e0ca49f3c4b50d88d764d0ff..aac783fb991317c8c0c1867d0553c551e5723042 100644 (file)
--- a/meson.build
+++ b/meson.build
@@ -2404,6 +2404,7 @@ executable('systemd-shutdown',
            systemd_shutdown_sources,
            include_directories : includes,
            link_with : [libshared],
+           dependencies : [libmount],
            install_rpath : rootlibexecdir,
            install : true,
            install_dir : rootlibexecdir)

diff --git a/src/core/mount.c b/src/core/mount.c
index cfe8ec9044ae1e34b8ee867ddeb6293a003bc7ce..0e755da5c0234bfa81cb54c84881c1608cb5df11 100644 (file)
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -1608,11 +1608,8 @@ static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) {
         assert(m);
 
         t = mnt_new_table();
-        if (!t)
-                return log_oom();
-
         i = mnt_new_iter(MNT_ITER_FORWARD);
-        if (!i)
+        if (!t || !i)
                 return log_oom();
 
         r = mnt_table_parse_mtab(t, NULL);
@@ -1621,9 +1618,9 @@ static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) {
 
         r = 0;
         for (;;) {
+                struct libmnt_fs *fs;
                 const char *device, *path, *options, *fstype;
                 _cleanup_free_ char *d = NULL, *p = NULL;
-                struct libmnt_fs *fs;
                 int k;
 
                 k = mnt_table_next_fs(t, i, &fs);

diff --git a/src/core/umount.c b/src/core/umount.c
index ab2d4ffa6258dc10c94f7a1155ce458b5dde4b90..8ac40e5759373445056a18cb6cb08554a506c6e7 100644 (file)
--- a/src/core/umount.c
+++ b/src/core/umount.c
@@ -25,6 +25,9 @@
 #include <sys/mount.h>
 #include <sys/swap.h>
 
+/* This needs to be after sys/mount.h :( */
+#include <libmount.h>
+
 #include "libudev.h"
 
 #include "alloc-util.h"
@@ -46,6 +49,9 @@
 #include "util.h"
 #include "virt.h"
 
+DEFINE_TRIVIAL_CLEANUP_FUNC(struct libmnt_table*, mnt_free_table);
+DEFINE_TRIVIAL_CLEANUP_FUNC(struct libmnt_iter*, mnt_free_iter);
+
 typedef struct MountPoint {
         char *path;
         char *remount_options;
@@ -74,46 +80,45 @@ static void mount_points_list_free(MountPoint **head) {
 }
 
 static int mount_points_list_get(MountPoint **head) {
-        _cleanup_fclose_ FILE *proc_self_mountinfo = NULL;
-        unsigned int i;
+        _cleanup_(mnt_free_tablep) struct libmnt_table *t = NULL;
+        _cleanup_(mnt_free_iterp) struct libmnt_iter *i = NULL;
         int r;
 
         assert(head);
 
-        proc_self_mountinfo = fopen("/proc/self/mountinfo", "re");
-        if (!proc_self_mountinfo)
-                return -errno;
+        t = mnt_new_table();
+        i = mnt_new_iter(MNT_ITER_FORWARD);
+        if (!t || !i)
+                return log_oom();
 
-        for (i = 1;; i++) {
-                _cleanup_free_ char *path = NULL, *options = NULL, *flags = NULL, *type = NULL, *p = NULL;
+        r = mnt_table_parse_mtab(t, NULL);
+        if (r < 0)
+                return log_error_errno(r, "Failed to parse /proc/self/mountinfo: %m");
+
+        for (;;) {
+                struct libmnt_fs *fs;
+                const char *path, *options, *fstype;
+                _cleanup_free_ char *d = NULL, *p = NULL;
+                unsigned long remount_flags = 0u;
+                _cleanup_free_ char *remount_options = NULL;
+                bool try_remount_ro;
                 MountPoint *m;
-                int k;
 
-                k = fscanf(proc_self_mountinfo,
-                           "%*s "       /* (1) mount id */
-                           "%*s "       /* (2) parent id */
-                           "%*s "       /* (3) major:minor */
-                           "%*s "       /* (4) root */
-                           "%ms "       /* (5) mount point */
-                           "%ms"        /* (6) mount flags */
-                           "%*[^-]"     /* (7) optional fields */
-                           "- "         /* (8) separator */
-                           "%ms "       /* (9) file system type */
-                           "%*s"        /* (10) mount source */
-                           "%ms"        /* (11) mount options */
-                           "%*[^\n]",   /* some rubbish at the end */
-                           &path, &flags, &type, &options);
-                if (k != 4) {
-                        if (k == EOF)
-                                break;
+                r = mnt_table_next_fs(t, i, &fs);
+                if (r == 1)
+                        break;
+                if (r < 0)
+                        return log_error_errno(r, "Failed to get next entry from /proc/self/mountinfo: %m");
 
-                        log_warning("Failed to parse /proc/self/mountinfo:%u.", i);
+                path = mnt_fs_get_target(fs);
+                if (!path)
                         continue;
-                }
 
-                r = cunescape(path, UNESCAPE_RELAX, &p);
-                if (r < 0)
-                        return r;
+                if (cunescape(path, UNESCAPE_RELAX, &p) < 0)
+                        return log_oom();
+
+                options = mnt_fs_get_options(fs);
+                fstype = mnt_fs_get_fstype(fs);
 
                 /* Ignore mount points we can't unmount because they
                  * are API or because we are keeping them open (like
@@ -129,11 +134,6 @@ static int mount_points_list_get(MountPoint **head) {
                     path_startswith(p, "/proc"))
                         continue;
 
-                m = new0(MountPoint, 1);
-                if (!m)
-                        return -ENOMEM;
-
-                free_and_replace(m->path, p);
 
                 /* If we are in a container, don't attempt to
                  * read-only mount anything as that brings no real
@@ -145,35 +145,44 @@ static int mount_points_list_get(MountPoint **head) {
                  * a "dirty fs") and could hang if the network is down.
                  * Note that umount2() is more careful and will not
                  * hang because of the network being down. */
-                m->try_remount_ro = detect_container() <= 0 &&
-                                    !fstype_is_network(type) &&
-                                    !fstype_is_api_vfs(type) &&
-                                    !fstype_is_ro(type) &&
-                                    !fstab_test_yes_no_option(options, "ro\0rw\0");
-
-                if (m->try_remount_ro) {
-                        _cleanup_free_ char *unknown_flags = NULL;
+                try_remount_ro = detect_container() <= 0 &&
+                                 !fstype_is_network(fstype) &&
+                                 !fstype_is_api_vfs(fstype) &&
+                                 !fstype_is_ro(fstype) &&
+                                 !fstab_test_yes_no_option(options, "ro\0rw\0");
 
+                if (try_remount_ro) {
                         /* mount(2) states that mount flags and options need to be exactly the same
                          * as they were when the filesystem was mounted, except for the desired
                          * changes. So we reconstruct both here and adjust them for the later
                          * remount call too. */
 
-                        r = mount_option_mangle(flags, 0, &m->remount_flags, &unknown_flags);
-                        if (r < 0)
-                                return r;
-                        if (!isempty(unknown_flags))
-                                log_warning("Ignoring unknown mount flags '%s'.", unknown_flags);
+                        r = mnt_fs_get_propagation(fs, &remount_flags);
+                        if (r < 0) {
+                                log_warning_errno(r, "mnt_fs_get_propagation() failed for %s, ignoring: %m", path);
+                                continue;
+                        }
 
-                        r = mount_option_mangle(options, m->remount_flags, &m->remount_flags, &m->remount_options);
-                        if (r < 0)
-                                return r;
+                        r = mount_option_mangle(options, remount_flags, &remount_flags, &remount_options);
+                        if (r < 0) {
+                                log_warning_errno(r, "mount_option_mangle failed for %s, ignoring: %m", path);
+                                continue;
+                        }
 
                         /* MS_BIND is special. If it is provided it will only make the mount-point
                          * read-only. If left out, the super block itself is remounted, which we want. */
-                        m->remount_flags = (m->remount_flags|MS_REMOUNT|MS_RDONLY) & ~MS_BIND;
+                        remount_flags = (remount_flags|MS_REMOUNT|MS_RDONLY) & ~MS_BIND;
                 }
 
+                m = new0(MountPoint, 1);
+                if (!m)
+                        return log_oom();
+
+                free_and_replace(m->path, p);
+                free_and_replace(m->remount_options, remount_options);
+                m->remount_flags = remount_flags;
+                m->try_remount_ro = try_remount_ro;
+
                 LIST_PREPEND(mount_point, *head, m);
         }