From mboxrd@z Thu Jan 1 00:00:00 1970 From: ebiederm@xmission.com (Eric W. Biederman) Date: Tue, 30 Mar 2010 16:25:01 +0000 Subject: Re: [patch] s390: potential buffer overflow Message-Id: List-Id: References: <20100330094342.GJ5069@bicker> In-Reply-To: <20100330094342.GJ5069@bicker> (Dan Carpenter's message of "Tue\, 30 Mar 2010 12\:43\:42 +0300") MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Dan Carpenter Cc: Martin Schwidefsky , Heiko Carstens , linux390@de.ibm.com, Hans-Joachim Picht , Sebastian Ott , linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Dan Carpenter writes: > "len" hasn't been properly range checked so we shouldn't use it as an > array offset. This can only be written to by root but it would still be > annoying to accidentally write more than 3 characters and corrupt your > memory. > > Signed-off-by: Dan Carpenter > --- > I don't have a cross compile environment to compile this. Sorry. :/ > > diff --git a/drivers/s390/char/sclp_async.c b/drivers/s390/char/sclp_async.c > index f449c69..72c2e90 100644 > --- a/drivers/s390/char/sclp_async.c > +++ b/drivers/s390/char/sclp_async.c > @@ -84,7 +84,7 @@ static int proc_handler_callhome(struct ctl_table *ctl, int write, > rc = copy_from_user(buf, buffer, sizeof(buf)); > if (rc != 0) > return -EFAULT; > - buf[len - 1] = '\0'; > + buf[sizeof(buf) - 1] = '\0'; > if (strict_strtoul(buf, 0, &val) != 0) > return -EINVAL; > if (val != 0 && val != 1) > @@ -99,6 +99,7 @@ static int proc_handler_callhome(struct ctl_table *ctl, int write, > static struct ctl_table callhome_table[] = { > { > .procname = "callhome", > + .maxlen = 3, > .mode = 0644, > .proc_handler = proc_handler_callhome, > }, Setting maxlen won't do any good as proc_handler_callhome doesn't look at it. Eric