From owner-freebsd-hackers@FreeBSD.ORG Mon Mar 1 18:47:15 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7635A1065769; Mon, 1 Mar 2010 18:47:15 +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 342698FC33; Mon, 1 Mar 2010 18:47:15 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id BFFC046B49; Mon, 1 Mar 2010 13:47:14 -0500 (EST) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPA id C30998A024; Mon, 1 Mar 2010 13:47:13 -0500 (EST) From: John Baldwin To: Juergen Lock Date: Mon, 1 Mar 2010 13:25:53 -0500 User-Agent: KMail/1.12.1 (FreeBSD/7.3-CBSD-20100217; KDE/4.3.1; amd64; ; ) References: <20100223215010.GA67619@triton8.kn-bremen.de> <201002261121.05068.jhb@freebsd.org> <20100226210952.GA2253@triton8.kn-bremen.de> In-Reply-To: <20100226210952.GA2253@triton8.kn-bremen.de> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201003011325.53617.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Mon, 01 Mar 2010 13:47:13 -0500 (EST) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-1.4 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: freebsd-hackers@freebsd.org, freebsd-emulation@freebsd.org, Tim Kientzle Subject: Re: 32 bit Linux lseek missing overflow check (was: Re: Linuxolator patches: stat and lseek SEEK_END for disk devices) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Mar 2010 18:47:15 -0000 On Friday 26 February 2010 4:09:52 pm Juergen Lock wrote: > On Fri, Feb 26, 2010 at 11:21:05AM -0500, John Baldwin wrote: > > On Thursday 25 February 2010 3:28:50 pm Juergen Lock wrote: > > > On Tue, Feb 23, 2010 at 10:50:10PM +0100, Juergen Lock wrote: > > > > Hi! > > > > > > > > Before this gets buried on -hackers in another thead... :) > > > > > > > > I now have disks appear as block devices for Linux processes (there > > > > already was commented out code for that in linux_stats.c, I hope my > > > > version is now `correct enough' to be usable [1]), and I made a simple > > > > patch to make lseek SEEK_END (L_XTND in the source) dtrt on disk > > > > devices too by simply invoking the DIOCGMEDIASIZE ioctl there; [2] > > > > both of these things are what (some) Linux processes expect. > > > > > > > > Patches are here: (made on stable/8, if they don't apply on head > > > > I'll have to make extra versions for that...) > > > > http://people.freebsd.org/~nox/linuxdisk-blk.patch [1] > > > > http://people.freebsd.org/~nox/lseek-seek_end.patch [2] > > > > > > > > And yes, with these patches the Linux bsdtar mentioned on -hackers > > > > in the `"tar tfv /dev/cd0" speedup patch' thread now also runs fast > > > > on FreeBSD. :) > > > > > > I now added an vn_isdisk() check to the second patch after comments from > > > julian, and I made a new patch that adds an overflow check to the 32 bit > > > linux lseek: (also at > > > http://people.freebsd.org/~nox/linux-lseek-overflow.patch > > > ) > > > > Hmm, when I asked Bruce, he actually said it was a feature that you didn't use > > vn_isdisk(). He also suggested that the proper way to fix lseek on devices is > > to fix stat in devfs to return a non-zero size. I have a possible fix to do > > that but haven't tested it yet: > > > > Index: devfs_vnops.c > > =================================================================== > > --- devfs_vnops.c (revision 204207) > > +++ devfs_vnops.c (working copy) > > @@ -44,6 +44,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -564,7 +565,11 @@ > > struct vattr *vap = ap->a_vap; > > int error = 0; > > struct devfs_dirent *de; > > + struct cdevsw *dsw; > > + struct thread *td; > > + struct file *fpop; > > struct cdev *dev; > > + off_t size; > > > > de = vp->v_data; > > KASSERT(de != NULL, ("Null dirent in devfs_getattr vp=%p", vp)); > > @@ -612,6 +617,18 @@ > > vap->va_ctime = dev->si_ctime; > > > > vap->va_rdev = cdev2priv(dev)->cdp_inode; > > + > > + dsw = dev_refthread(dev); > > + if (dsw != NULL) { > > + td = curthread; > > + fpop = td->td_fpop; > > + td->td_fpop = NULL; > > + if (dsw->d_ioctl(dev, DIOCGMEDIASIZE, (caddr_t)&size, > > + FREAD, td) == 0) > > + vap->va_size = size; > > + td->td_fpop = fpop; > > + dev_relthread(dev); > > + } > > } > > vap->va_gen = 0; > > vap->va_flags = 0; > > > > I had to add an D_DISK check, else it would panic at boot in a revoke > syscall in ttydev_ioctl in ttydev_enter, apparently in the tty_lock call: > (I didn't get a dump as it panic'd way too early in boot) > http://fxr.watson.org/fxr/source/kern/tty.c#L159 > > Maybe dev->si_drv1 is null here? > http://fxr.watson.org/fxr/source/kern/tty.c#L486 > > Anyway, with the D_DISK check added the patch seems to work: Unfortunately it panics in g_dev_ioctl() with INVARIANTS enabled. I will bug phk about that. -- John Baldwin