The branch main has been updated by kib: URL: https://cgit.FreeBSD.org/src/commit/?id=8df4bc48c89a1302078282f22139a8368dc06971
commit 8df4bc48c89a1302078282f22139a8368dc06971 Author: Konstantin Belousov <[email protected]> AuthorDate: 2021-08-06 01:03:19 +0000 Commit: Konstantin Belousov <[email protected]> CommitDate: 2021-08-13 14:52:26 +0000 ufs rename: ensure that the result of ufs_checkpath() is stable ufs_rename() calls ufs_checkpath() to ensure that the target directory is not a child of the source. If not, rename would create a loop. For instance: source->X1->X2->target and if source moved under target, we get corrupted filesystem. Suppose that we initially have source->X1 .... and X2->target where X1 is not on path from root to X2. Then ufs_checkpath() accepts the inodes, but there is nothing preventing parallel rename of X2 to become under X1, after checkpath finished. Ensure stability of ufs_checkpath() result by taking a per-mount sx in ufs_rename right before ufs_checkpath() and till the end. Reviewed by: chs, mckusick Tested by: pho Sponsored by: The FreeBSD Foundation MFC after: 2 weeks --- sys/ufs/ffs/ffs_vfsops.c | 5 ++++- sys/ufs/ufs/ufs_lookup.c | 2 ++ sys/ufs/ufs/ufs_vnops.c | 12 ++++++++++-- sys/ufs/ufs/ufsmount.h | 3 +++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c index 2ff71cc3e4d1..aad7b4f2decd 100644 --- a/sys/ufs/ffs/ffs_vfsops.c +++ b/sys/ufs/ffs/ffs_vfsops.c @@ -1151,6 +1151,7 @@ ffs_mountfs(odevvp, mp, td) else ump->um_check_blkno = NULL; mtx_init(UFS_MTX(ump), "FFS", "FFS Lock", MTX_DEF); + sx_init(&ump->um_checkpath_lock, "uchpth"); ffs_oldfscompat_read(fs, ump, fs->fs_sblockloc); fs->fs_ronly = ronly; fs->fs_active = NULL; @@ -1318,8 +1319,9 @@ out: g_vfs_close(cp); g_topology_unlock(); } - if (ump) { + if (ump != NULL) { mtx_destroy(UFS_MTX(ump)); + sx_destroy(&ump->um_checkpath_lock); if (mp->mnt_gjprovider != NULL) { free(mp->mnt_gjprovider, M_UFSMNT); mp->mnt_gjprovider = NULL; @@ -1545,6 +1547,7 @@ ffs_unmount(mp, mntflags) vrele(ump->um_odevvp); dev_rel(ump->um_dev); mtx_destroy(UFS_MTX(ump)); + sx_destroy(&ump->um_checkpath_lock); if (mp->mnt_gjprovider != NULL) { free(mp->mnt_gjprovider, M_UFSMNT); mp->mnt_gjprovider = NULL; diff --git a/sys/ufs/ufs/ufs_lookup.c b/sys/ufs/ufs/ufs_lookup.c index ac3a8ee641a0..fc78c017e2c6 100644 --- a/sys/ufs/ufs/ufs_lookup.c +++ b/sys/ufs/ufs/ufs_lookup.c @@ -1450,6 +1450,8 @@ ufs_checkpath(ino_t source_ino, ino_t parent_ino, struct inode *target, vp = tvp = ITOV(target); mp = vp->v_mount; *wait_ino = 0; + sx_assert(&VFSTOUFS(mp)->um_checkpath_lock, SA_XLOCKED); + if (target->i_number == source_ino) return (EEXIST); if (target->i_number == parent_ino) diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c index 2dfc2e24f772..00ec8f41f432 100644 --- a/sys/ufs/ufs/ufs_vnops.c +++ b/sys/ufs/ufs/ufs_vnops.c @@ -1249,9 +1249,9 @@ ufs_rename(ap) struct mount *mp; ino_t ino; seqc_t fdvp_s, fvp_s, tdvp_s, tvp_s; - bool want_seqc_end; + bool checkpath_locked, want_seqc_end; - want_seqc_end = false; + checkpath_locked = want_seqc_end = false; #ifdef INVARIANTS if ((tcnp->cn_flags & HASBUF) == 0 || @@ -1453,6 +1453,9 @@ relock: error = VOP_ACCESS(fvp, VWRITE, tcnp->cn_cred, tcnp->cn_thread); if (error) goto unlockout; + + sx_xlock(&VFSTOUFS(mp)->um_checkpath_lock); + checkpath_locked = true; error = ufs_checkpath(ino, fdp->i_number, tdp, tcnp->cn_cred, &ino); /* @@ -1460,6 +1463,8 @@ relock: * everything else and VGET before restarting. */ if (ino) { + sx_xunlock(&VFSTOUFS(mp)->um_checkpath_lock); + checkpath_locked = false; VOP_UNLOCK(fdvp); VOP_UNLOCK(fvp); VOP_UNLOCK(tdvp); @@ -1690,6 +1695,9 @@ unlockout: vn_seqc_write_end(fdvp); } + if (checkpath_locked) + sx_xunlock(&VFSTOUFS(mp)->um_checkpath_lock); + vput(fdvp); vput(fvp); diff --git a/sys/ufs/ufs/ufsmount.h b/sys/ufs/ufs/ufsmount.h index f2951d74d44c..da9a22127125 100644 --- a/sys/ufs/ufs/ufsmount.h +++ b/sys/ufs/ufs/ufsmount.h @@ -70,6 +70,7 @@ LIST_HEAD(trimlist_hashhead, ffs_blkfree_trim_params); #include <sys/_lock.h> #include <sys/_mutex.h> +#include <sys/_sx.h> /* * This structure describes the UFS specific mount structure data. @@ -99,6 +100,8 @@ struct ufsmount { uint64_t um_maxsymlinklen; /* (c) max size of short symlink */ struct mtx um_lock; /* (c) Protects ufsmount & fs */ + struct sx um_checkpath_lock; /* (c) Protects ufs_checkpath() + result */ pid_t um_fsckpid; /* (u) PID can do fsck sysctl */ struct mount_softdeps *um_softdep; /* (c) softdep mgmt structure */ struct vnode *um_quotas[MAXQUOTAS]; /* (q) pointer to quota files */ _______________________________________________ [email protected] mailing list https://lists.freebsd.org/mailman/listinfo/dev-commits-src-all To unsubscribe, send any mail to "[email protected]"
