Fix zfsctl_snapshot_{,un}mount() issues

Fix use after free in zfsctl_snapshot_unmount(). Use /usr/bin/env
instead of /bin/sh to fix a shell code injection flaw and allow use
with grsecurity.

Reviewed-by: Richard Laager <rlaager@wiktel.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov
Reviewed-by: Chunwei Chen <david.chen@osnexus.com>
Signed-off-by: Stian Ellingsen <stian@plaimi.net>  
Closes #5250 
Closes #4377
This commit is contained in:
Brian Behlendorf 2016-10-11 09:56:28 -07:00 committed by GitHub
commit 1697d2dcf1
1 changed files with 10 additions and 19 deletions

View File

@ -1009,16 +1009,11 @@ out:
* best effort. In the case where it does fail, perhaps because * best effort. In the case where it does fail, perhaps because
* it's in use, the unmount will fail harmlessly. * 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 int
zfsctl_snapshot_unmount(char *snapname, int flags) 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 }; char *envp[] = { NULL };
zfs_snapentry_t *se; zfs_snapentry_t *se;
int error; int error;
@ -1030,12 +1025,12 @@ zfsctl_snapshot_unmount(char *snapname, int flags)
} }
rw_exit(&zfs_snapshot_lock); rw_exit(&zfs_snapshot_lock);
argv[2] = kmem_asprintf(SET_UNMOUNT_CMD, if (flags & MNT_FORCE)
flags & MNT_FORCE ? "-f " : "", se->se_path); argv[4] = "-fn";
zfsctl_snapshot_rele(se); argv[5] = se->se_path;
dprintf("unmount; path=%s\n", se->se_path); dprintf("unmount; path=%s\n", se->se_path);
error = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC); 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 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 int
zfsctl_snapshot_mount(struct path *path, int flags) 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_sb_t *snap_zsb;
zfs_snapentry_t *se; zfs_snapentry_t *se;
char *full_name, *full_path; 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 }; char *envp[] = { NULL };
int error; int error;
struct path spath; struct path spath;
@ -1110,9 +1101,9 @@ zfsctl_snapshot_mount(struct path *path, int flags)
* value from call_usermodehelper() will be (exitcode << 8 + signal). * value from call_usermodehelper() will be (exitcode << 8 + signal).
*/ */
dprintf("mount; name=%s path=%s\n", full_name, full_path); 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); error = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
strfree(argv[2]);
if (error) { if (error) {
if (!(error & MOUNT_BUSY << 8)) { if (!(error & MOUNT_BUSY << 8)) {
cmn_err(CE_WARN, "Unable to automount %s/%s: %d", cmn_err(CE_WARN, "Unable to automount %s/%s: %d",