32 bit Linux lseek missing overflow check (was: Re: Linuxolator
patches: stat and lseek SEEK_END for disk devices)
John Baldwin
jhb at freebsd.org
Mon Mar 1 18:47:15 UTC 2010
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 <sys/systm.h>
> > #include <sys/conf.h>
> > #include <sys/dirent.h>
> > +#include <sys/disk.h>
> > #include <sys/fcntl.h>
> > #include <sys/file.h>
> > #include <sys/filedesc.h>
> > @@ -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
More information about the freebsd-hackers
mailing list