Module Name:    src
Committed By:   hannken
Date:           Mon Dec 30 09:03:07 UTC 2024

Modified Files:
        src/sys/ufs/ffs: ffs_vfsops.c ffs_wapbl.c

Log Message:
Protect test/clear fs->fs_fmod with um_lock like it is already
protected in ffs_alloc.c.

When writing to disk protect moving superblock to buffer with um_lock.

Set/clear fs->fmod while mounting, updating a mount or unmounting
is safe as these operations run exclusive, either mounting creates
a new file system or the file system is suspended.  Assert suspension
for update and unmount.

PR kern/58837 "ffs: Missing locking around fs_fmod/time"


To generate a diff of this commit:
cvs rdiff -u -r1.383 -r1.384 src/sys/ufs/ffs/ffs_vfsops.c
cvs rdiff -u -r1.49 -r1.50 src/sys/ufs/ffs/ffs_wapbl.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/ufs/ffs/ffs_vfsops.c
diff -u src/sys/ufs/ffs/ffs_vfsops.c:1.383 src/sys/ufs/ffs/ffs_vfsops.c:1.384
--- src/sys/ufs/ffs/ffs_vfsops.c:1.383	Mon Dec 30 09:01:35 2024
+++ src/sys/ufs/ffs/ffs_vfsops.c	Mon Dec 30 09:03:07 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: ffs_vfsops.c,v 1.383 2024/12/30 09:01:35 hannken Exp $	*/
+/*	$NetBSD: ffs_vfsops.c,v 1.384 2024/12/30 09:03:07 hannken Exp $	*/
 
 /*-
  * Copyright (c) 2008, 2009 The NetBSD Foundation, Inc.
@@ -61,7 +61,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ffs_vfsops.c,v 1.383 2024/12/30 09:01:35 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ffs_vfsops.c,v 1.384 2024/12/30 09:03:07 hannken Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_ffs.h"
@@ -618,8 +618,9 @@ ffs_mount(struct mount *mp, const char *
 		fs = ump->um_fs;
 	} else {
 		/*
-		 * Update the mount.
+		 * Update the mount.  The file system is suspended.
 		 */
