libmount: improve X-mount.mkdir for non-root users

Since v2.35 mount(8) drops suid on -EPERM and repeat necessary actions
before mount(2) syscall. This patch also improves this behavior for
X-mount.mkdir too.

mount(8):
 * return -EPERM on sanitize_paths() rather than call err()
 * call suid_drop() on failed sanitize_paths()
 * update man page

libmount:
 * mnt_context_prepare_target() refactoring
 * return -EPERM when in restricted mode for X-mount.mkdir

Fixed version:
 /home/kzak/mnt-foo   sr.net.home:/home/kzak   fuse.sshfs noauto,X-mount.mkdir

 $ mount /home/kzak/mnt-foo
 kzak@sr.net.home's password:

 $ /home/projects/util-linux/util-linux  findmnt /home/kzak/mnt-foo
 TARGET             SOURCE                 FSTYPE     OPTIONS
 /home/kzak/mnt-foo sr.net.home:/home/kzak fuse.sshfs rw,nosuid,nodev,relatime,user_id=1000,group_id=1000

Addresses: https://github.com/systemd/systemd/issues/14418
Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/libmount/src/context.c b/libmount/src/context.c
index be71764d8cafa005615566e396e082a9eb353a1f..47585b071f69afe2b2f28b36e44e1d3b8705e186 100644 (file)
--- a/libmount/src/context.c
+++ b/libmount/src/context.c
@@ -1903,7 +1903,6 @@ static int mkdir_target(const char *tgt, struct libmnt_fs *fs)
 int mnt_context_prepare_target(struct libmnt_context *cxt)
 {
        const char *tgt, *prefix;
-       struct libmnt_cache *cache;
        int rc = 0;
        struct libmnt_ns *ns_old;
 
@@ -1944,37 +1943,35 @@ int mnt_context_prepare_target(struct libmnt_context *cxt)
        if (!ns_old)
                return -MNT_ERR_NAMESPACE;
 
-       /* mkdir target */
+       /* X-mount.mkdir target */
        if (cxt->action == MNT_ACT_MOUNT
-           && !mnt_context_is_restricted(cxt)
            && (cxt->user_mountflags & MNT_MS_XCOMMENT ||
                cxt->user_mountflags & MNT_MS_XFSTABCOMM)) {
 
-               rc = mkdir_target(tgt, cxt->fs);
-               if (rc) {
-                       if (!mnt_context_switch_ns(cxt, ns_old))
-                               return -MNT_ERR_NAMESPACE;
-                       return rc;      /* mkdir or parse error */
-               }
+               /* supported only for root or non-suid mount(8) */
+               if (!mnt_context_is_restricted(cxt))
+                       rc = mkdir_target(tgt, cxt->fs);
+               else
+                       rc = -EPERM;
        }
 
        /* canonicalize the path */
-       cache = mnt_context_get_cache(cxt);
-       if (cache) {
-               char *path = mnt_resolve_path(tgt, cache);
-               if (path && strcmp(path, tgt) != 0)
-                       rc = mnt_fs_set_target(cxt->fs, path);
+       if (rc == 0) {
+               struct libmnt_cache *cache = mnt_context_get_cache(cxt);
+
+               if (cache) {
+                       char *path = mnt_resolve_path(tgt, cache);
+                       if (path && strcmp(path, tgt) != 0)
+                               rc = mnt_fs_set_target(cxt->fs, path);
+               }
        }
 
        if (!mnt_context_switch_ns(cxt, ns_old))
                return -MNT_ERR_NAMESPACE;
 
-       if (rc)
-               DBG(CXT, ul_debugobj(cxt, "failed to prepare target '%s'", tgt));
-       else
-               DBG(CXT, ul_debugobj(cxt, "final target '%s'",
-                                       mnt_fs_get_target(cxt->fs)));
-       return 0;
+       DBG(CXT, ul_debugobj(cxt, "final target '%s' [rc=%d]",
+                               mnt_fs_get_target(cxt->fs), rc));
+       return rc;
 }
 
 /* Guess type, but not set to cxt->fs, always use free() for the result. It's

diff --git a/libmount/src/context_mount.c b/libmount/src/context_mount.c
index ecfc78b144d642e36f74c53b90e00b6658b50cad..088d1328e14a1a9edbbc52cfb4038b3d932d4c44 100644 (file)
--- a/libmount/src/context_mount.c
+++ b/libmount/src/context_mount.c
@@ -992,10 +992,10 @@ int mnt_context_prepare_mount(struct libmnt_context *cxt)
                rc = fix_optstr(cxt);
        if (!rc)
                rc = mnt_context_prepare_srcpath(cxt);
-       if (!rc)
-               rc = mnt_context_prepare_target(cxt);
        if (!rc)
                rc = mnt_context_guess_fstype(cxt);
+       if (!rc)
+               rc = mnt_context_prepare_target(cxt);
        if (!rc)
                rc = mnt_context_prepare_helper(cxt, "mount", NULL);
        if (rc) {

diff --git a/sys-utils/mount.8 b/sys-utils/mount.8
index 6a02a85bf8ce7a247f0e550b2ff700be88fe6512..162e2387acca8cfc97bbd0c1553f23eab050d327 100644 (file)
--- a/sys-utils/mount.8
+++ b/sys-utils/mount.8
@@ -1311,8 +1311,9 @@ Allow to make a target directory (mountpoint).  The optional argument
 specifies the filesystem access mode used for
 .BR mkdir (2)
 in octal notation.  The default mode is 0755.  This functionality is supported
-only for root users.  The option is also supported as x-mount.mkdir, this notation
-is deprecated for mount.mkdir since v2.30.
+only for root users or when mount executed without suid permissions.  The option
+is also supported as x-mount.mkdir, this notation is deprecated for mount.mkdir
+since v2.30.
 
 .SH "FILESYSTEM-SPECIFIC MOUNT OPTIONS"
 You should consult the respective man page for the filesystem first.

diff --git a/sys-utils/mount.c b/sys-utils/mount.c
index 5842bc2ecc88ffebd0e3a4abe46c66decc115cf5..257e355fe801c916bfba874b5c955cc3a4229790 100644 (file)
--- a/sys-utils/mount.c
+++ b/sys-utils/mount.c
@@ -384,19 +384,19 @@ static struct libmnt_table *append_fstab(struct libmnt_context *cxt,
  * Check source and target paths -- non-root user should not be able to
  * resolve paths which are unreadable for him.
  */
-static void sanitize_paths(struct libmnt_context *cxt)
+static int sanitize_paths(struct libmnt_context *cxt)
 {
        const char *p;
        struct libmnt_fs *fs = mnt_context_get_fs(cxt);
 
        if (!fs)
-               return;
+               return 0;
 
        p = mnt_fs_get_target(fs);
        if (p) {
                char *np = canonicalize_path_restricted(p);
                if (!np)
-                       err(MNT_EX_USAGE, "%s", p);
+                       return -EPERM;
                mnt_fs_set_target(fs, np);
                free(np);
        }
@@ -405,10 +405,11 @@ static void sanitize_paths(struct libmnt_context *cxt)
        if (p) {
                char *np = canonicalize_path_restricted(p);
                if (!np)
-                       err(MNT_EX_USAGE, "%s", p);
+                       return -EPERM;
                mnt_fs_set_source(fs, np);
                free(np);
        }
+       return 0;
 }
 
 static void append_option(struct libmnt_context *cxt, const char *opt)
@@ -952,8 +953,8 @@ int main(int argc, char **argv)
                errtryhelp(MNT_EX_USAGE);
        }
 
-       if (mnt_context_is_restricted(cxt))
-               sanitize_paths(cxt);
+       if (mnt_context_is_restricted(cxt) && sanitize_paths(cxt) != 0)
+               suid_drop(cxt);
 
        if (is_move)
                /* "move" as option string is not supported by libmount */