From owner-freebsd-hackers@FreeBSD.ORG Fri Feb 26 21:11:57 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 3E988106566C; Fri, 26 Feb 2010 21:11:57 +0000 (UTC) (envelope-from nox@jelal.kn-bremen.de) Received: from smtp.kn-bremen.de (gelbbaer.kn-bremen.de [78.46.108.116]) by mx1.freebsd.org (Postfix) with ESMTP id D601A8FC1C; Fri, 26 Feb 2010 21:11:56 +0000 (UTC) Received: by smtp.kn-bremen.de (Postfix, from userid 10) id E62D51E0014B; Fri, 26 Feb 2010 22:11:55 +0100 (CET) Received: from triton8.kn-bremen.de (noident@localhost [127.0.0.1]) by triton8.kn-bremen.de (8.14.3/8.14.3) with ESMTP id o1QL9qH6002505; Fri, 26 Feb 2010 22:09:52 +0100 (CET) (envelope-from nox@triton8.kn-bremen.de) Received: (from nox@localhost) by triton8.kn-bremen.de (8.14.3/8.14.3/Submit) id o1QL9qvY002504; Fri, 26 Feb 2010 22:09:52 +0100 (CET) (envelope-from nox) From: Juergen Lock Date: Fri, 26 Feb 2010 22:09:52 +0100 To: John Baldwin Message-ID: <20100226210952.GA2253@triton8.kn-bremen.de> References: <20100223215010.GA67619@triton8.kn-bremen.de> <20100225202850.GA79505@triton8.kn-bremen.de> <201002261121.05068.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201002261121.05068.jhb@freebsd.org> User-Agent: Mutt/1.5.20 (2009-06-14) X-Mailman-Approved-At: Fri, 26 Feb 2010 21:19:22 +0000 Cc: freebsd-hackers@freebsd.org, freebsd-emulation@freebsd.org, Tim Kientzle , Juergen Lock 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: Fri, 26 Feb 2010 21:11:57 -0000 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: Index: src/sys/fs/devfs/devfs_vnops.c =================================================================== RCS file: /home/scvs/src/sys/fs/devfs/devfs_vnops.c,v retrieving revision 1.181.2.1 diff -u -p -p -u -r1.181.2.1 devfs_vnops.c --- src/sys/fs/devfs/devfs_vnops.c 3 Aug 2009 08:13:06 -0000 1.181.2.1 +++ src/sys/fs/devfs/devfs_vnops.c 26 Feb 2010 20:46:17 -0000 @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -563,7 +564,11 @@ devfs_getattr(struct vop_getattr_args *a 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)); @@ -611,6 +616,19 @@ devfs_getattr(struct vop_getattr_args *a 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_flags & D_DISK) && + 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;