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