Use env, not sh in zfsctl_snapshot_{,un}mount()

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.
This commit is contained in:
Stian Ellingsen 2016-10-06 20:03:41 +02:00 committed by Brian Behlendorf
parent d71db895a1
commit dc3d6a6db1
1 changed files with 9 additions and 18 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,11 +1025,11 @@ 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";
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); 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",