From 4085590d39e38cd1cbbf34dcb2efe2ef71895f90 Mon Sep 17 00:00:00 2001 From: Konstantin Belousov Date: Fri, 27 Dec 2019 16:43:34 +0000 Subject: [PATCH] ufs: do not leave non-reclaimed vnodes with zero i_mode around. After a recent change, vput() relocks even the exclusively locked vnode before inactivating it. Before that, UFS could safely instantiate a vnode for cleared inode, then the last vput() after ffs_vgetf() noted that ip->i_mode == 0 and recycled. Now, it is possible for other threads to note the half-constructed vnode, e.g. to insert it into hash, which makes other threads to use it despite mode is zero, before inactivation and reclaim. Handle the found cases in SU code, by explicitly doing reclaim. Assert that other places get fully constructed inode from ffs_vgetf(), which cannot be cleared before dependencies are resolved. Reported and tested by: pho Reviewed by: mckusick Sponsored by: The FreeBSD Foundation MFC after: 1 week --- sys/ufs/ffs/ffs_softdep.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/sys/ufs/ffs/ffs_softdep.c b/sys/ufs/ffs/ffs_softdep.c index 00b3c5eec83..7d8f9041e0d 100644 --- a/sys/ufs/ffs/ffs_softdep.c +++ b/sys/ufs/ffs/ffs_softdep.c @@ -8052,7 +8052,9 @@ handle_complete_freeblocks(freeblks, flags) flags, &vp, FFSV_FORCEINSMQ) != 0) return (EBUSY); ip = VTOI(vp); - if (DIP(ip, i_modrev) == freeblks->fb_modrev) { + if (ip->i_mode == 0) { + vgone(vp); + } else if (DIP(ip, i_modrev) == freeblks->fb_modrev) { DIP_SET(ip, i_blocks, DIP(ip, i_blocks) - spare); ip->i_flag |= IN_CHANGE; /* @@ -9835,6 +9837,7 @@ handle_workitem_remove(dirrem, flags) if (ffs_vgetf(mp, oldinum, flags, &vp, FFSV_FORCEINSMQ) != 0) return (EBUSY); ip = VTOI(vp); + MPASS(ip->i_mode != 0); ACQUIRE_LOCK(ump); if ((inodedep_lookup(mp, oldinum, 0, &inodedep)) == 0) panic("handle_workitem_remove: lost inodedep"); @@ -12530,6 +12533,7 @@ restart: VOP_UNLOCK(vp, 0); error = ffs_vgetf(mp, parentino, LK_EXCLUSIVE, &pvp, FFSV_FORCEINSMQ); + MPASS(VTOI(pvp)->i_mode != 0); vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); if (VN_IS_DOOMED(vp)) { if (error == 0) @@ -13172,6 +13176,7 @@ restart: if ((error = ffs_vgetf(mp, inum, LK_EXCLUSIVE, &vp, FFSV_FORCEINSMQ))) break; + MPASS(VTOI(vp)->i_mode != 0); error = flush_newblk_dep(vp, mp, 0); /* * If we still have the dependency we might need to @@ -13236,6 +13241,7 @@ retry: if ((error = ffs_vgetf(mp, inum, LK_EXCLUSIVE, &vp, FFSV_FORCEINSMQ))) break; + MPASS(VTOI(vp)->i_mode != 0); error = ffs_update(vp, 1); vput(vp); if (error) @@ -13830,6 +13836,7 @@ clear_remove(mp) softdep_error("clear_remove: vget", error); goto finish_write; } + MPASS(VTOI(vp)->i_mode != 0); if ((error = ffs_syncvnode(vp, MNT_NOWAIT, 0))) softdep_error("clear_remove: fsync", error); bo = &vp->v_bufobj; @@ -13911,7 +13918,9 @@ clear_inodedeps(mp) return; } vfs_unbusy(mp); - if (ino == lastino) { + if (VTOI(vp)->i_mode == 0) { + vgone(vp); + } else if (ino == lastino) { if ((error = ffs_syncvnode(vp, MNT_WAIT, 0))) softdep_error("clear_inodedeps: fsync1", error); } else {