Messages in this thread | | | Date | Mon, 27 Oct 2008 18:54:18 -0700 | From | Andrew Morton <> | Subject | Re: [RESEND PATCH] autofs4 - remove string terminator check |
| |
On Tue, 28 Oct 2008 10:14:30 +0900 Ian Kent <raven@themaw.net> wrote:
> In a previous patch a comment was made that checking for the existence of > a NULL terminator in strings copied from userspace wasn't needed as this > is done in many places in the kernel without problem. This patch removes > this string terminator check. >
ah, OK. Now I'm worried.
> > fs/autofs4/dev-ioctl.c | 20 -------------------- > 1 files changed, 0 insertions(+), 20 deletions(-) > > diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c > index 625abf5..304c1ff 100644 > --- a/fs/autofs4/dev-ioctl.c > +++ b/fs/autofs4/dev-ioctl.c > @@ -51,18 +51,6 @@ static int check_name(const char *name) > } > > /* > - * Check a string doesn't overrun the chunk of > - * memory we copied from user land. > - */ > -static int invalid_str(char *str, void *end) > -{ > - while ((void *) str <= end) > - if (!*str++) > - return 0; > - return -EINVAL; > -} > - > -/* > * Check that the user compiled against correct version of autofs > * misc device code. > * > @@ -143,14 +131,6 @@ static int validate_dev_ioctl(int cmd, struct autofs_dev_ioctl *param) > cmd); > goto out; > } > - > - err = invalid_str(param->path, > - (void *) ((size_t) param + param->size)); > - if (err) { > - AUTOFS_WARN("invalid path supplied for cmd(0x%08x)", > - cmd); > - goto out; > - } > } > > err = 0;
What will now happen if userspace passes in a non-null-terminated string (if that's possible)?
Presumably that isn't possible, or it's never been tested, because before we check for null-termination we run check_name(), which _assumes_ null-termination!
The comment over validate_dev_ioctl() will need the "and is terminated" removed after this change, yes?
| |