dm ioctl: optimize functions without variable params

Device-mapper ioctls receive and send data in a buffer supplied by userspace. The buffer has two parts. The first part contains a 'struct dm_ioctl' and has a fixed size. The second part depends on the ioctl and has a variable size. This patch recognises the specific ioctls that do not use the variable part of the buffer and skips allocating memory for it. In particular, when a device is suspended and a resume ioctl is sent, this now avoid memory allocation completely. The variable "struct dm_ioctl tmp" is moved from the function copy_params to its caller ctl_ioctl and renamed to param_kernel. It is used directly when the ioctl function doesn't need any arguments. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Signed-off-by: Alasdair G Kergon <agk@redhat.com>

dm ioctl: optimize functions without variable params
Device-mapper ioctls receive and send data in a buffer supplied by userspace. The buffer has two parts. The first part contains a 'struct dm_ioctl' and has a fixed size. The second part depends on the ioctl and has a variable size. This patch recognises the specific ioctls that do not use the variable part of the buffer and skips allocating memory for it. In particular, when a device is suspended and a resume ioctl is sent, this now avoid memory allocation completely. The variable "struct dm_ioctl tmp" is moved from the function copy_params to its caller ctl_ioctl and renamed to param_kernel. It is used directly when the ioctl function doesn't need any arguments. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Signed-off-by: Alasdair G Kergon <agk@redhat.com>
Mikulas Patocka · Alasdair G Kergon
1 parent e2914cc26b
Showing 2 changed files with 37 additions and 21 deletions Side-by-side Diff
drivers/md/dm-ioctl.c
include/uapi/linux/dm-ioctl.h
@@ -1560,7 +1560,8 @@
 	return r;
 }
  
-#define DM_PARAMS_VMALLOC	0x0001	/* Params alloced with vmalloc not kmalloc */
+#define DM_PARAMS_KMALLOC	0x0001	/* Params alloced with kmalloc */
+#define DM_PARAMS_VMALLOC	0x0002	/* Params alloced with vmalloc */
 #define DM_WIPE_BUFFER		0x0010	/* Wipe input buffer before returning from ioctl */
  
 static void free_params(struct dm_ioctl *param, size_t param_size, int param_flags)
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
@@ -1568,66 +1569,80 @@
 	if (param_flags & DM_WIPE_BUFFER)
 		memset(param, 0, param_size);
  
+	if (param_flags & DM_PARAMS_KMALLOC)
+		kfree(param);
 	if (param_flags & DM_PARAMS_VMALLOC)
 		vfree(param);
-	else
-		kfree(param);
 }
  