+		KASSERT(fstrans_is_owner(mp));
 
 		/*
 		 * The initial mount got a reference on this
@@ -767,19 +768,20 @@ ffs_mount(struct mount *mp, const char *
 		    fs->fs_clean);
 	}
 
-	if (fs->fs_fmod != 0) {
-		int err;
+	if (UFS_WAPBL_BEGIN(mp) == 0) {
+		mutex_enter(&ump->um_lock);
+		if (fs->fs_fmod != 0) {
+			KASSERT(!fs->fs_ronly);
 
-		KASSERT(!fs->fs_ronly);
-
-		if (fs->fs_clean & FS_WASCLEAN)
-			fs->fs_time = time_second;
-		fs->fs_fmod = 0;
-		err = UFS_WAPBL_BEGIN(mp);
-		if (err == 0) {
+			if (fs->fs_clean & FS_WASCLEAN)
+				fs->fs_time = time_second;
+			fs->fs_fmod = 0;
+			mutex_exit(&ump->um_lock);
 			(void) ffs_cgupdate(ump, MNT_WAIT);
-			UFS_WAPBL_END(mp);
+		} else {
+			mutex_exit(&ump->um_lock);
 		}
+		UFS_WAPBL_END(mp);
 	}
 	if ((mp->mnt_flag & MNT_SOFTDEP) != 0) {
 		printf("%s: `-o softdep' is no longer supported, "
@@ -1768,6 +1770,9 @@ ffs_unmount(struct mount *mp, int mntfla
 	extern int doforce;
 #endif
 
+	/* The file system is suspended. */
+	KASSERT(fstrans_is_owner(mp));
+
 	if (ump->um_discarddata) {
 		ffs_discard_finish(ump->um_discarddata, mntflags);
 		ump->um_discarddata = NULL;
@@ -1778,17 +1783,17 @@ ffs_unmount(struct mount *mp, int mntfla
 		flags |= FORCECLOSE;
 	if ((error = ffs_flushfiles(mp, flags, l)) != 0)
 		return (error);
-	error = UFS_WAPBL_BEGIN(mp);
-	if (error == 0)
-		if (fs->fs_ronly == 0 &&
-		    ffs_cgupdate(ump, MNT_WAIT) == 0 &&
+	if (fs->fs_ronly == 0 && UFS_WAPBL_BEGIN(mp) == 0) {
+		if (ffs_cgupdate(ump, MNT_WAIT) == 0 &&
 		    fs->fs_clean & FS_WASCLEAN) {
+			mutex_enter(&ump->um_lock);
 			fs->fs_clean = FS_ISCLEAN;
 			fs->fs_fmod = 0;
+			mutex_exit(&ump->um_lock);
 			(void) ffs_sbupdate(ump, MNT_WAIT);
 		}
-	if (error == 0)
 		UFS_WAPBL_END(mp);
+	}
 #ifdef WAPBL
 	KASSERT(!(mp->mnt_wapbl_replay && mp->mnt_wapbl));
 	if (mp->mnt_wapbl_replay) {
@@ -2045,17 +2050,21 @@ ffs_sync(struct mount *mp, int waitfor, 
 	/*
 	 * Write back modified superblock.
 	 */
-	if (fs->fs_fmod != 0) {
-		fs->fs_fmod = 0;
-		fs->fs_time = time_second;
-		error = UFS_WAPBL_BEGIN(mp);
-		if (error)
-			allerror = error;
-		else {
+	error = UFS_WAPBL_BEGIN(mp);
+	if (error) {
+		allerror = error;
+	} else {
+		mutex_enter(&ump->um_lock);
+		if (fs->fs_fmod != 0) {
+			fs->fs_fmod = 0;
+			fs->fs_time = time_second;
+			mutex_exit(&ump->um_lock);
 			if ((error = ffs_cgupdate(ump, waitfor)))
 				allerror = error;
-			UFS_WAPBL_END(mp);
+		} else {
+			mutex_exit(&ump->um_lock);
 		}
+		UFS_WAPBL_END(mp);
 	}
 
 #ifdef WAPBL
@@ -2402,31 +2411,32 @@ int
 ffs_sbupdate(struct ufsmount *mp, int waitfor)
 {
 	struct fs *fs = mp->um_fs;
+	struct fs *bfs;
 	struct buf *bp;
 	int error;
-	u_int32_t saveflag;
 
 	error = ffs_getblk(mp->um_devvp,
 	    fs->fs_sblockloc / DEV_BSIZE, FFS_NOBLK,
 	    fs->fs_sbsize, false, &bp);
 	if (error)
 		return error;
-	saveflag = fs->fs_flags & FS_INTERNAL;
-	fs->fs_flags &= ~FS_INTERNAL;
 
+	mutex_enter(&mp->um_lock);
 	memcpy(bp->b_data, fs, fs->fs_sbsize);
+	mutex_exit(&mp->um_lock);
+
+	bfs = (struct fs *)bp->b_data;
 
+	bfs->fs_flags &= ~FS_INTERNAL;
 	ffs_oldfscompat_write((struct fs *)bp->b_data, mp);
 	if (mp->um_flags & UFS_EA) {
-		struct fs *bfs = (struct fs *)bp->b_data;
 		KASSERT(bfs->fs_magic == FS_UFS2_MAGIC);
 		bfs->fs_magic = FS_UFS2EA_MAGIC;
 	}
 #ifdef FFS_EI
 	if (mp->um_flags & UFS_NEEDSWAP)
-		ffs_sb_swap((struct fs *)bp->b_data, (struct fs *)bp->b_data);
+		ffs_sb_swap(bfs, bfs);
 #endif
-	fs->fs_flags |= saveflag;
 
 	if (waitfor == MNT_WAIT)
 		error = bwrite(bp);

Index: src/sys/ufs/ffs/ffs_wapbl.c
diff -u src/sys/ufs/ffs/ffs_wapbl.c:1.49 src/sys/ufs/ffs/ffs_wapbl.c:1.50
--- src/sys/ufs/ffs/ffs_wapbl.c:1.49	Mon May 13 00:24:19 2024
+++ src/sys/ufs/ffs/ffs_wapbl.c	Mon Dec 30 09:03:07 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: ffs_wapbl.c,v 1.49 2024/05/13 00:24:19 msaitoh Exp $	*/
+/*	$NetBSD: ffs_wapbl.c,v 1.50 2024/12/30 09:03:07 hannken Exp $	*/
 
 /*-
  * Copyright (c) 2003,2006,2008 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ffs_wapbl.c,v 1.49 2024/05/13 00:24:19 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ffs_wapbl.c,v 1.50 2024/12/30 09:03:07 hannken Exp $");
 
 #define WAPBL_INTERNAL
 
@@ -188,11 +188,15 @@ ffs_wapbl_sync_metadata(struct mount *mp
 		    FFS_DBTOFSB(fs, wd->wd_blkno), wd->wd_len, -1);
 	}
 
+	mutex_enter(&ump->um_lock);
 	if (fs->fs_fmod != 0) {
 		fs->fs_fmod = 0;
 		fs->fs_time = time_second;
+		mutex_exit(&ump->um_lock);
 		error = ffs_cgupdate(ump, 0);
 		KASSERT(error == 0);
+	} else {
+		mutex_exit(&ump->um_lock);
 	}
 }
 

Reply via email to