git: 7c7a6681fab2 - main - ffs: clear MNT_SOFTDEP earlier when remounting rw to ro

Konstantin Belousov kib at FreeBSD.org
Fri Mar 12 11:32:26 UTC 2021


The branch main has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=7c7a6681fab2c0453085d30424f479c0f766904d

commit 7c7a6681fab2c0453085d30424f479c0f766904d
Author:     Konstantin Belousov <kib at FreeBSD.org>
AuthorDate: 2021-02-28 18:55:35 +0000
Commit:     Konstantin Belousov <kib at FreeBSD.org>
CommitDate: 2021-03-12 11:31:07 +0000

    ffs: clear MNT_SOFTDEP earlier when remounting rw to ro
    
    Suppose that we remount rw->ro and in parallel some reader tries to
    instantiate a vnode, e.g. during lookup.  Suppose that softdep_unmount()
    already started, but we did not cleared the MNT_SOFTDEP flag yet.
    Then ffs_vgetf() calls into softdep_load_inodeblock() which accessed
    destroyed hashes and freed memory.
    
    Set/clear fs_ronly simultaneously (WRT to files flush) with MNT_SOFTDEP.
    It might be reasonable to move the change of fs_ronly to under MNT_ILOCK,
    but no readers take it.
    
    Reported and tested by: pho
    Reviewed by:    mckusick
    Sponsored by:   The FreeBSD Foundation
    MFC after:      2 weeks
    Differential revision:  https://reviews.freebsd.org/D29178
---
 sys/ufs/ffs/ffs_softdep.c | 21 ++++++++++++---------
 sys/ufs/ffs/ffs_vfsops.c  | 28 +++++++++++++++++++++++++---
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/sys/ufs/ffs/ffs_softdep.c b/sys/ufs/ffs/ffs_softdep.c
index 519a3679cac9..af5b9f57b328 100644
--- a/sys/ufs/ffs/ffs_softdep.c
+++ b/sys/ufs/ffs/ffs_softdep.c
@@ -1792,10 +1792,10 @@ softdep_process_worklist(mp, full)
 	long starttime;
 
 	KASSERT(mp != NULL, ("softdep_process_worklist: NULL mp"));
-	if (MOUNTEDSOFTDEP(mp) == 0)
+	ump = VFSTOUFS(mp);
+	if (ump->um_softdep == NULL)
 		return (0);
 	matchcnt = 0;
-	ump = VFSTOUFS(mp);
 	ACQUIRE_LOCK(ump);
 	starttime = time_second;
 	softdep_process_journal(mp, NULL, full ? MNT_WAIT : 0);
@@ -2134,6 +2134,8 @@ softdep_waitidle(struct mount *mp, int flags __unused)
 	int error, i;
 
 	ump = VFSTOUFS(mp);
+	KASSERT(ump->um_softdep != NULL,
+	    ("softdep_waitidle called on non-softdep filesystem"));
 	devvp = ump->um_devvp;
 	td = curthread;
 	error = 0;
