From: Mikulas Patocka <[email-masked]>

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 the CAP_SYS_ADMIN
capability is required to run this code.  But it should be fixed anyway.

Signed-off-by: Mikulas Patocka <[email-masked]>
Signed-off-by: Alasdair G Kergon <[email-masked]>
Cc: [email-masked]

---
 drivers/md/dm-ioctl.c |    9 +++++++++
 1 file changed, 9 insertions(+)

Index: linux/drivers/md/dm-ioctl.c
===================================================================
--- linux.orig/drivers/md/dm-ioctl.c
+++ linux/drivers/md/dm-ioctl.c
@@ -1566,11 +1566,20 @@ static int copy_params(struct dm_ioctl _
 	if (copy_from_user(dmi, user, tmp.data_size))
 		goto bad;
 
+	/*
+	 * Abort if we discover something changed the ioctl data whilst we copied it.
+	 */
+	if (dmi->data_size != tmp.data_size) {
+		DMERR("rejecting ioctl: data size modified while copying parameters");
+		goto bad;
+	}
+
 	/* Wipe the user buffer so we do not return it to userspace */
 	if (secure_data && clear_user(user, tmp.data_size))
 		goto bad;
 
 	*param = dmi;
+
 	return 0;
 
 bad: