Re: [PATCH 0/2] strndup_user, v2

[Davi Arnaut <davi.arnaut@gmail.com>]

On Thu, 16 Feb 2006 01:25:56 +0000
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> On Mer, 2006-02-15 at 18:22 -0300, Davi Arnaut wrote:
> > +static inline char *strdup_user(const char __user *s)
> > +{
> > +	return strndup_user(s, 4096);
> > +}
> 
> Still shouldn't exist. Its just a bad idea to give people broken
> function they don't yet use.

Ok, I will remove it. But it's a sane default, if someone wants more
than 4096, they should use strndup_user.
 
> > +	length = strlen_user(s);
> 
> Should use strnlen_user or this function is useless for most cases.

Ok.

> > +
> > +	if (!length)
> > +		return ERR_PTR(-EFAULT);
> 
> Zero isn't an -EFAULT length. Its a null string and valid

strlen_user returns _0_ on exception. If you don't belive me,
kernel/module.c or arch/x86_64/lib/usercopy.c are a good starting
point.

> > +
> > +	if (length > n)
> > +		length = n;
> > +
> > +	p = kmalloc(length, GFP_KERNEL);
> > +
> > +	if (!p)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	if (strncpy_from_user(p, s, length) < 0) {
> > +		kfree(p);
> > +		return ERR_PTR(-EFAULT);
> > +	}
> > +
> > +	p[length - 1] = '\0';
> 
> And still broken.
> 
> "Hello" -> length = 5   "Hello\0"[4] = 0 "Hell"
> 

NO! strlen_user("Hello") -> length = 6

strlen_user returns the size of the string INCLUDING the
terminating NUL.

Are we talking in the same language ? 

--
Davi Arnaut