Merge patch series "add memdup_nul(), use it and memdup() in a few places"

Rasmus Villemoes <[email protected]> says:

There are quite a few places where we allocate X+1 bytes, initialize
the first X bytes via memcpy() and then set the last byte to 0.

The kernel has a helper for that, kmemdup_nul(). Introduce a similar
one, and start making use of it in a few places. Also the existing
memdup() helper can be put to more use.

There are lots more places one could modify. But for code shared with
host tools, one would need to do some refactoring, putting memdup()
and memdup_nul() in their own str-util.c TU which could then also be
included in the tools build.

Link: https://lore.kernel.org/r/[email protected]

10 files changed, 76 insertions, 67 deletions

diff --git a/common/cli.c b/common/cli.c
index bcc7264d51a..87ce0e4d144 100644
--- a/common/cli.c
+++ b/common/cli.c
@@ -138,11 +138,9 @@ int run_command_list(const char *cmd, int len, int flag)
 #endif
 	}
 	if (need_buff) {
-		buff = malloc(len + 1);
+		buff = memdup_nul(cmd, len);
 		if (!buff)
 			return 1;
-		memcpy(buff, cmd, len);
-		buff[len] = '\0';
 	}
 #ifdef CONFIG_HUSH_PARSER
 	if (use_hush_old()) {
diff --git a/common/stdio.c b/common/stdio.c
index fc965944209..038e576147b 100644
--- a/common/stdio.c
+++ b/common/stdio.c
@@ -217,27 +217,11 @@ struct stdio_dev *stdio_get_by_name(const char *name)
 	return NULL;
 }
 
