kern/116608: [panic] [patch] [msdosfs] msdosfs fails to
checkmount options
Eugene Grosbein
eugen at kuzbass.ru
Mon Sep 24 21:00:15 PDT 2007
The following reply was made to PR kern/116608; it has been noted by GNATS.
From: Eugene Grosbein <eugen at kuzbass.ru>
To: bug-followup at 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
More information about the freebsd-bugs
mailing list