From owner-freebsd-stable@FreeBSD.ORG Wed May 20 14:02:23 2009 Return-Path: Delivered-To: freebsd-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 063681065726; Wed, 20 May 2009 14:02:22 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 8C9C38FC16; Wed, 20 May 2009 14:02:22 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 4178C46B52; Wed, 20 May 2009 10:02:22 -0400 (EDT) Received: from jhbbsd.hudson-trading.com (unknown [209.249.190.8]) by bigwig.baldwin.cx (Postfix) with ESMTPA id 003408A028; Wed, 20 May 2009 10:02:21 -0400 (EDT) From: John Baldwin To: Andriy Gapon Date: Wed, 20 May 2009 08:32:00 -0400 User-Agent: KMail/1.9.7 References: <4A1165A0.6000800@icyb.net.ua> <4A1167A2.6040407@freebsd.org> <4A13F30A.2090401@freebsd.org> In-Reply-To: <4A13F30A.2090401@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200905200832.00771.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Wed, 20 May 2009 10:02:21 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00,RDNS_NONE autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: freebsd-stable@freebsd.org, Chagin Dmitry Subject: Re: stack abuse by linux_ioctl_cdrom X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 May 2009 14:02:23 -0000 On Wednesday 20 May 2009 8:09:46 am Andriy Gapon wrote: > > This is a patch that I currently use to fix the problem for myself - both 2KB > structs are allocated on the heap. > I am not sure what is the proper style for chained calls using chained if-else, > but I think that the chaining is the best way to organize that piece of code, so > that there is only one exit point from case-block to make sure that FREE is always > called. I usually use goto for that. Error handling does seem to be one of the few appropriate uses of goto. In this case you would basically be replacing all the 'break's with 'goto out' or some such. Also, please do not use the MALLOC() or FREE() macros in new code as they are deprecated (I think they are completely removed in 8). > diff --git a/sys/compat/linux/linux_ioctl.c b/sys/compat/linux/linux_ioctl.c > index 8e42ec1..7e3453c 100644 > --- a/sys/compat/linux/linux_ioctl.c > +++ b/sys/compat/linux/linux_ioctl.c > @@ -1538,23 +1538,28 @@ linux_ioctl_cdrom(struct thread *td, struct > linux_ioctl_args *args) > /* LINUX_CDROMAUDIOBUFSIZ */ > > case LINUX_DVD_READ_STRUCT: { > - l_dvd_struct lds; > - struct dvd_struct bds; > + l_dvd_struct *p_lds; > + struct dvd_struct *p_bds; > > - error = copyin((void *)args->arg, &lds, sizeof(lds)); > - if (error) > - break; > - error = linux_to_bsd_dvd_struct(&lds, &bds); > - if (error) > - break; > - error = fo_ioctl(fp, DVDIOCREADSTRUCTURE, (caddr_t)&bds, > - td->td_ucred, td); > - if (error) > - break; > - error = bsd_to_linux_dvd_struct(&bds, &lds); > - if (error) > - break; > - error = copyout(&lds, (void *)args->arg, sizeof(lds)); > + MALLOC(p_lds, l_dvd_struct *, sizeof(*p_lds), > + M_LINUX, M_WAITOK); > + MALLOC(p_bds, struct dvd_struct *, sizeof(*p_bds), > + M_LINUX, M_WAITOK); > + if ((error = copyin((void *)args->arg, p_lds, sizeof(*p_lds))) > + != 0) > + ; /* nothing */ > + else if ((error = linux_to_bsd_dvd_struct(p_lds, p_bds)) != 0) > + ; /* nothing */ > + else if ((error = fo_ioctl(fp, DVDIOCREADSTRUCTURE, > + (caddr_t)p_bds, td->td_ucred, td)) != 0) > + ; /* nothing */ > + else if ((error = bsd_to_linux_dvd_struct(p_bds, p_lds)) != 0) > + ; /* nothing */ > + else > + error = copyout(p_lds, (void *)args->arg, > + sizeof(*p_lds)); > + FREE(p_bds, M_LINUX); > + FREE(p_lds, M_LINUX); > break; > } > > > > > -- > Andriy Gapon > -- John Baldwin