-static int copy_params(struct dm_ioctl __user *user, struct dm_ioctl **param, int *param_flags)
+static int copy_params(struct dm_ioctl __user *user, struct dm_ioctl *param_kernel,
+		       int ioctl_flags,
+		       struct dm_ioctl **param, int *param_flags)
 {
-	struct dm_ioctl tmp, *dmi;
+	struct dm_ioctl *dmi;
 	int secure_data;
+	const size_t minimum_data_size = sizeof(*param_kernel) - sizeof(param_kernel->data);
  
-	if (copy_from_user(&tmp, user, sizeof(tmp) - sizeof(tmp.data)))
+	if (copy_from_user(param_kernel, user, minimum_data_size))
 		return -EFAULT;
  
-	if (tmp.data_size < (sizeof(tmp) - sizeof(tmp.data)))
+	if (param_kernel->data_size < minimum_data_size)
 		return -EINVAL;
  
-	secure_data = tmp.flags & DM_SECURE_DATA_FLAG;
+	secure_data = param_kernel->flags & DM_SECURE_DATA_FLAG;
  
 	*param_flags = secure_data ? DM_WIPE_BUFFER : 0;
  
+	if (ioctl_flags & IOCTL_FLAGS_NO_PARAMS) {
+		dmi = param_kernel;
+		dmi->data_size = minimum_data_size;
+		goto data_copied;
+	}
+
 	/*
 	 * Try to avoid low memory issues when a device is suspended.
 	 * Use kmalloc() rather than vmalloc() when we can.
 	 */
 	dmi = NULL;
-	if (tmp.data_size <= KMALLOC_MAX_SIZE)
-		dmi = kmalloc(tmp.data_size, GFP_NOIO | __GFP_NORETRY | __GFP_NOMEMALLOC | __GFP_NOWARN);
+	if (param_kernel->data_size <= KMALLOC_MAX_SIZE) {
+		dmi = kmalloc(param_kernel->data_size, GFP_NOIO | __GFP_NORETRY | __GFP_NOMEMALLOC | __GFP_NOWARN);
+		if (dmi)
+			*param_flags |= DM_PARAMS_KMALLOC;
+	}
  
 	if (!dmi) {
-		dmi = __vmalloc(tmp.data_size, GFP_NOIO | __GFP_REPEAT | __GFP_HIGH, PAGE_KERNEL);
-		*param_flags |= DM_PARAMS_VMALLOC;
+		dmi = __vmalloc(param_kernel->data_size, GFP_NOIO | __GFP_REPEAT | __GFP_HIGH, PAGE_KERNEL);
+		if (dmi)
+			*param_flags |= DM_PARAMS_VMALLOC;
 	}
  
 	if (!dmi) {
-		if (secure_data && clear_user(user, tmp.data_size))
+		if (secure_data && clear_user(user, param_kernel->data_size))
 			return -EFAULT;
 		return -ENOMEM;
 	}
  
-	if (copy_from_user(dmi, user, tmp.data_size))
+	if (copy_from_user(dmi, user, param_kernel->data_size))
 		goto bad;
  
+data_copied:
 	/*
 	 * Abort if something changed the ioctl data while it was being copied.
 	 */
-	if (dmi->data_size != tmp.data_size) {
+	if (dmi->data_size != param_kernel->data_size) {
 		DMERR("rejecting ioctl: data size modified while processing parameters");
 		goto bad;
 	}
  
 	/* Wipe the user buffer so we do not return it to userspace */
-	if (secure_data && clear_user(user, tmp.data_size))
+	if (secure_data && clear_user(user, param_kernel->data_size))
 		goto bad;
  
 	*param = dmi;
 	return 0;
  
 bad:
-	free_params(dmi, tmp.data_size, *param_flags);
+	free_params(dmi, param_kernel->data_size, *param_flags);
  
 	return -EFAULT;
 }
@@ -1671,6 +1686,7 @@
 	struct dm_ioctl *uninitialized_var(param);
 	ioctl_fn fn = NULL;
 	size_t input_param_size;
+	struct dm_ioctl param_kernel;
  
 	/* only root can play with this */
 	if (!capable(CAP_SYS_ADMIN))
@@ -1704,7 +1720,7 @@
 	/*
 	 * Copy the parameters into kernel space.
 	 */
-	r = copy_params(user, &param, &param_flags);
+	r = copy_params(user, &param_kernel, ioctl_flags, &param, &param_flags);
  
 	if (r)
 		return r;
@@ -267,9 +267,9 @@
 #define DM_DEV_SET_GEOMETRY	_IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD, struct dm_ioctl)
  
 #define DM_VERSION_MAJOR	4
-#define DM_VERSION_MINOR	23
-#define DM_VERSION_PATCHLEVEL	1
-#define DM_VERSION_EXTRA	"-ioctl (2012-12-18)"
+#define DM_VERSION_MINOR	24
+#define DM_VERSION_PATCHLEVEL	0
+#define DM_VERSION_EXTRA	"-ioctl (2013-01-15)"
  
 /* Status bits */
 #define DM_READONLY_FLAG	(1 << 0) /* In/Out */
...	...	@@ -1560,7 +1560,8 @@
1560	1560	return r;
1561	1561	}
1562	1562
1563		-#define DM_PARAMS_VMALLOC 0x0001 /* Params alloced with vmalloc not kmalloc */
	1563	+#define DM_PARAMS_KMALLOC 0x0001 /* Params alloced with kmalloc */
	1564	+#define DM_PARAMS_VMALLOC 0x0002 /* Params alloced with vmalloc */
1564	1565	#define DM_WIPE_BUFFER 0x0010 /* Wipe input buffer before returning from ioctl */
1565	1566
1566	1567	static void free_params(struct dm_ioctl *param, size_t param_size, int param_flags)
1567	1568
1568	1569
1569	1570
1570	1571
1571	1572
1572	1573
1573	1574
1574	1575
1575	1576
1576	1577
1577	1578
1578	1579
1579	1580
1580	1581
1581	1582
1582	1583
...	...	@@ -1568,66 +1569,80 @@
1568	1569	if (param_flags & DM_WIPE_BUFFER)
1569	1570	memset(param, 0, param_size);
1570	1571
	1572	+ if (param_flags & DM_PARAMS_KMALLOC)
	1573	+ kfree(param);
1571	1574	if (param_flags & DM_PARAMS_VMALLOC)
1572	1575	vfree(param);
1573		- else
1574		- kfree(param);
1575	1576	}
1576	1577
1577		-static int copy_params(struct dm_ioctl __user user, struct dm_ioctl param, int param_flags)
	1578	+static int copy_params(struct dm_ioctl __user user, struct dm_ioctl param_kernel,
	1579	+ int ioctl_flags,
	1580	+ struct dm_ioctl *param, int param_flags)
