Skip to content

Commit

Permalink
Use env, not sh in zfsctl_snapshot_{,un}mount()
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
stiell committed Oct 8, 2016
1 parent 00b65db commit 5dc1ff2
Showing 1 changed file with 9 additions and 18 deletions.
27 changes: 9 additions & 18 deletions module/zfs/zfs_ctldir.c
Original file line number Diff line number Diff line change
Expand Up @@ -1009,16 +1009,11 @@ zfsctl_snapdir_mkdir(struct inode *dip, char *dirname, vattr_t *vap,
* 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;
Expand All @@ -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);


Expand All @@ -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)
Expand All @@ -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;
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 5dc1ff2

Please sign in to comment.