-struct stdio_dev *stdio_clone(struct stdio_dev *dev)
-{
-	struct stdio_dev *_dev;
-
-	if (!dev)
-		return NULL;
-
-	_dev = calloc(1, sizeof(struct stdio_dev));
-	if (!_dev)
-		return NULL;
-
-	memcpy(_dev, dev, sizeof(struct stdio_dev));
-
-	return _dev;
-}
-
 int stdio_register_dev(struct stdio_dev *dev, struct stdio_dev **devp)
 {
 	struct stdio_dev *_dev;
 
-	_dev = stdio_clone(dev);
+	_dev = memdup(dev, sizeof(*dev));
 	if (!_dev)
 		return -ENODEV;
 	list_add_tail(&_dev->list, &devs.list);
diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
index 4763963914b..6a431171c8d 100644
--- a/drivers/core/acpi.c
+++ b/drivers/core/acpi.c
@@ -154,10 +154,9 @@ static int add_item(struct acpi_ctx *ctx, struct udevice *dev,
 	if (!item->size)
 		return 0;
 	if (type != TYPE_OTHER) {
-		item->buf = malloc(item->size);
+		item->buf = memdup(start, item->size);
 		if (!item->buf)
 			return log_msg_ret("mem", -ENOMEM);
-		memcpy(item->buf, start, item->size);
 	}
 	item_count++;
 	log_debug("* %s: Added type %d, %p, size %x\n",
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index d605c0f7b7c..1388b481031 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -1798,10 +1798,9 @@ int ofnode_write_prop(ofnode node, const char *propname, const void *value,
 		void *newval;
 
 		if (copy) {
-			newval = malloc(len);
+			newval = memdup(value, len);
 			if (!newval)
 				return log_ret(-ENOMEM);
-			memcpy(newval, value, len);
 			value = newval;
 		}
 		ret = of_write_prop(ofnode_to_np(node), propname, len, value);
diff --git a/drivers/core/root.c b/drivers/core/root.c
index d43645f34dd..1f32f33b295 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -81,10 +81,9 @@ static int dm_setup_inst(void)
 		/* Now allocate space for the priv/plat data, and copy it in */
 		size = __priv_data_end - __priv_data_start;
 
-		base = calloc(1, size);
+		base = memdup(__priv_data_start, size);
 		if (!base)
 			return log_msg_ret("priv", -ENOMEM);
-		memcpy(base, __priv_data_start, size);
 		gd_set_dm_priv_base(base);
 	}
 
diff --git a/include/linux/string.h b/include/linux/string.h
index a8a6cf4af50..850356d7c3f 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -101,12 +101,12 @@ size_t strcspn(const char *s, const char *reject);
 # define strndup		sandbox_strndup
 #endif
 
-#ifndef __HAVE_ARCH_STRDUP
 extern char * strdup(const char *);
 extern char * strndup(const char *, size_t);
+
 extern const char *strdup_const(const char *s);
 extern void kfree_const(const void *x);
-#endif
+
 #ifndef __HAVE_ARCH_STRSWAB
 extern char * strswab(const char *);
 #endif
@@ -144,7 +144,20 @@ void *memchr_inv(const void *, int, size_t);
  *	memory is available
  *
  */
-char *memdup(const void *src, size_t len);
+void *memdup(const void *src, size_t len);
+
+/**
+ * memdup_nul() - allocate a buffer and copy in the contents, appending a nul byte
+ *
+ * Note that this returns a valid pointer even if @len is 0
+ *
+ * @src: data to copy in
+ * @len: number of bytes to copy
+ * Return: allocated buffer with the copied contents and an extra nul byte,
+ *      or NULL if not enough memory is available
+ *
+ */
+void *memdup_nul(const void *src, size_t len);
 
 unsigned long ustrtoul(const char *cp, char **endp, unsigned int base);
 unsigned long long ustrtoull(const char *cp, char **endp, unsigned int base);
diff --git a/include/stdio_dev.h b/include/stdio_dev.h
index f7f9c10199e..d93604331ff 100644
--- a/include/stdio_dev.h
+++ b/include/stdio_dev.h
@@ -96,7 +96,6 @@ int stdio_add_devices(void);
 int stdio_deregister_dev(struct stdio_dev *dev, int force);
 struct list_head *stdio_get_list(void);
 struct stdio_dev *stdio_get_by_name(const char *name);
-struct stdio_dev *stdio_clone(struct stdio_dev *dev);
 
 int drv_lcd_init(void);
 int drv_video_init(void);
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 75c263b5053..f96a8e686f6 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -821,13 +821,12 @@ int himport_r(struct hsearch_data *htab,
 	}
 
 	/* we allocate new space to make sure we can write to the array */
-	if ((data = malloc(size + 1)) == NULL) {
-		debug("himport_r: can't malloc %lu bytes\n", (ulong)size + 1);
+	data = memdup_nul(env, size);
+	if (data == NULL) {
+		debug("himport_r: can't duplicate env block\n");
 		__set_errno(ENOMEM);
 		return 0;
 	}
-	memcpy(data, env, size);
-	data[size] = '\0';
 	dp = data;
 
 	/* make a local copy of the list of variables */
diff --git a/lib/string.c b/lib/string.c
index 302efe048b0..37ea8c29561 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -343,41 +343,29 @@ size_t strcspn(const char *s, const char *reject)
 }
 #endif
 
-#ifndef __HAVE_ARCH_STRDUP
-char * strdup(const char *s)
+void *memdup_nul(const void *src, size_t len)
 {
-	char *new;
+	char *dst;
 
-	if ((s == NULL)	||
-	    ((new = malloc (strlen(s) + 1)) == NULL) ) {
+	if (len + 1 < len)
 		return NULL;
-	}
 
-	strcpy (new, s);
-	return new;
-}
-
-char * strndup(const char *s, size_t n)
-{
-	size_t len;
-	char *new;
-
-	if (s == NULL)
+	dst = malloc(len + 1);
+	if (!dst)
 		return NULL;
 
-	len = strlen(s);
-
-	if (n < len)
-		len = n;
-
-	new = malloc(len + 1);
-	if (new == NULL)
-		return NULL;
+	dst[len] = '\0';
+	return memcpy(dst, src, len);
+}
 
-	strncpy(new, s, len);
-	new[len] = '\0';
+char * strdup(const char *s)
+{
+	return s ? memdup_nul(s, strlen(s)) : NULL;
+}
 
-	return new;
+char * strndup(const char *s, size_t n)
+{
+	return s ? memdup_nul(s, strnlen(s, n)) : NULL;
 }
 
 /**
@@ -410,7 +398,6 @@ void kfree_const(const void *x)
 		free((void *)x);
 }
 
-#endif
 
 #ifndef __HAVE_ARCH_STRSPN
 /**
@@ -698,17 +685,15 @@ void * memscan(void * addr, int c, size_t size)
 }
 #endif
 
-char *memdup(const void *src, size_t len)
+void *memdup(const void *src, size_t len)
 {
-	char *p;
+	void *p;
 
 	p = malloc(len);
 	if (!p)
 		return NULL;
 
-	memcpy(p, src, len);
-
-	return p;
+	return memcpy(p, src, len);
 }
 
 #ifndef __HAVE_ARCH_STRNSTR
diff --git a/test/lib/string.c b/test/lib/string.c
index f56c2e4c946..db6f28dbfdf 100644
--- a/test/lib/string.c
+++ b/test/lib/string.c
@@ -223,6 +223,40 @@ static int lib_memdup(struct unit_test_state *uts)
 }
 LIB_TEST(lib_memdup, 0);
 
+/** lib_memdup_nul() - unit test for memdup_nul() */
+static int lib_memdup_nul(struct unit_test_state *uts)
+{
+	char buf[BUFLEN];
+	size_t len;
+	char *p, *q;
+
+	/* Zero size should return a buffer containing a single nul byte */
+	p = memdup_nul(NULL, 0);
+	ut_assertnonnull(p);
+	ut_assert(p[0] == '\0');
+	free(p);
+
+	p = memdup_nul(buf, 0);
+	ut_assertnonnull(p);
+	ut_assert(p[0] == '\0');
+	free(p);
+
+	strcpy(buf, TEST_STR);
+	len = sizeof(TEST_STR);
+	p = memdup_nul(buf, len);
+	ut_asserteq_mem(p, buf, len);
+	ut_assert(p[len] == '\0');
+
+	q = memdup_nul(p, len);
+	ut_asserteq_mem(q, buf, len);
+	ut_assert(q[len] == '\0');
+	free(q);
+	free(p);
+
+	return 0;
+}
+LIB_TEST(lib_memdup_nul, 0);
+
 /** lib_strnstr() - unit test for strnstr() */
 static int lib_strnstr(struct unit_test_state *uts)
 {