[PATCH 1/3] dm: __list_versions(): Only process targets once

[David Laight]

Instead of doing a prescan to determine the length of buffer required,
checking the supplied buffer is big enough, and then doing a second
scan to fill the output buffer just do a single scan and detect when
the buffer is too short.

This removes any problems that might occur if a 'target' is added
between the scans.

For additional safety only call strlen(tt->name) once and use the
returned length for everything (incuding the copy).
Ensure than all the pad bytes between the entries are zero.

Set param->data_size to the actual size of the data.
It was slightly large because ALIGN_MASK was added in instead of the size
being rounded up.

Signed-off-by: David Laight <david.laight.linux@xxxxxxxxx>
---

This seems to be the only code that uses dm_target_iterate() and the
dm_get_target_type() call is just a linear scan of the same list that
replaces the lock() with MODULE_GET().
It would all be simpler with a loop here that scanned the list,
but the required locking primitives aren't exposed.
Changing that is a larger change that I wanted to do now.

 drivers/md/dm-ioctl.c | 73 +++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 44 deletions(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index a529174c94cf..6234cb8b86b7 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -54,10 +54,8 @@ struct hash_cell {
 };
 
 struct vers_iter {
-	size_t param_size;
 	struct dm_target_versions *vers, *old_vers;
 	char *end;
-	uint32_t flags;
 };
 
 
@@ -684,41 +682,38 @@ static int list_devices(struct file *filp, struct dm_ioctl *param, size_t param_
 	return 0;
 }
 
-static void list_version_get_needed(struct target_type *tt, void *needed_param)
-{
-	size_t *needed = needed_param;
-
-	*needed += sizeof(struct dm_target_versions);
-	*needed += strlen(tt->name) + 1;
-	*needed += ALIGN_MASK;
-}
-
 static void list_version_get_info(struct target_type *tt, void *param)
 {
 	struct vers_iter *info = param;
+	struct dm_target_versions *vers = info->vers;
+	size_t name_len = strlen(tt->name);
+
+	if (!vers)
+		return;
 
-	/* Check space - it might have changed since the first iteration */
-	if ((char *)info->vers + sizeof(tt->version) + strlen(tt->name) + 1 > info->end) {
-		info->flags = DM_BUFFER_FULL_FLAG;
+	info->old_vers = vers;
+	info->vers = align_ptr((void *)(info->vers + 1) + name_len + 1);
+
+	/* Check space */
+	if ((char *)info->vers > info->end) {
+		info->vers = NULL;
 		return;
 	}
 
-	if (info->old_vers)
-		info->old_vers->next = (uint32_t) ((void *)info->vers - (void *)info->old_vers);
+	/* Zero padding and terminate vers->name[] */
+	((u64 *)info->vers)[-1] = 0;
 
-	info->vers->version[0] = tt->version[0];
-	info->vers->version[1] = tt->version[1];
-	info->vers->version[2] = tt->version[2];
-	info->vers->next = 0;
-	strcpy(info->vers->name, tt->name);
+	vers->next = (char *)info->vers - (char *)vers;
 
-	info->old_vers = info->vers;
-	info->vers = align_ptr((void *)(info->vers + 1) + strlen(tt->name) + 1);
+	vers->version[0] = tt->version[0];
+	vers->version[1] = tt->version[1];
+	vers->version[2] = tt->version[2];
+	memcpy(vers->name, tt->name, name_len);
 }
 
 static int __list_versions(struct dm_ioctl *param, size_t param_size, const char *name)
 {
-	size_t len, needed = 0;
+	size_t len;
 	struct dm_target_versions *vers;
 	struct vers_iter iter_info;
 	struct target_type *tt = NULL;
@@ -729,41 +724,31 @@ static int __list_versions(struct dm_ioctl *param, size_t param_size, const char
 			return -EINVAL;
 	}
 
-	/*
-	 * Loop through all the devices working out how much
-	 * space we need.
-	 */
-	if (!tt)
-		dm_target_iterate(list_version_get_needed, &needed);
-	else
-		list_version_get_needed(tt, &needed);
-
 	/*
 	 * Grab our output buffer.
 	 */
 	vers = get_result_buffer(param, param_size, &len);
-	if (len < needed) {
-		param->flags |= DM_BUFFER_FULL_FLAG;
-		goto out;
-	}
-	param->data_size = param->data_start + needed;
 
-	iter_info.param_size = param_size;
 	iter_info.old_vers = NULL;
 	iter_info.vers = vers;
-	iter_info.flags = 0;
-	iter_info.end = (char *)vers + needed;
+	iter_info.end = (char *)vers + len;
 
 	/*
-	 * Now loop through filling out the names & versions.
+	 * Loop through filling out the names & versions.
 	 */
 	if (!tt)
 		dm_target_iterate(list_version_get_info, &iter_info);
 	else
 		list_version_get_info(tt, &iter_info);
-	param->flags |= iter_info.flags;
 
- out:
+	if (iter_info.vers) {
+		if (iter_info.old_vers)
+			iter_info.old_vers->next = 0;
+		param->data_size = param->data_start + ((char *)iter_info.vers - (char *)vers);
+	} else {
+		param->flags |= DM_BUFFER_FULL_FLAG;
+	}
+
 	if (tt)
 		dm_put_target_type(tt);
 	return 0;
-- 
2.39.5