From owner-dev-commits-src-main@freebsd.org Fri Mar 12 11:32:23 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id B1A16575C9D; Fri, 12 Mar 2021 11:32:23 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DxkF262dxz3QtF; Fri, 12 Mar 2021 11:32:21 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4362117033; Fri, 12 Mar 2021 11:32:21 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 12CBWLBa077386; Fri, 12 Mar 2021 11:32:21 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 12CBWLmm077385; Fri, 12 Mar 2021 11:32:21 GMT (envelope-from git) Date: Fri, 12 Mar 2021 11:32:21 GMT Message-Id: <202103121132.12CBWLmm077385@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Konstantin Belousov Subject: git: 7c7a6681fab2 - main - ffs: clear MNT_SOFTDEP earlier when remounting rw to ro MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kib X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 7c7a6681fab2c0453085d30424f479c0f766904d Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Mar 2021 11:32:26 -0000 The branch main has been updated by kib: URL: https://cgit.FreeBSD.org/src/commit/?id=7c7a6681fab2c0453085d30424f479c0f766904d commit 7c7a6681fab2c0453085d30424f479c0f766904d Author: Konstantin Belousov AuthorDate: 2021-02-28 18:55:35 +0000 Commit: Konstantin Belousov 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);