lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 28 Oct 2008 11:07:23 +0900 From: Ian Kent <raven@...maw.net> To: Andrew Morton <akpm@...ux-foundation.org> Cc: autofs mailing list <autofs@...ux.kernel.org>, Kernel Mailing List <linux-kernel@...r.kernel.org>, linux-fsdevel <linux-fsdevel@...r.kernel.org> Subject: Re: [RESEND PATCH] autofs4 - remove string terminator check On Mon, 2008-10-27 at 18:54 -0700, Andrew Morton wrote: > On Tue, 28 Oct 2008 10:14:30 +0900 Ian Kent <raven@...maw.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? Yes, but now I think I shouldn't have removed it. For my part I would have been happy to keep this and now I think I should so let's drop this patch. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists