From patchwork Fri Nov 23 00:20:36 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: dm-ioctl: fix unsafe use of dm_ioctl.data_size Date: Fri, 23 Nov 2012 05:20:36 -0000 From: Mikulas Patocka X-Patchwork-Id: 55548 Message-id: dm-ioctl: fix unsafe use of dm_ioctl.data_size 1. ctl_ioctl calls copy_params 2. copy_params makes a first copy of the beginning of userspace parameters into the local variable "tmp" 3. copy_params then validates tmp.data_size and allocates a new structure and copies the whole userspace buffer there 4. ctl_ioctl reads userspace data the second time and copies the whole buffer into the pointer "param" 5. ctl_ioctl reads param->data_size without any validation and stores it in the variable "input_param_size" 6. "input_param_size" is further used as the authoritative size of the kernel buffer The problem is that the userspace code can change the content of user memory between steps 2. and 4. The userspace can set valid data_size, call the ioctl, the kernel code concludes that tmp.data_size is valid, the userspace changes the value to something invalid, the kernel reads the userspace buffer at step 4. The kernel then uses the invalid value in param->data_size without any checks. Thus, userspace can force the kernel to access invalid kernel memory. This patch fixes it so that "input_param_size" is read in copy_params, so the validated value is used. Hopefully, this doesn't have security impact because only root can access /dev/mapper/control device. But it should be fixed anyway. Signed-off-by: Mikulas Patocka Cc: stable@kernel.org --- drivers/md/dm-ioctl.c | 8 +++++--- drivers/md/dm-ioctl.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel Index: linux/drivers/md/dm-ioctl.c =================================================================== --- linux.orig/drivers/md/dm-ioctl.c +++ linux/drivers/md/dm-ioctl.c @@ -1543,7 +1543,8 @@ static int check_version(unsigned int cm return r; } -static int copy_params(struct dm_ioctl __user *user, struct dm_ioctl **param) +static int copy_params(struct dm_ioctl __user *user, struct dm_ioctl **param, + size_t *param_size) { struct dm_ioctl tmp, *dmi; int secure_data; @@ -1554,6 +1555,8 @@ static int copy_params(struct dm_ioctl _ if (tmp.data_size < (sizeof(tmp) - sizeof(tmp.data))) return -EINVAL; + *param_size = tmp.data_size; + secure_data = tmp.flags & DM_SECURE_DATA_FLAG; dmi = vmalloc(tmp.data_size); @@ -1657,14 +1660,13 @@ static int ctl_ioctl(uint command, struc /* * Copy the parameters into kernel space. */ - r = copy_params(user, ¶m); + r = copy_params(user, ¶m, &input_param_size); current->flags &= ~PF_MEMALLOC; if (r) return r; - input_param_size = param->data_size; wipe_buffer = param->flags & DM_SECURE_DATA_FLAG; r = validate_params(cmd, param);