Commit 5516e595 authored by Mark Fasheh's avatar Mark Fasheh Committed by Chris Mason

Btrfs: Null terminate strings passed in from userspace

The 'char name[BTRFS_PATH_NAME_MAX]' member of struct btrfs_ioctl_vol_args
is passed directly to strlen() after being copied from user. I haven't
verified this, but in theory a userspace program could pass in an
unterminated string and cause a kernel crash as strlen walks off the end of
the array.

This patch terminates the ->name string in all btrfs ioctl functions which
currently use a 'struct btrfs_ioctl_vol_args'. Since the string is now
properly terminated, it's length will never be longer than
BTRFS_PATH_NAME_MAX so that error check has been removed.

By the way, it might be better overall to just have the ioctl pass an
unterminated string + length structure but I didn't bother with that since
it'd change the kernel/user interface.
Signed-off-by: default avatarMark Fasheh <mfasheh@suse.com>
Signed-off-by: default avatarChris Mason <chris.mason@oracle.com>
parent 9652480b
...@@ -310,11 +310,9 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg) ...@@ -310,11 +310,9 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg)
ret = -EFAULT; ret = -EFAULT;
goto out; goto out;
} }
vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
namelen = strlen(vol_args->name); namelen = strlen(vol_args->name);
if (namelen > BTRFS_VOL_NAME_MAX) {
ret = -EINVAL;
goto out;
}
mutex_lock(&root->fs_info->volume_mutex); mutex_lock(&root->fs_info->volume_mutex);
sizestr = vol_args->name; sizestr = vol_args->name;
...@@ -412,11 +410,8 @@ static noinline int btrfs_ioctl_snap_create(struct btrfs_root *root, ...@@ -412,11 +410,8 @@ static noinline int btrfs_ioctl_snap_create(struct btrfs_root *root,
goto out; goto out;
} }
vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
namelen = strlen(vol_args->name); namelen = strlen(vol_args->name);
if (namelen > BTRFS_VOL_NAME_MAX) {
ret = -EINVAL;
goto out;
}
if (strchr(vol_args->name, '/')) { if (strchr(vol_args->name, '/')) {
ret = -EINVAL; ret = -EINVAL;
goto out; goto out;
...@@ -487,6 +482,7 @@ long btrfs_ioctl_add_dev(struct btrfs_root *root, void __user *arg) ...@@ -487,6 +482,7 @@ long btrfs_ioctl_add_dev(struct btrfs_root *root, void __user *arg)
ret = -EFAULT; ret = -EFAULT;
goto out; goto out;
} }
vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
ret = btrfs_init_new_device(root, vol_args->name); ret = btrfs_init_new_device(root, vol_args->name);
out: out:
...@@ -508,6 +504,7 @@ long btrfs_ioctl_rm_dev(struct btrfs_root *root, void __user *arg) ...@@ -508,6 +504,7 @@ long btrfs_ioctl_rm_dev(struct btrfs_root *root, void __user *arg)
ret = -EFAULT; ret = -EFAULT;
goto out; goto out;
} }
vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
ret = btrfs_rm_device(root, vol_args->name); ret = btrfs_rm_device(root, vol_args->name);
out: out:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment