[Pkg-ltsp-devel] Bug#758090: ltspfs: lbmount use of "mount --move" incompatible with systemd

Vagrant Cascadian vagrant at debian.org
Thu Aug 14 22:32:09 UTC 2014


On 2014-08-14, Vagrant Cascadian wrote:
> Honestly, I'd rather figure out a way to safely mount the ltspfs mount
> directly without "mount --move". We could have a setuid wrapper that instead
> creates the mountpoint directly as /media/USERNAME/MOUNT 

Patch below which basically implements this part...


> and ensures it actually mounts, and some way of ensuring that it
> unmounts and removes the /media/USERNAME/MOUNT directory when
> done... possibly with a mount.helper and/or umount.helper

It doesn't include a mount/umount helper, or prevent the user from using
lbmount to create arbitrary /media/USERNAME/* dirs owned by them.

It is a huge reduction in setuid root code:

 scripts/ltspfsmounter | 37 +++++++++++--------------------------
 src/lbmount.c         | 68 --------------------------------------------------------------------
 2 files changed, 11 insertions(+), 94 deletions(-)

diff --git a/scripts/ltspfsmounter b/scripts/ltspfsmounter
index 5322b23..97b45ff 100644
--- a/scripts/ltspfsmounter
+++ b/scripts/ltspfsmounter
@@ -28,47 +28,35 @@ def run_hooks(mode, mountpoint):
 def get_var(name):
     return os.environ.get(name)
 
-def add_ltspfsmount(conn, path, root, dev, mediaroot):
-    hidden_mount = '%s/%s' % (root, dev)
+def add_ltspfsmount(conn, path, dev, mediaroot):
     lbmount_command = ['lbmount', dev]
-    ltspfs_mount = ['ltspfs', conn+':'+path, root+'/'+dev]
-    ltspfs_umount=['fusermount', '-uzq', hidden_mount]
-
-    if not os.access(root, 0):
-        os.mkdir(root)
-    if not os.access(hidden_mount, 0):
-        os.mkdir(hidden_mount)
+    ltspfs_mount = ['ltspfs', conn+':'+path, mediaroot+'/'+dev]
 
     env = os.environ.copy()
 
     try:
-        call(ltspfs_mount, env=env)
-    except OSError, e:
-        print >>sys.stderr, "mount failed:", e
-    try:
         call(lbmount_command)
-        if os.access(hidden_mount, 0):
-            call(ltspfs_umount)
-            os.rmdir(hidden_mount)
-        if os.access(root, 0):
-            os.rmdir(root)
         run_hooks('add', os.path.join(mediaroot, dev))
     except OSError, e:
         print >>sys.stderr, "suid mount failed:", e
+    try:
+        call(ltspfs_mount, env=env)
+    except OSError, e:
+        print >>sys.stderr, "mount failed:", e
 
 def remove_ltspfsmount(root, dev):
     lbumount_command=['lbmount', '--umount', dev]
     ltspfs_umount=['fusermount', '-uzq', root+'/'+dev]
 
     try:
-        call(lbumount_command)
-    except OSError, e:
-        print >>sys.stderr, "suid umount failed:", e
-    try:
         call(ltspfs_umount)
         run_hooks('remove', os.path.join(root, dev))
     except OSError, e:
         print >>sys.stderr, "umount failed:", e
+    try:
+        call(lbumount_command)
+    except OSError, e:
+        print >>sys.stderr, "suid umount failed:", e
 
 def cleanup(user):
     known_mounts = open( '/proc/mounts', 'r' ).readlines()
@@ -78,8 +66,6 @@ def cleanup(user):
                 mountpoint=mount.split()[1]
                 device=mountpoint.split('/')[-1]
                 if dir=='/media' and mountpoint.startswith(dir):
-                    call(['lbmount', '--umount', device])
-                elif dir=='/tmp' and mountpoint.startswith(dir):
                     call(['fusermount', '-uzq', mountpoint])
                     if os.access(mountpoint, 0):
                         os.rmdir(mountpoint)
@@ -104,7 +90,6 @@ def main():
     path = sys.argv[1]
     command = sys.argv[2]
     username = get_var('USER')
-    root = "/tmp/.%s-ltspfs" % username
     mediaroot = "/media/%s" % username
 
     if not get_var('SSH_CONNECTION'):
@@ -116,7 +101,7 @@ def main():
     dev = path.split('/')[-1]
 
     if command=='add':
-        add_ltspfsmount(conn, path, root, dev, mediaroot)
+        add_ltspfsmount(conn, path, dev, mediaroot)
     elif command=='remove':
         remove_ltspfsmount(mediaroot, dev)
     elif command=='cleanup':
diff --git a/src/lbmount.c b/src/lbmount.c
index 61152f5..dea7bd6 100644
--- a/src/lbmount.c
+++ b/src/lbmount.c
@@ -53,8 +53,6 @@ static uid_t uidReal;           /* Users real userid */
  */
 
 static char *mediadir = "/media";
