On Wed, 15 Feb 2006 18:22:58 -0300 Davi Arnaut wrote: > This patch series creates a strndup_user() function in order to avoid duplicated > and error-prone (userspace modifying the string after the strlen_user()) code. > > v2: Inline strdup_user and fixed a bogus strdup_user usage. > > The diffstat: > > include/linux/string.h | 7 ++ > kernel/module.c | 19 +------- > mm/util.c | 37 +++++++++++++++ > security/keys/keyctl.c | 116 ++++++++++--------------------------------------- > 4 files changed, 72 insertions(+), 107 deletions(-) > > > Signed-off-by: Davi Arnaut <[email protected]> > -- > > diff --git a/include/linux/string.h b/include/linux/string.h > index 369be32..8fbf139 100644 > --- a/include/linux/string.h > +++ b/include/linux/string.h > @@ -18,6 +18,13 @@ extern char * strsep(char **,const char > extern __kernel_size_t strspn(const char *,const char *); > extern __kernel_size_t strcspn(const char *,const char *); > > +extern char *strndup_user(const char __user *, long); > + > +static inline char *strdup_user(const char __user *s) > +{ > + return strndup_user(s, 4096); PAGE_SIZE ? > +} > + > /* > * Include machine specific inline routines > */ > diff --git a/mm/util.c b/mm/util.c > index 5f4bb59..09c2c3b 100644 > --- a/mm/util.c > +++ b/mm/util.c > @@ -1,6 +1,8 @@ > #include <linux/slab.h> > #include <linux/string.h> > #include <linux/module.h> > +#include <linux/err.h> > +#include <asm/uaccess.h> > > /** > * kzalloc - allocate memory. The memory is set to zero. > @@ -37,3 +39,38 @@ char *kstrdup(const char *s, gfp_t gfp) > return buf; > } > EXPORT_SYMBOL(kstrdup); > + > +/* > + * strndup_user - duplicate an existing string from user space > + * > + * @s: The string to duplicate > + * @n: Maximum number of bytes to copy, including the trailing NUL. > + */ > +char *strndup_user(const char __user *s, long n) > +{ > + char *p; > + long length; > + > + length = strlen_user(s); This should be strnlen_user(s, n) - no need to look at the whole potentially huge string if you already have the limit. > + > + if (!length) > + return ERR_PTR(-EFAULT); > + > + if (length > n) > + length = n; This silently truncates a too long string, which might not be proper behavior (in fact, your patch #2 changes the behavior of keyctl functions, which were rejecting too long strings with -EINVAL - this is not good). > + > + p = kmalloc(length, GFP_KERNEL); > + > + if (!p) > + return ERR_PTR(-ENOMEM); > + > + if (strncpy_from_user(p, s, length) < 0) { This can be plain copy_from_user() - you already have the length, and even if someone sneaks a NUL byte in the middle, copying bytes past it will not matter. > + kfree(p); > + return ERR_PTR(-EFAULT); > + } > + > + p[length - 1] = '\0'; > + > + return p; > +} > +EXPORT_SYMBOL(strndup_user);
Attachment:
pgpXFJmlpqdbO.pgp
Description: PGP signature
- References:
- [PATCH 0/2] strndup_user, v2
- From: Davi Arnaut <[email protected]>
- [PATCH 0/2] strndup_user, v2
- Prev by Date: Re: [PATCH 1/2] strndup_user, convert (module)
- Next by Date: Re: Linux 2.6.16-rc3
- Previous by thread: Re: [PATCH 0/2] strndup_user, v2
- Next by thread: [PATCH 1/2] strndup_user, convert (module)
- Index(es):
 |