@@ -2171,14 +2173,15 @@ softdep_flushfiles(oldmnt, flags, td)
 	int flags;
 	struct thread *td;
 {
-#ifdef QUOTA
 	struct ufsmount *ump;
+#ifdef QUOTA
 	int i;
 #endif
 	int error, early, depcount, loopcnt, retry_flush_count, retry;
 	int morework;
 
-	KASSERT(MOUNTEDSOFTDEP(oldmnt) != 0,
+	ump = VFSTOUFS(oldmnt);
+	KASSERT(ump->um_softdep != NULL,
 	    ("softdep_flushfiles called on non-softdep filesystem"));
 	loopcnt = 10;
 	retry_flush_count = 3;
@@ -2222,7 +2225,6 @@ retry_flush:
 			MNT_ILOCK(oldmnt);
 			morework = oldmnt->mnt_nvnodelistsize > 0;
 #ifdef QUOTA
-			ump = VFSTOUFS(oldmnt);
 			UFS_LOCK(ump);
 			for (i = 0; i < MAXQUOTAS; i++) {
 				if (ump->um_quotas[i] != NULLVP)
@@ -2783,7 +2785,7 @@ softdep_unmount(mp)
 	    ("softdep_unmount called on non-softdep filesystem"));
 	MNT_ILOCK(mp);
 	mp->mnt_flag &= ~MNT_SOFTDEP;
-	if (MOUNTEDSUJ(mp) == 0) {
+	if ((mp->mnt_flag & MNT_SUJ) == 0) {
 		MNT_IUNLOCK(mp);
 	} else {
 		mp->mnt_flag &= ~MNT_SUJ;
@@ -3706,12 +3708,12 @@ softdep_process_journal(mp, needwk, flags)
 	int off;
 	int devbsize;
 
-	if (MOUNTEDSUJ(mp) == 0)
+	ump = VFSTOUFS(mp);
+	if (ump->um_softdep == NULL || ump->um_softdep->sd_jblocks == NULL)
 		return;
 	shouldflush = softdep_flushcache;
 	bio = NULL;
 	jseg = NULL;
-	ump = VFSTOUFS(mp);
 	LOCK_OWNED(ump);
 	fs = ump->um_fs;
 	jblocks = ump->softdep_jblocks;
@@ -14201,7 +14203,8 @@ check_clear_deps(mp)
 	 * causes deferred work to be done sooner.
 	 */
 	ump = VFSTOUFS(mp);
-	suj_susp = MOUNTEDSUJ(mp) && ump->softdep_jblocks->jb_suspended;
+	suj_susp = ump->um_softdep->sd_jblocks != NULL &&
+	    ump->softdep_jblocks->jb_suspended;
 	if (req_clear_remove || req_clear_inodedeps || suj_susp) {
 		FREE_LOCK(ump);
 		softdep_send_speedup(ump, 0, BIO_SPEEDUP_TRIM | BIO_SPEEDUP_WRITE);
diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c
index f2bfe13cf89b..273d6e497955 100644
--- a/sys/ufs/ffs/ffs_vfsops.c
+++ b/sys/ufs/ffs/ffs_vfsops.c
@@ -380,6 +380,7 @@ ffs_mount(struct mount *mp)
 	accmode_t accmode;
 	struct nameidata ndp;
 	char *fspec;
+	bool mounted_softdep;
 
 	td = curthread;
 	if (vfs_filteropt(mp->mnt_optnew, ffs_opts))
@@ -491,6 +492,16 @@ ffs_mount(struct mount *mp)
 			error = vfs_write_suspend_umnt(mp);
 			if (error != 0)
 				return (error);
+
+			fs->fs_ronly = 1;
+			if (MOUNTEDSOFTDEP(mp)) {
+				MNT_ILOCK(mp);
+				mp->mnt_flag &= ~MNT_SOFTDEP;
+				MNT_IUNLOCK(mp);
+				mounted_softdep = true;
+			} else
+				mounted_softdep = false;
+
 			/*
 			 * Check for and optionally get rid of files open
 			 * for writing.
@@ -498,15 +509,22 @@ ffs_mount(struct mount *mp)
 			flags = WRITECLOSE;
 			if (mp->mnt_flag & MNT_FORCE)
 				flags |= FORCECLOSE;
-			if (MOUNTEDSOFTDEP(mp)) {
+			if (mounted_softdep) {
 				error = softdep_flushfiles(mp, flags, td);
 			} else {
 				error = ffs_flushfiles(mp, flags, td);
 			}
 			if (error) {
+				fs->fs_ronly = 0;
+				if (mounted_softdep) {
+					MNT_ILOCK(mp);
+					mp->mnt_flag |= MNT_SOFTDEP;
+					MNT_IUNLOCK(mp);
+				}
 				vfs_write_resume(mp, 0);
 				return (error);
 			}
+
 			if (fs->fs_pendingblocks != 0 ||
 			    fs->fs_pendinginodes != 0) {
 				printf("WARNING: %s Update error: blocks %jd "
@@ -521,10 +539,15 @@ ffs_mount(struct mount *mp)
 			if ((error = ffs_sbupdate(ump, MNT_WAIT, 0)) != 0) {
 				fs->fs_ronly = 0;
 				fs->fs_clean = 0;
+				if (mounted_softdep) {
+					MNT_ILOCK(mp);
+					mp->mnt_flag |= MNT_SOFTDEP;
+					MNT_IUNLOCK(mp);
+				}
 				vfs_write_resume(mp, 0);
 				return (error);
 			}
-			if (MOUNTEDSOFTDEP(mp))
+			if (mounted_softdep)
 				softdep_unmount(mp);
 			g_topology_lock();
 			/*
@@ -532,7 +555,6 @@ ffs_mount(struct mount *mp)
 			 */
 			g_access(ump->um_cp, 0, -1, -1);
 			g_topology_unlock();
-			fs->fs_ronly = 1;
 			MNT_ILOCK(mp);
 			mp->mnt_flag |= MNT_RDONLY;
 			MNT_IUNLOCK(mp);


More information about the dev-commits-src-main mailing list