Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 25 Sep 2007 04:00:14 GMT
From:      Eugene Grosbein <eugen@kuzbass.ru>
To:        freebsd-bugs@FreeBSD.org
Subject:   Re: kern/116608: [panic] [patch] [msdosfs] msdosfs fails to checkmount options
Message-ID:  <200709250400.l8P40Ent027548@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/116608; it has been noted by GNATS.

From: Eugene Grosbein <eugen@kuzbass.ru>
To: bug-followup@freebsd.org
Cc:  
Subject: Re: kern/116608: [panic] [patch] [msdosfs] msdosfs fails to checkmount 
 options
Date: Tue, 25 Sep 2007 11:39:02 +0800

 Bruce Evans wrote:
 
 > >> Description:
 > >       Suppose, there is a line in /etc/fstab:
 > >
 > > /dev/md0 /mnt/tmp  msdosfs ro,noauto 0 0
 > >
 > >       The command 'mount /mnt/tmp' works all right.
 > >
 > >       One may try to use 'mount -o rw /mnt/tmp' when wishes
 > >       to mount it read-write initially. It works also, but
 > 
 > I think it should fail because it is missing -u, and -u is only added
 > automatically for root (and doesn't seem to be added for you).
 
 It's not about second mount, it's about initial mount - see How-To-Repeat
 section.
 
 > >> How-To-Repeat:
 > >
 > >       Let's make filesystem to play with (be ready for panic, though)
 > >
 > > mdconfig -a -t swap -s 1440k
 > > newfs_msdosfs -f 1440 /dev/md0
 > > mount -o ro -o rw /dev/md0 /mnt/tmp
 > >
 > >       (the point of no return)
 > >
 > > touch /mnt/tmp/file
 
 > >> Fix:
 > >
 > >       One way to fix this is to rely on vfs_donmount's processing
 > >       of mount options for MNT_RDONLY flag instead of using own version,
 > >       because this gives us the behavour we expect: an option that comes
 > >       from command line overrides one coming from fstab.
 > >
 > >       Note that this is partial backout (very little one)
 > >       of msdosfs_vfsops.c,1.134
 > >
 > > --- sys/fs/msdosfs/msdosfs_vfsops.c.orig      2007-09-24 22:16:52.000000000 +0800
 > > +++ sys/fs/msdosfs/msdosfs_vfsops.c   2007-09-24 22:49:37.000000000 +0800
 > > @@ -417,7 +417,7 @@
 > >       struct g_consumer *cp;
 > >       struct bufobj *bo;
 > >
 > > -     ronly = !vfs_getopt(mp->mnt_optnew, "ro", NULL, NULL);
 > > +     ronly = (mp->mnt_flag & MNT_RDONLY) != 0;
 > >       /* XXX: use VOP_ACCESS to check FS perms */
 > >       DROP_GIANT();
 > >       g_topology_lock();
 > 
 > This fix is already in -current (rev.1.167 = part of fixing root mounts;
 > for root mounts, no options are passed; MNT_RDONLY is passed, but when
 > it wasn't checked root on msdosfs was always mounted rw and so fsck -p
 > on it always failed, causing further problems).  I don't plan to MFC this
 > until after 7.0 is released.
 
 I do not expect that root mount of msdosfs would work for RELENG_6,
 but why not MFC this small fix for the PR?
 
 > ... Multiple mounts (that are not updates) of the same device are supposed
 > to work now, if they are all ro.  This case does sort of work (the mounts
 > work, but unmount followed by remount panics unless the unmount is in
 > stack order; also, mount -v i/o statistics depended on multiple mounts
 > not being supported and are now just null).  This seems to create problems
 > for your case of a second mount (that is not an update) of the same device
 > when the initial mount is ro and the second mount is rw.  I think think
 > this should fail with errno EBUSY like it is documented to and used to.  (All
 > attempts for multiple mounts are still documented to fail with errno EBUSY.
 > The case where the initial mount is rw and the second mount is anything
 > still fails, but with a broken errno, EPERM IIRC.  IIRC, the error is
 > direct from g_access().)  But for ro to rw without -u, the second mount
 > apparently tries to act like an updating mount.  I don't see how this can
 > be safe.  I say "apparent" because I haven't checked all the details and
 > am arguing based on your patch making a difference.  Your patch is for
 > mountmsdosfs(), which is only reached in the non-MNT_UPDATE case.  But
 > mountmsdosfs() doesn't do any of the things necessary for an updating
 > mount.  I think it just does a complete initial mount, leaving the ro
 > mount stacked underneath.  That is dangerous and shouldn't be allowed.
 > And how does mountmsdosfs() see the mount flags for the ro mount without
 > seeing that the new mount is incompatible with them?
 
 I'm not talking about second mount.
 
 Eugene



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200709250400.l8P40Ent027548>