Bug 133614 - [panic] panic: ffs_truncate: read-only filesystem
Summary: [panic] panic: ffs_truncate: read-only filesystem
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 7.2-PRERELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Jaakko Heinonen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-11 22:30 UTC by Mikhail T.
Modified: 2011-07-26 17:31 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail T. 2009-04-11 22:30:04 UTC
	Hello! I have a filesystem, that is mounted read-only by
	default:

/dev/ad10		/ad10		ufs	ro		0	1

	It is holding data, that's modified infrequently and I thought,
	keeping it RO most of the time will be safer.

	I had to add a few (large) files recently and remounted the FS
	in read-write mode. "To be sure", I did an umount, fsck, and
	rw-mount:

		umount /ad10
		fsck -y /ad10	# No errors reported by fsck
		mount -orw /ad10
		mount	# See /ad10 mounted as (ufs, local, soft-updates)

	When I then proceeded to write to the filesystem (via Samba),
	the system paniced as follows:

Unread portion of the kernel message buffer:
panic: ffs_truncate: read-only filesystem
cpuid = 3
Uptime: 35m50s
Physical memory: 5911 MB
Dumping 519 MB: 504 488 472 456 440 424 408 392 376 360 344 328 312 296 280 264 248 232 216 200 184 168 152 136 120 104 88 72 56 40 24 8Attempt to write outside dump device boundaries.

** DUMP FAILED (ERROR 6) **
Automatic reboot in 15 seconds - press a key on the console to abort
fs = /ad10
panic: ffs_sync: rofs mod
cpuid = 3
Rebooting...
cpu_reset: Stopping other CPUs
Uptime: 36m33s
Physical memory: 5911 MB
Dumping 519 MB: 504 488 472 456 440 424 408 392 376 360 344 328 312 296 280 264 248 232 216 200 184 168 152 136 120 104 88 72 56 40 24 8

Reading symbols from /opt/modules/rtc.ko...done.
Loaded symbols for /opt/modules/rtc.ko
#0  doadump () at pcpu.h:195
195		__asm __volatile("movq %%gs:0,%0" : "=r" (td));
(kgdb) 

	Please, advise... Thanks!

Fix: 

I suppose, I could just be mounting it RW from the beginning,
	but it was certainly nicer to be able to switch to/from RW
	on the fly...
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2009-04-11 22:40:22 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-fs

Over to maintainer(s).
Comment 2 Jaakko Heinonen freebsd_committer freebsd_triage 2009-10-29 17:02:00 UTC
The bug has been described in this message:

http://docs.freebsd.org/cgi/mid.cgi?20090910091329.GA2726

There is a workaround patch available in this message:

http://docs.freebsd.org/cgi/mid.cgi?20090914173208.GA4273
Comment 3 dfilter service freebsd_committer freebsd_triage 2011-02-19 14:27:20 UTC
Author: jh
Date: Sat Feb 19 14:27:14 2011
New Revision: 218852
URL: http://svn.freebsd.org/changeset/base/218852

Log:
  Don't restore old mount options and flags if VFS_MOUNT(9) succeeds but
  vfs_export() fails. Restoring old options and flags after successful
  VFS_MOUNT(9) call may cause the file system internal state to become
  inconsistent with mount options and flags. Specifically the FFS super
  block fs_ronly field and the MNT_RDONLY flag may get out of sync.
  
  PR:		kern/133614
  Discussed on:	freebsd-hackers

Modified:
  head/sys/kern/vfs_mount.c

Modified: head/sys/kern/vfs_mount.c
==============================================================================
--- head/sys/kern/vfs_mount.c	Sat Feb 19 13:23:13 2011	(r218851)
+++ head/sys/kern/vfs_mount.c	Sat Feb 19 14:27:14 2011	(r218852)
@@ -70,7 +70,7 @@ __FBSDID("$FreeBSD$");
 #define	VFS_MOUNTARG_SIZE_MAX	(1024 * 64)
 
 static int	vfs_domount(struct thread *td, const char *fstype,
-		    char *fspath, int fsflags, void *fsdata);
+		    char *fspath, int fsflags, struct vfsoptlist **optlist);
 static void	free_mntarg(struct mntarg *ma);
 
 static int	usermount = 0;
@@ -667,7 +667,7 @@ vfs_donmount(struct thread *td, int fsfl
 		goto bail;
 	}
 
-	error = vfs_domount(td, fstype, fspath, fsflags, optlist);
+	error = vfs_domount(td, fstype, fspath, fsflags, &optlist);
 bail:
 	/* copyout the errmsg */
 	if (errmsg_pos != -1 && ((2 * errmsg_pos + 1) < fsoptions->uio_iovcnt)
@@ -683,7 +683,7 @@ bail:
 		}
 	}
 
