Date: Wed, 8 May 2019 22:28:14 +0000 (UTC) From: Alan Somers <asomers@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r347372 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs Message-ID: <201905082228.x48MSENh031533@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: asomers Date: Wed May 8 22:28:13 2019 New Revision: 347372 URL: https://svnweb.freebsd.org/changeset/base/347372 Log: fusefs: fix a permission handling bug during VOP_RENAME If the file to be renamed is a directory and it's going to get a new parent, then the user must have write permissions to that directory, because the ".." dirent must be changed. Reported by: pjdfstest Sponsored by: The FreeBSD Foundation Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_vnops.c Wed May 8 21:26:11 2019 (r347371) +++ projects/fuse2/sys/fs/fuse/fuse_vnops.c Wed May 8 22:28:13 2019 (r347372) @@ -1423,7 +1423,8 @@ fuse_vnop_rename(struct vop_rename_args *ap) struct vnode *tvp = ap->a_tvp; struct componentname *tcnp = ap->a_tcnp; struct fuse_data *data; - + bool newparent = fdvp != tdvp; + bool isdir = fvp->v_type == VDIR; int err = 0; if (fuse_isdeadfs(fdvp)) { @@ -1442,7 +1443,17 @@ fuse_vnop_rename(struct vop_rename_args *ap) * under the source directory in the file system tree. * Linux performs this check at VFS level. */ + /* + * If source is a directory, and it will get a new parent, user must + * have write permission to it, so ".." can be modified. + */ data = fuse_get_mpdata(vnode_mount(tdvp)); + if (data->dataflags & FSESS_DEFAULT_PERMISSIONS && isdir && newparent) { + err = fuse_internal_access(fvp, VWRITE, + tcnp->cn_thread, tcnp->cn_cred); + if (err) + goto out; + } sx_xlock(&data->rename_lock); err = fuse_internal_rename(fdvp, fcnp, tdvp, tcnp); if (err == 0) { Modified: projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc Wed May 8 21:26:11 2019 (r347371) +++ projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc Wed May 8 22:28:13 2019 (r347372) @@ -833,6 +833,49 @@ TEST_F(Rename, eperm_on_sticky_srcdir) ASSERT_EQ(EPERM, errno); } +/* + * A user cannot move out a subdirectory that he does not own, because that + * would require changing the subdirectory's ".." dirent + */ +TEST_F(Rename, eperm_for_subdirectory) +{ + const char FULLDST[] = "mountpoint/d/dst"; + const char FULLSRC[] = "mountpoint/src"; + const char RELDSTDIR[] = "d"; + const char RELDST[] = "dst"; + const char RELSRC[] = "src"; + uint64_t ino = 42; + uint64_t dstdir_ino = 43; + + expect_getattr(1, S_IFDIR | 0777, UINT64_MAX, 1, 0); + expect_lookup(RELSRC, ino, S_IFDIR | 0755, UINT64_MAX, 0); + expect_lookup(RELDSTDIR, dstdir_ino, S_IFDIR | 0777, UINT64_MAX, 0); + EXPECT_LOOKUP(dstdir_ino, RELDST).WillOnce(Invoke(ReturnErrno(ENOENT))); + + ASSERT_EQ(-1, rename(FULLSRC, FULLDST)); + ASSERT_EQ(EACCES, errno); +} + +/* + * A user _can_ rename a subdirectory to which he lacks write permissions, if + * it will keep the same parent + */ +TEST_F(Rename, subdirectory_to_same_dir) +{ + const char FULLDST[] = "mountpoint/dst"; + const char FULLSRC[] = "mountpoint/src"; + const char RELDST[] = "dst"; + const char RELSRC[] = "src"; + uint64_t ino = 42; + + expect_getattr(1, S_IFDIR | 0777, UINT64_MAX, 1, 0); + expect_lookup(RELSRC, ino, S_IFDIR | 0755, UINT64_MAX, 0); + EXPECT_LOOKUP(1, RELDST).WillOnce(Invoke(ReturnErrno(ENOENT))); + expect_rename(0); + + ASSERT_EQ(0, rename(FULLSRC, FULLDST)) << strerror(errno); +} + TEST_F(Rename, eperm_on_sticky_dstdir) { const char FULLDST[] = "mountpoint/d/dst";
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201905082228.x48MSENh031533>