Re: [PATCH -v2] memdup_user(): introduce

[Andrew Morton]

On Fri, 06 Mar 2009 17:49:45 +0800
Li Zefan <lizf@xxxxxxxxxxxxxx> wrote:

> I notice there are many places doing copy_from_user() which follows
> kmalloc():
> 
>         dst = kmalloc(len, GFP_KERNEL);
>         if (!dst)
>                 return -ENOMEM;
>         if (copy_from_user(dst, src, len)) {
> 		kfree(dst);
> 		return -EFAULT
> 	}
> 
> memdup_user() is a wrapper of the above code. With this new function,
> we don't have to write 'len' twice, which can lead to typos/mistakes.
> It also produces smaller code and kernel text.
> 
> A quick grep shows 250+ places where memdup_user() *may* be used. I'll
> prepare a patchset to do this conversion.
> 
> v1 -> v2: change the name from kmemdup_from_user to memdup_user.
> 
> Signed-off-by: Li Zefan <lizf@xxxxxxxxxxxxxx>
> ---
> 
> Can this get into 2.6.29, so I can prepare patches based on linux-next?
> And this won't cause regression, since no one uses it yet. :)

I'd be OK with doing that from a patch logistics point of view, but I'd
prefer to leave a patch like this for a few days to gather more
feedback from other developers, which might push this into 2.6.30.


> diff --git a/include/linux/string.h b/include/linux/string.h
> index 76ec218..79f30f3 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -12,6 +12,7 @@
>  #include <linux/stddef.h>	/* for NULL */
>  
>  extern char *strndup_user(const char __user *, long);
> +extern void *memdup_user(const void __user *, size_t, gfp_t);
>  
>  /*
>   * Include machine specific inline routines
> diff --git a/mm/util.c b/mm/util.c
> index 37eaccd..3d21c21 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -70,6 +70,32 @@ void *kmemdup(const void *src, size_t len, gfp_t gfp)
>  EXPORT_SYMBOL(kmemdup);
>  
>  /**
> + * memdup_user - duplicate memory region from user space
> + *
> + * @src: source address in user space
> + * @len: number of bytes to copy
> + * @gfp: GFP mask to use
> + *
> + * Returns an ERR_PTR() on failure.
> + */
> +void *memdup_user(const void __user *src, size_t len, gfp_t gfp)
> +{
> +	void *p;
> +
> +	p = kmalloc_track_caller(len, gfp);
> +	if (!p)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (copy_from_user(p, src, len)) {
> +		kfree(p);
> +		return ERR_PTR(-EFAULT);
> +	}
> +
> +	return p;
> +}
> +EXPORT_SYMBOL(memdup_user);
> +
> +/**
>   * __krealloc - like krealloc() but don't free @p.
>   * @p: object to reallocate memory for.
>   * @new_size: how many bytes of memory are required.
> -- 
> 1.5.4.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/