[PATCH] Fix using of possible uninitialised params struct (secure flag)

[PATCH] Fix using of possible uninitialised params struct (secure flag)

[Milan Broz]

If copy_param() fails, we cannot trust param structure,
so access it only if this call returned success.

Bug introduced in patch adding secure flag to dm-ioctl (currently in linux-next).

Found during regression tests.

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 drivers/md/dm-ioctl.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index e7af88b..516def2 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1611,14 +1611,15 @@ static int ctl_ioctl(uint command, struct dm_ioctl __user *user)
 	 * Copy the parameters into kernel space.
 	 */
 	r = copy_params(user, &param);
-	input_param_size = param->data_size;
-	wipe_buffer = param->flags & DM_SECURE_DATA_FLAG;
 
 	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);
 	if (r)
 		goto out;
-- 
1.7.2.3

[PATCH] Wipe buffer even if vamlloc fails

[Milan Broz]

We can honor secure data flag even when allocation
of local buffer fails but user params is valid.

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 drivers/md/dm-ioctl.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 516def2..4cacdad 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1504,6 +1504,7 @@ static int check_version(unsigned int cmd, struct dm_ioctl __user *user)
 static int copy_params(struct dm_ioctl __user *user, struct dm_ioctl **param)
 {
 	struct dm_ioctl tmp, *dmi;
+	int secure_data;
 
 	if (copy_from_user(&tmp, user, sizeof(tmp) - sizeof(tmp.data)))
 		return -EFAULT;
@@ -1511,23 +1512,27 @@ static int copy_params(struct dm_ioctl __user *user, struct dm_ioctl **param)
 	if (tmp.data_size < (sizeof(tmp) - sizeof(tmp.data)))
 		return -EINVAL;
 
+	secure_data = tmp.flags & DM_SECURE_DATA_FLAG;
+
 	dmi = vmalloc(tmp.data_size);
-	if (!dmi)
+	if (!dmi) {
+		if (secure_data && clear_user(user, tmp.data_size))
+			return -EFAULT;
 		return -ENOMEM;
+	}
 
 	if (copy_from_user(dmi, user, tmp.data_size))
 		goto bad;
 
 	/* Wipe the user buffer so we do not return it to userspace */
-	if ((tmp.flags & DM_SECURE_DATA_FLAG) &&
-	    clear_user(user, tmp.data_size))
+	if (secure_data && clear_user(user, tmp.data_size))
 		goto bad;
 
 	*param = dmi;
 	return 0;
 
 bad:
-	if (tmp.flags & DM_SECURE_DATA_FLAG)
+	if (secure_data)
 		memset(dmi, 0, tmp.data_size);
 	vfree(dmi);
 	return -EFAULT;
-- 
1.7.2.3