1578	1581	{
1579		- struct dm_ioctl tmp, *dmi;
	1582	+ struct dm_ioctl *dmi;
1580	1583	int secure_data;
	1584	+ const size_t minimum_data_size = sizeof(*param_kernel) - sizeof(param_kernel->data);
1581	1585
1582		- if (copy_from_user(&tmp, user, sizeof(tmp) - sizeof(tmp.data)))
	1586	+ if (copy_from_user(param_kernel, user, minimum_data_size))
1583	1587	return -EFAULT;
1584	1588
1585		- if (tmp.data_size < (sizeof(tmp) - sizeof(tmp.data)))
	1589	+ if (param_kernel->data_size < minimum_data_size)
1586	1590	return -EINVAL;
1587	1591
1588		- secure_data = tmp.flags & DM_SECURE_DATA_FLAG;
	1592	+ secure_data = param_kernel->flags & DM_SECURE_DATA_FLAG;
1589	1593
1590	1594	*param_flags = secure_data ? DM_WIPE_BUFFER : 0;
1591	1595
	1596	+ if (ioctl_flags & IOCTL_FLAGS_NO_PARAMS) {
	1597	+ dmi = param_kernel;
	1598	+ dmi->data_size = minimum_data_size;
	1599	+ goto data_copied;
	1600	+ }
	1601	+
1592	1602	/*
1593	1603	* Try to avoid low memory issues when a device is suspended.
1594	1604	* Use kmalloc() rather than vmalloc() when we can.
1595	1605	*/
1596	1606	dmi = NULL;
1597		- if (tmp.data_size <= KMALLOC_MAX_SIZE)
1598		- dmi = kmalloc(tmp.data_size, GFP_NOIO \| __GFP_NORETRY \| __GFP_NOMEMALLOC \| __GFP_NOWARN);
	1607	+ if (param_kernel->data_size <= KMALLOC_MAX_SIZE) {
	1608	+ dmi = kmalloc(param_kernel->data_size, GFP_NOIO \| __GFP_NORETRY \| __GFP_NOMEMALLOC \| __GFP_NOWARN);
	1609	+ if (dmi)
	1610	+ *param_flags \|= DM_PARAMS_KMALLOC;
	1611	+ }
1599	1612
1600	1613	if (!dmi) {
1601		- dmi = __vmalloc(tmp.data_size, GFP_NOIO \| __GFP_REPEAT \| __GFP_HIGH, PAGE_KERNEL);
1602		- *param_flags \|= DM_PARAMS_VMALLOC;
	1614	+ dmi = __vmalloc(param_kernel->data_size, GFP_NOIO \| __GFP_REPEAT \| __GFP_HIGH, PAGE_KERNEL);
	1615	+ if (dmi)
	1616	+ *param_flags \|= DM_PARAMS_VMALLOC;
1603	1617	}
1604	1618
1605	1619	if (!dmi) {
1606		- if (secure_data && clear_user(user, tmp.data_size))
	1620	+ if (secure_data && clear_user(user, param_kernel->data_size))
1607	1621	return -EFAULT;
1608	1622	return -ENOMEM;
1609	1623	}
1610	1624
1611		- if (copy_from_user(dmi, user, tmp.data_size))
	1625	+ if (copy_from_user(dmi, user, param_kernel->data_size))
1612	1626	goto bad;
1613	1627
	1628	+data_copied:
1614	1629	/*
1615	1630	* Abort if something changed the ioctl data while it was being copied.
1616	1631	*/
1617		- if (dmi->data_size != tmp.data_size) {
	1632	+ if (dmi->data_size != param_kernel->data_size) {
1618	1633	DMERR("rejecting ioctl: data size modified while processing parameters");
1619	1634	goto bad;
1620	1635	}
1621	1636
1622	1637	/* Wipe the user buffer so we do not return it to userspace */
1623		- if (secure_data && clear_user(user, tmp.data_size))
	1638	+ if (secure_data && clear_user(user, param_kernel->data_size))
1624	1639	goto bad;
1625	1640
1626	1641	*param = dmi;
1627	1642	return 0;
1628	1643
1629	1644	bad:
1630		- free_params(dmi, tmp.data_size, *param_flags);
	1645	+ free_params(dmi, param_kernel->data_size, *param_flags);
1631	1646
1632	1647	return -EFAULT;
1633	1648	}
...	...	@@ -1671,6 +1686,7 @@
1671	1686	struct dm_ioctl *uninitialized_var(param);
1672	1687	ioctl_fn fn = NULL;
1673	1688	size_t input_param_size;
	1689	+ struct dm_ioctl param_kernel;
1674	1690
1675	1691	/* only root can play with this */
1676	1692	if (!capable(CAP_SYS_ADMIN))
...	...	@@ -1704,7 +1720,7 @@
1704	1720	/*
1705	1721	* Copy the parameters into kernel space.
1706	1722	*/
1707		- r = copy_params(user, &param, &param_flags);
	1723	+ r = copy_params(user, &param_kernel, ioctl_flags, &param, &param_flags);
1708	1724
1709	1725	if (r)
1710	1726	return r;
...	...	@@ -267,9 +267,9 @@
267	267	#define DM_DEV_SET_GEOMETRY _IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD, struct dm_ioctl)
268	268
269	269	#define DM_VERSION_MAJOR 4
270		-#define DM_VERSION_MINOR 23
271		-#define DM_VERSION_PATCHLEVEL 1
272		-#define DM_VERSION_EXTRA "-ioctl (2012-12-18)"
	270	+#define DM_VERSION_MINOR 24
	271	+#define DM_VERSION_PATCHLEVEL 0
	272	+#define DM_VERSION_EXTRA "-ioctl (2013-01-15)"
273	273
274	274	/* Status bits */
275	275	#define DM_READONLY_FLAG (1 << 0) /* In/Out */