From owner-freebsd-bugs@FreeBSD.ORG Tue Sep 25 04:00:15 2007 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3239716A41B for ; Tue, 25 Sep 2007 04:00:15 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 140C713C455 for ; Tue, 25 Sep 2007 04:00:15 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.14.1/8.14.1) with ESMTP id l8P40ENT027549 for ; Tue, 25 Sep 2007 04:00:14 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.1/8.14.1/Submit) id l8P40Ent027548; Tue, 25 Sep 2007 04:00:14 GMT (envelope-from gnats) Date: Tue, 25 Sep 2007 04:00:14 GMT Message-Id: <200709250400.l8P40Ent027548@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Eugene Grosbein Cc: Subject: Re: kern/116608: [panic] [patch] [msdosfs] msdosfs fails to checkmount options X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Eugene Grosbein List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Sep 2007 04:00:15 -0000 The following reply was made to PR kern/116608; it has been noted by GNATS. From: Eugene Grosbein 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