[RFC 1/2] kmemdup: introduce

[RFC 1/2] kmemdup: introduce

[Alexey Dobriyan]

One of idiomatic ways to duplicate a region of memory is

	dst = kmalloc(len, GFP_KERNEL);
	if (!dst)
		return -ENOMEM;
	memcpy(dst, src, len);

which is neat code except a programmer needs to write size twice. Which
sometimes leads to mistakes. If len passed to kmalloc is smaller that len
passed to memcpy, it's straight overwrite-beyond-end. If len passed to
memcpy is smaller than len passed to kmalloc, it's either a) legit
behaviour ;-), or b) cloned buffer will contain garbage in second half.

	[TODO: list of such bugs in Linux, other kernels, userspace]

If programmer is given only one place to play with lengths, I believe, such
mistakes could be avoided.

With kmemdup, the snippet above will be rewritten as:

	dst = kmemdup(src, len, GFP_KERNEL);
	if (!dst)
		return -ENOMEM;

This also leads to smaller code (kzalloc effect).

Not tested yet, this is for semantics commentary.

The plan is to
a) merge kmemdup
b) get some users (patch #2)
c) stick kmemdup conversion into -kj TODO
d) grep for memcpy and caaaarefully convert the rest. Promise to be a bastard.
----------------
e) try to get memdup(3) somehow, but I'm probably dreaming...

P.S.: No idea why kstrdup() and kzalloc() use _____________kmalloc(),
      but I followed the crowd.
---

 include/linux/string.h |    1 +
 mm/util.c              |   18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -99,6 +99,7 @@ extern void * memchr(const void *,int,__
 #endif
 
 extern char *kstrdup(const char *s, gfp_t gfp);
+extern void *kmemdup(const void *src, size_t len, gfp_t gfp);
 
 #ifdef __cplusplus
 }
--- a/mm/util.c
+++ b/mm/util.c
@@ -40,6 +40,24 @@ char *kstrdup(const char *s, gfp_t gfp)
 }
 EXPORT_SYMBOL(kstrdup);
 
+/**
+ * kmemdup - duplicate region of memory
+ *
+ * @src: memory region to duplicate
+ * @len: memory region length
+ * @gfp: GFP mask to use
+ */
+void *kmemdup(const void *src, size_t len, gfp_t gfp)
+{
+	void *p;
+
+	p = ____kmalloc(len, gfp);
+	if (p)
+		memcpy(p, src, len);
+	return p;
+}
+EXPORT_SYMBOL(kmemdup);
+
 /*
  * strndup_user - duplicate an existing string from user space
  *

Re: [RFC 1/2] kmemdup: introduce

[Andrew Morton]

On Sat, 9 Sep 2006 05:35:55 +0400
Alexey Dobriyan <adobriyan@gmail.com> wrote:

> P.S.: No idea why kstrdup() and kzalloc() use _____________kmalloc(),

It's all to do with slab debugging and __kmalloc_track_caller(): we want
to record the _caller_ of kstrdup() within the slab object rather than kstrdup()
itself.

Re: [RFC 1/2] kmemdup: introduce

[Pekka Enberg]

On 9/9/06, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> +/**
> + * kmemdup - duplicate region of memory
> + *
> + * @src: memory region to duplicate
> + * @len: memory region length
> + * @gfp: GFP mask to use
> + */
> +void *kmemdup(const void *src, size_t len, gfp_t gfp)
> +{
> +       void *p;
> +
> +       p = ____kmalloc(len, gfp);
> +       if (p)
> +               memcpy(p, src, len);
> +       return p;
> +}
> +EXPORT_SYMBOL(kmemdup);

Assuming there are enough callers that can be converted to justify
this, looks good to me.

Re: [RFC 1/2] kmemdup: introduce

[Randy.Dunlap]

On Sat, 9 Sep 2006 05:35:55 +0400 Alexey Dobriyan wrote:

> Not tested yet, this is for semantics commentary.
> 
>  include/linux/string.h |    1 +
>  mm/util.c              |   18 ++++++++++++++++++
>  2 files changed, 19 insertions(+)
> 
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -40,6 +40,24 @@ char *kstrdup(const char *s, gfp_t gfp)
>  }
>  EXPORT_SYMBOL(kstrdup);
>  
> +/**
> + * kmemdup - duplicate region of memory
> + *

No blank line here, please.  kernel-doc "language" doesn't allow
that.  Hopefully that will be fixed someday.

> + * @src: memory region to duplicate
> + * @len: memory region length
> + * @gfp: GFP mask to use
> + */
> +void *kmemdup(const void *src, size_t len, gfp_t gfp)
> +{
> +	void *p;
> +
> +	p = ____kmalloc(len, gfp);
> +	if (p)
> +		memcpy(p, src, len);
> +	return p;
> +}
> +EXPORT_SYMBOL(kmemdup);

---
~Randy