kern/116608: [panic] [patch] [msdosfs] msdosfs fails to check mount options

[Eugene Grosbein]

>Number:         116608
>Category:       kern
>Synopsis:       [panic] [patch] [msdosfs] msdosfs fails to check mount options
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Mon Sep 24 15:30:02 GMT 2007
>Closed-Date:
>Last-Modified:
>Originator:     Eugene Grosbein
>Release:        FreeBSD 6.2-STABLE i386
>Organization:
Svyaz-Service JSC
>Environment:
System: FreeBSD grosbein.pp.ru 6.2-STABLE FreeBSD 6.2-STABLE #3: Mon Sep 24 17:05:42 KRAST 2007 eu at grosbein.pp.ru:/home/obj/usr/local/src/sys/DADV i386

>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
	any write access to the filesystem returns 'Permission denied'
	from geom layer, so filesystem cannot be unmounted and
	kernel panic is imminent. The reason is that latter command
	translates to 'mount_msdosfs -o ro -o rw /mnt/tmp'
	and vfs_donmount() clears MNT_RDONLY flag for this mount.

	But msdosfs code checks for "ro" option (and does no check for "rw")
	and passes read-only indicator to g_vfs_open().

>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

	Here you'll get EPERM for touch and errors from geom like this:

g_vfs_done():md0[WRITE(offset=XXX, length=YYY)]error = 1

	We made it dirty and won't be able to flush buffer,
	so there will be a panic.

>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();


>Release-Note:
>Audit-Trail:
>Unformatted: