Fix rename when renamed directory not owned by user, but when user owns the sticky parent directory.
Details
- Reviewers
kib - Commits
- rGcbac1f346495: Fix pjfstest issue tests/rename/09.t
tests/rename/09.t - should pass without errors
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
This change triggered me because explicit cr_uid checks mean that e.g. a future implementation of ACLs would be broken there.
VOP_ACCESS()/VOP_ACCESSX() should encapsulate such knowledge of permissions.
Also, isn't remove from sticky directory has similar problem?
[For UFS, the implementation denies improper renames and deletes at the lookup stage. The logic is in ufs_delete_denied(). I think tmpfs should copy this approach]
The similar test for removing will pass (tests/rmdir/11.t).
Let's consider the test explanation from pjdfstest comment [https://github.com/pjd/pjdfstest/blob/master/tests/rename/09.t#L121]:
# User owns the source sticky directory, but doesn't own the source file. # This fails when changing parent directory, because this will modify # source directory inode (the .. link in it), but we can still rename it # without changing its parent directory.
So, it mean, we cannot modify renamed inode, but can remove it.
Let's add some graphics:
sticky directory child directory owned by user (ino 100500) not owned by user (ino 100501) | | \|/ \|/ directory entries: directory entries: - "." (ino 100500) - "." (ino 100501) - ".." (ino XXXXXX) <----- - ".." (ino 100500) <- cannot be modified ... ... - "child" (ino 100501) -----> ...
The error come when we will change child directory parent entry, mean entry with "cannot be modified" tag,
but we can rename and remove child directory without errors, because only parent entry will be modified
and child directory entries will not be touched.
[For UFS, the implementation denies improper renames and deletes at the lookup stage. The logic is in ufs_delete_denied(). I think tmpfs should copy this approach]
The equal code to ufs_delete_denied() is placed in tmpfs_vnops.c:194
The only thing is needed, add the check to tmpfs_rename(), that the user have write permissions to child directory, like it is done in ufs_vnops.c:1485