-static char *ltspfsdir1 = "/tmp/.";
-static char *ltspfsdir2 = "-ltspfs/";
 static char *mountprog = "/bin/mount";  /* system mount program */
 static char *umountprog = "/bin/umount";        /* system umount program */
 
@@ -135,52 +133,6 @@ void mkdir_safe(char *dir)
 }
 
 /*
- * domount: actually bindmounts path1 onto path2.
- */
-
-int root_mounter(const char *path1, const char *path2)
-{
-    int status;
-    pid_t child;
-    char *program;
-    char *null_env[] = { NULL };
-
-    child = fork();
-
-    if (child == 0) {
-        if (setreuid(0, -1)) {
-            /* Couldn't become root */
-            perror("Couldn't obtain root privs");
-            exit(1);
-        }
-        /* Statically build command line to prevent monkey business */
-        if (path2)
-            execle(mountprog, mountprog, "--bind", path1, path2, NULL,
-                   null_env);
-        else
-            execle(umountprog, umountprog, "-l", path1, NULL, null_env);
-        perror("Error: execl() returned");
-        exit(1);                                 /* exec should never return */
-    } else if (child > 0) {
-        if (waitpid(child, &status, 0) < 0) {
-            perror("Error: wait() call failed");
-            exit(1);
-        }
-    } else if (child < 0) {
-        perror("Error: fork() failed");
-        exit(1);
-    }
-
-    if (!WIFEXITED(status)) {
-        fprintf(stderr, "Error: execle() returned no status");
-        exit(1);
-    }
-
-    return WEXITSTATUS(status);
-}
-
-
-/*
  * mainline
  */
 
@@ -190,7 +142,6 @@ int main(int argc, char **argv)
     struct passwd *pwent;
     char *mountpoint = NULL;    /* command line supplied media name */
     char mediamount[PATH_MAX];  /* fully pathed mountpoint in /media */
-    char ltspfsmount[PATH_MAX]; /* fully pathed ltspfs mount in /tmp */
 
     int option;
     static struct option long_opts[] = {
@@ -257,17 +208,7 @@ int main(int argc, char **argv)
         exit(1);
     }
 
-    /*
-     * Build our ltspfs mountpoint string, and check and see if it exists.
-     */
-
-    snprintf(ltspfsmount, sizeof(ltspfsmount), "%s%s%s%s",
-             ltspfsdir1, pwent->pw_name, ltspfsdir2, mountpoint);
-
     if (!umount) {
-        if (!is_mounted(ltspfsmount))
-            exit(1);
-
         /* OK, name's a normal size, and looks valid. Begin creating the media
          * mount point. First, we need to create /media/uid */
 
@@ -282,21 +223,12 @@ int main(int argc, char **argv)
                  pwent->pw_name, mountpoint);
 
         mkdir_safe(mediamount);
-
-        return root_mounter(ltspfsmount, mediamount);
     } else {
         /* umount */
 
         snprintf(mediamount, sizeof(mediamount), "%s/%s/%s", mediadir,
                  pwent->pw_name, mountpoint);
 
-        if (is_mounted(mediamount))
-            root_mounter(mediamount, NULL);
-        else {
-            fprintf(stderr, "Error: %s unmountable", mediamount);
-            exit(1);
-        }
-
         if (rmdir(mediamount)) {
             perror("Unable to rmdir() in /media");
             exit(1);


live well,
  vagrant
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-ltsp-devel/attachments/20140814/62716831/attachment.sig>


More information about the Pkg-ltsp-devel mailing list