-	if (error != 0)
+	if (optlist != NULL)
 		vfs_freeopts(optlist);
 	return (error);
 }
@@ -762,12 +762,12 @@ mount(td, uap)
  */
 static int
 vfs_domount_first(
-	struct thread *td,	/* Calling thread. */
-	struct vfsconf *vfsp,	/* File system type. */
-	char *fspath,		/* Mount path. */
-	struct vnode *vp,	/* Vnode to be covered. */
-	int fsflags,		/* Flags common to all filesystems. */
-	void *fsdata		/* Options local to the filesystem. */
+	struct thread *td,		/* Calling thread. */
+	struct vfsconf *vfsp,		/* File system type. */
+	char *fspath,			/* Mount path. */
+	struct vnode *vp,		/* Vnode to be covered. */
+	int fsflags,			/* Flags common to all filesystems. */
+	struct vfsoptlist **optlist	/* Options local to the filesystem. */
 	)
 {
 	struct vattr va;
@@ -807,7 +807,7 @@ vfs_domount_first(
 	/* Allocate and initialize the filesystem. */
 	mp = vfs_mount_alloc(vp, vfsp, fspath, td->td_ucred);
 	/* XXXMAC: pass to vfs_mount_alloc? */
-	mp->mnt_optnew = fsdata;
+	mp->mnt_optnew = *optlist;
 	/* Set the mount level flags. */
 	mp->mnt_flag = (fsflags & (MNT_UPDATEMASK | MNT_ROOTFS | MNT_RDONLY));
 
@@ -830,6 +830,7 @@ vfs_domount_first(
 	if (mp->mnt_opt != NULL)
 		vfs_freeopts(mp->mnt_opt);
 	mp->mnt_opt = mp->mnt_optnew;
+	*optlist = NULL;
 	(void)VFS_STATFS(mp, &mp->mnt_stat);
 
 	/*
@@ -872,16 +873,16 @@ vfs_domount_first(
  */
 static int
 vfs_domount_update(
-	struct thread *td,	/* Calling thread. */
-	struct vnode *vp,	/* Mount point vnode. */
-	int fsflags,		/* Flags common to all filesystems. */
-	void *fsdata		/* Options local to the filesystem. */
+	struct thread *td,		/* Calling thread. */
+	struct vnode *vp,		/* Mount point vnode. */
+	int fsflags,			/* Flags common to all filesystems. */
+	struct vfsoptlist **optlist	/* Options local to the filesystem. */
 	)
 {
 	struct oexport_args oexport;
 	struct export_args export;
 	struct mount *mp;
-	int error, flag;
+	int error, export_error, flag;
 
 	mtx_assert(&Giant, MA_OWNED);
 	ASSERT_VOP_ELOCKED(vp, __func__);
@@ -932,7 +933,7 @@ vfs_domount_update(
 	if ((mp->mnt_flag & MNT_ASYNC) == 0)
 		mp->mnt_kern_flag &= ~MNTK_ASYNC;
 	MNT_IUNLOCK(mp);
-	mp->mnt_optnew = fsdata;
+	mp->mnt_optnew = *optlist;
 	vfs_mergeopts(mp->mnt_optnew, mp->mnt_opt);
 
 	/*
@@ -942,11 +943,12 @@ vfs_domount_update(
 	 */
 	error = VFS_MOUNT(mp);
 
+	export_error = 0;
 	if (error == 0) {
 		/* Process the export option. */
 		if (vfs_copyopt(mp->mnt_optnew, "export", &export,
 		    sizeof(export)) == 0) {
-			error = vfs_export(mp, &export);
+			export_error = vfs_export(mp, &export);
 		} else if (vfs_copyopt(mp->mnt_optnew, "export", &oexport,
 		    sizeof(oexport)) == 0) {
 			export.ex_flags = oexport.ex_flags;
@@ -958,7 +960,7 @@ vfs_domount_update(
 			export.ex_masklen = oexport.ex_masklen;
 			export.ex_indexfile = oexport.ex_indexfile;
 			export.ex_numsecflavors = 0;
-			error = vfs_export(mp, &export);
+			export_error = vfs_export(mp, &export);
 		}
 	}
 
@@ -988,6 +990,7 @@ vfs_domount_update(
 	if (mp->mnt_opt != NULL)
 		vfs_freeopts(mp->mnt_opt);
 	mp->mnt_opt = mp->mnt_optnew;
+	*optlist = NULL;
 	(void)VFS_STATFS(mp, &mp->mnt_stat);
 	/*
 	 * Prevent external consumers of mount options from reading
@@ -1005,7 +1008,7 @@ end:
 	vp->v_iflag &= ~VI_MOUNT;
 	VI_UNLOCK(vp);
 	vrele(vp);
-	return (error);
+	return (error != 0 ? error : export_error);
 }
 
 /*
@@ -1013,11 +1016,11 @@ end:
  */
 static int
 vfs_domount(
-	struct thread *td,	/* Calling thread. */
-	const char *fstype,	/* Filesystem type. */
-	char *fspath,		/* Mount path. */
-	int fsflags,		/* Flags common to all filesystems. */
-	void *fsdata		/* Options local to the filesystem. */
+	struct thread *td,		/* Calling thread. */
+	const char *fstype,		/* Filesystem type. */
+	char *fspath,			/* Mount path. */
+	int fsflags,			/* Flags common to all filesystems. */
+	struct vfsoptlist **optlist	/* Options local to the filesystem. */
 	)
 {
 	struct vfsconf *vfsp;
@@ -1087,9 +1090,9 @@ vfs_domount(
 	vp = nd.ni_vp;
 	if ((fsflags & MNT_UPDATE) == 0) {
 		error = vfs_domount_first(td, vfsp, fspath, vp, fsflags,
-		    fsdata);
+		    optlist);
 	} else {
-		error = vfs_domount_update(td, vp, fsflags, fsdata);
+		error = vfs_domount_update(td, vp, fsflags, optlist);
 	}
 	mtx_unlock(&Giant);
 
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 4 Eitan Adler freebsd_committer freebsd_triage 2011-03-01 15:15:49 UTC
State Changed
From-To: open->patched

committed in head (r218852)
Comment 5 Eitan Adler freebsd_committer freebsd_triage 2011-03-01 15:23:09 UTC
Responsible Changed
From-To: freebsd-fs->jh

same as above
Comment 6 dfilter service freebsd_committer freebsd_triage 2011-03-23 17:56:52 UTC
Author: jh
Date: Wed Mar 23 17:56:38 2011
New Revision: 219925
URL: http://svn.freebsd.org/changeset/base/219925

Log:
  Recognize "ro", "rdonly", "norw", "rw" and "noro" as equal options in
  vfs_equalopts(). This allows vfs_sanitizeopts() to filter redundant
  occurrences of these options. It was possible that for example both "ro"
  and "rw" options became active concurrently.
  
  PR:		kern/133614
  Discussed on:	freebsd-hackers
  MFC after:	1 month

Modified:
  head/sys/kern/vfs_mount.c

Modified: head/sys/kern/vfs_mount.c
==============================================================================
--- head/sys/kern/vfs_mount.c	Wed Mar 23 17:33:04 2011	(r219924)
+++ head/sys/kern/vfs_mount.c	Wed Mar 23 17:56:38 2011	(r219925)
@@ -175,6 +175,27 @@ vfs_deleteopt(struct vfsoptlist *opts, c
 	}
 }
 
+static int
+vfs_isopt_ro(const char *opt)
+{
+
+	if (strcmp(opt, "ro") == 0 || strcmp(opt, "rdonly") == 0 ||
+	    strcmp(opt, "norw") == 0)
+		return (1);
+	else
+		return (0);
+}
+
+static int
+vfs_isopt_rw(const char *opt)
+{
+
+	if (strcmp(opt, "rw") == 0 || strcmp(opt, "noro") == 0)
+		return (1);
+	else
+		return (0);
+}
+
 /*
  * Check if options are equal (with or without the "no" prefix).
  */
@@ -203,6 +224,11 @@ vfs_equalopts(const char *opt1, const ch
 		if (strncmp(opt2, "no", 2) == 0 && strcmp(opt1, opt2 + 2) == 0)
 			return (1);
 	}
+	/* "ro" / "rdonly" / "norw" / "rw" / "noro" */
+	if ((vfs_isopt_ro(opt1) || vfs_isopt_rw(opt1)) &&
+	    (vfs_isopt_ro(opt2) || vfs_isopt_rw(opt2)))
+		return (1);
+
 	return (0);
 }
 
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 7 dfilter service freebsd_committer freebsd_triage 2011-06-13 16:54:10 UTC
Author: jh
Date: Mon Jun 13 15:53:56 2011
New Revision: 223050
URL: http://svn.freebsd.org/changeset/base/223050

Log:
  MFC r219925:
  
  Recognize "ro", "rdonly", "norw", "rw" and "noro" as equal options in
  vfs_equalopts(). This allows vfs_sanitizeopts() to filter redundant
  occurrences of these options. It was possible that for example both "ro"
  and "rw" options became active concurrently.
  
  PR:		kern/133614
  
  MFC r220040:
  
  Fix some style issues in r219925.

Modified:
  stable/8/sys/kern/vfs_mount.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)

Modified: stable/8/sys/kern/vfs_mount.c
==============================================================================
--- stable/8/sys/kern/vfs_mount.c	Mon Jun 13 15:38:31 2011	(r223049)
+++ stable/8/sys/kern/vfs_mount.c	Mon Jun 13 15:53:56 2011	(r223050)
@@ -192,6 +192,25 @@ vfs_deleteopt(struct vfsoptlist *opts, c
 	}
 }
 
+static int
+vfs_isopt_ro(const char *opt)
+{
+
+	if (strcmp(opt, "ro") == 0 || strcmp(opt, "rdonly") == 0 ||
+	    strcmp(opt, "norw") == 0)
+		return (1);
+	return (0);
+}
+
+static int
+vfs_isopt_rw(const char *opt)
+{
+
+	if (strcmp(opt, "rw") == 0 || strcmp(opt, "noro") == 0)
+		return (1);
+	return (0);
+}
+
 /*
  * Check if options are equal (with or without the "no" prefix).
  */
@@ -220,6 +239,10 @@ vfs_equalopts(const char *opt1, const ch
 		if (strncmp(opt2, "no", 2) == 0 && strcmp(opt1, opt2 + 2) == 0)
 			return (1);
 	}
+	/* "ro" / "rdonly" / "norw" / "rw" / "noro" */
+	if ((vfs_isopt_ro(opt1) || vfs_isopt_rw(opt1)) &&
+	    (vfs_isopt_ro(opt2) || vfs_isopt_rw(opt2)))
+		return (1);
 	return (0);
 }
 
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 8 dfilter service freebsd_committer freebsd_triage 2011-07-26 17:15:11 UTC
Author: jh
Date: Tue Jul 26 16:14:57 2011
New Revision: 224449
URL: http://svn.freebsd.org/changeset/base/224449

Log:
  MFC r219925:
  
  Recognize "ro", "rdonly", "norw", "rw" and "noro" as equal options in
  vfs_equalopts(). This allows vfs_sanitizeopts() to filter redundant
  occurrences of these options. It was possible that for example both "ro"
  and "rw" options became active concurrently.
  
  PR:		kern/133614
  
  MFC r220040:
  
  Fix some style issues in r219925.

Modified:
  stable/7/sys/kern/vfs_mount.c
Directory Properties:
  stable/7/sys/   (props changed)
  stable/7/sys/cddl/contrib/opensolaris/   (props changed)
  stable/7/sys/contrib/dev/acpica/   (props changed)
  stable/7/sys/contrib/pf/   (props changed)

Modified: stable/7/sys/kern/vfs_mount.c
==============================================================================
--- stable/7/sys/kern/vfs_mount.c	Tue Jul 26 14:41:54 2011	(r224448)
+++ stable/7/sys/kern/vfs_mount.c	Tue Jul 26 16:14:57 2011	(r224449)
@@ -195,6 +195,25 @@ vfs_deleteopt(struct vfsoptlist *opts, c
 	}
 }
 
+static int
+vfs_isopt_ro(const char *opt)
+{
+
+	if (strcmp(opt, "ro") == 0 || strcmp(opt, "rdonly") == 0 ||
+	    strcmp(opt, "norw") == 0)
+		return (1);
+	return (0);
+}
+
+static int
+vfs_isopt_rw(const char *opt)
+{
+
+	if (strcmp(opt, "rw") == 0 || strcmp(opt, "noro") == 0)
+		return (1);
+	return (0);
+}
+
 /*
  * Check if options are equal (with or without the "no" prefix).
  */
@@ -211,6 +230,10 @@ vfs_equalopts(const char *opt1, const ch
 	/* "opt" vs. "noopt" */
 	if (strncmp(opt2, "no", 2) == 0 && strcmp(opt1, opt2 + 2) == 0)
 		return (1);
+	/* "ro" / "rdonly" / "norw" / "rw" / "noro" */
+	if ((vfs_isopt_ro(opt1) || vfs_isopt_rw(opt1)) &&
+	    (vfs_isopt_ro(opt2) || vfs_isopt_rw(opt2)))
+		return (1);
 	return (0);
 }
 
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 9 Jaakko Heinonen freebsd_committer freebsd_triage 2011-07-26 17:30:58 UTC
State Changed
From-To: patched->closed

Fixed in head and worked around in stable/8 and stable/7.