]> granicus.if.org Git - zfs/commitdiff
Use env, not sh in zfsctl_snapshot_{,un}mount()
authorStian Ellingsen <stian@plaimi.net>
Thu, 6 Oct 2016 18:03:41 +0000 (20:03 +0200)
committerStian Ellingsen <stian@plaimi.net>
Sat, 8 Oct 2016 15:43:29 +0000 (17:43 +0200)
Call mount and umount via /usr/bin/env instead of /bin/sh in
zfsctl_snapshot_mount() and zfsctl_snapshot_unmount().

This change fixes a shell code injection flaw.  The call to /bin/sh
passed the mountpoint unescaped, only surrounded by single quotes.  A
mountpoint containing one or more single quotes would cause the command
to fail or potentially execute arbitrary shell code.

This change also provides compatibility with grsecurity patches.
Grsecurity only allows call_usermodehelper() to use helper binaries in
certain paths.  /usr/bin/* is allowed, /bin/* is not.

module/zfs/zfs_ctldir.c

index c4c5ec522a2cbceb471882017e1fe9dcaf8336c4..44003a5a625706ef6b7e7a2edc3cd6ec2c72d612 100644 (file)
@@ -1009,16 +1009,11 @@ out:
  * best effort.  In the case where it does fail, perhaps because
  * it's in use, the unmount will fail harmlessly.
  */
-#define        SET_UNMOUNT_CMD \
-       "exec 0</dev/null " \
-       "     1>/dev/null " \
-       "     2>/dev/null; " \
-       "umount -t zfs -n %s'%s'"
-
 int
 zfsctl_snapshot_unmount(char *snapname, int flags)
 {
-       char *argv[] = { "/bin/sh", "-c", NULL, NULL };
+       char *argv[] = { "/usr/bin/env", "umount", "-t", "zfs", "-n", NULL,
+           NULL };
        char *envp[] = { NULL };
        zfs_snapentry_t *se;
        int error;
@@ -1030,11 +1025,11 @@ zfsctl_snapshot_unmount(char *snapname, int flags)
        }
        rw_exit(&zfs_snapshot_lock);
 
-       argv[2] = kmem_asprintf(SET_UNMOUNT_CMD,
-           flags & MNT_FORCE ? "-f " : "", se->se_path);
+       if (flags & MNT_FORCE)
+               argv[4] = "-fn";
+       argv[5] = se->se_path;
        dprintf("unmount; path=%s\n", se->se_path);
        error = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
-       strfree(argv[2]);
        zfsctl_snapshot_rele(se);
 
 
@@ -1050,11 +1045,6 @@ zfsctl_snapshot_unmount(char *snapname, int flags)
 }
 
 #define        MOUNT_BUSY 0x80         /* Mount failed due to EBUSY (from mntent.h) */
-#define        SET_MOUNT_CMD \
-       "exec 0</dev/null " \
-       "     1>/dev/null " \
-       "     2>/dev/null; " \
-       "mount -t zfs -n '%s' '%s'"
 
 int
 zfsctl_snapshot_mount(struct path *path, int flags)
@@ -1065,7 +1055,8 @@ zfsctl_snapshot_mount(struct path *path, int flags)
        zfs_sb_t *snap_zsb;
        zfs_snapentry_t *se;
        char *full_name, *full_path;
-       char *argv[] = { "/bin/sh", "-c", NULL, NULL };
+       char *argv[] = { "/usr/bin/env", "mount", "-t", "zfs", "-n", NULL, NULL,
+           NULL };
        char *envp[] = { NULL };
        int error;
        struct path spath;
@@ -1110,9 +1101,9 @@ zfsctl_snapshot_mount(struct path *path, int flags)
         * value from call_usermodehelper() will be (exitcode << 8 + signal).
         */
        dprintf("mount; name=%s path=%s\n", full_name, full_path);
-       argv[2] = kmem_asprintf(SET_MOUNT_CMD, full_name, full_path);
+       argv[5] = full_name;
+       argv[6] = full_path;
        error = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
-       strfree(argv[2]);
        if (error) {
                if (!(error & MOUNT_BUSY << 8)) {
                        cmn_err(CE_WARN, "Unable to automount %s/%s: %d",