@@ -23,6 +23,12 @@ _require_scratch_reflink
_scratch_mkfs >> $seqres.full
_scratch_mount
_require_congruent_file_oplen $SCRATCH_MNT 1048576
+
+# Due to multiple security issues and potential for subtle bugs around setgid
+# inheritance the rules in the write and chmod/chown paths have been made
+# consistent and are enforced by the VFS since kernel 6.2.
+_fixed_in_kernel_version "v6.2"
+
chmod a+rw $SCRATCH_MNT/
setup_testfile() {
@@ -102,8 +108,8 @@ setup_testfile
chmod a+rwxs $SCRATCH_MNT/a
commit_and_check
-#Commit to a non-exec file by an unprivileged user leaves sgid.
-echo "Test 9 - qa_user, non-exec file, only sgid"
+# Commit to a non-exec file by an unprivileged user clears sgid.
+echo "Test 9 - qa_user, non-exec file"
setup_testfile
chmod a+rw,g+rws $SCRATCH_MNT/a
commit_and_check "$qa_user"
@@ -115,7 +121,7 @@ chmod a+rw,g+rwxs $SCRATCH_MNT/a
commit_and_check "$qa_user"
#Commit to a user-exec file by an unprivileged user clears sgid
-echo "Test 11 - qa_user, user-exec file, only sgid"
+echo "Test 11 - qa_user, user-exec file"
setup_testfile
chmod a+rw,u+x,g+rws $SCRATCH_MNT/a
commit_and_check "$qa_user"
@@ -47,11 +47,11 @@ Test 8 - root, all-exec file
3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
6777 -rwsrwsrwx SCRATCH_MNT/a
-Test 9 - qa_user, non-exec file, only sgid
+Test 9 - qa_user, non-exec file
310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
2666 -rw-rwSrw- SCRATCH_MNT/a
3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
-2666 -rw-rwSrw- SCRATCH_MNT/a
+666 -rw-rw-rw- SCRATCH_MNT/a
Test 10 - qa_user, group-exec file, only sgid
310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
@@ -59,11 +59,11 @@ Test 10 - qa_user, group-exec file, only sgid
3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
676 -rw-rwxrw- SCRATCH_MNT/a
-Test 11 - qa_user, user-exec file, only sgid
+Test 11 - qa_user, user-exec file
310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
2766 -rwxrwSrw- SCRATCH_MNT/a
3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
-2766 -rwxrwSrw- SCRATCH_MNT/a
+766 -rwxrw-rw- SCRATCH_MNT/a
Test 12 - qa_user, all-exec file, only sgid
310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
@@ -30,6 +30,11 @@ verb=falloc
_require_xfs_io_command $verb
_require_congruent_file_oplen $TEST_DIR 65536
+# Due to multiple security issues and potential for subtle bugs around setgid
+# inheritance the rules in the write and chmod/chown paths have been made
+# consistent and are enforced by the VFS since kernel 6.2.
+_fixed_in_kernel_version "v6.2"
+
junk_dir=$TEST_DIR/$seq
junk_file=$junk_dir/a
mkdir -p $junk_dir/
@@ -110,8 +115,8 @@ setup_testfile
chmod a+rwxs $junk_file
commit_and_check "" "$verb" 64k 64k
-# Commit to a non-exec file by an unprivileged user leaves sgid.
-echo "Test 9 - qa_user, non-exec file $verb, only sgid"
+# Commit to a non-exec file by an unprivileged user clears sgid.
+echo "Test 9 - qa_user, non-exec file $verb"
setup_testfile
chmod a+rw,g+rws $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
@@ -123,7 +128,7 @@ chmod a+rw,g+rwxs $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
# Commit to a user-exec file by an unprivileged user clears sgid
-echo "Test 11 - qa_user, user-exec file $verb, only sgid"
+echo "Test 11 - qa_user, user-exec file $verb"
setup_testfile
chmod a+rw,u+x,g+rws $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
@@ -31,17 +31,17 @@ Test 8 - root, all-exec file falloc
6777 -rwsrwsrwx TEST_DIR/683/a
6777 -rwsrwsrwx TEST_DIR/683/a
-Test 9 - qa_user, non-exec file falloc, only sgid
-2666 -rw-rwSrw- TEST_DIR/683/a
+Test 9 - qa_user, non-exec file falloc
2666 -rw-rwSrw- TEST_DIR/683/a
+666 -rw-rw-rw- TEST_DIR/683/a
Test 10 - qa_user, group-exec file falloc, only sgid
2676 -rw-rwsrw- TEST_DIR/683/a
676 -rw-rwxrw- TEST_DIR/683/a
-Test 11 - qa_user, user-exec file falloc, only sgid
-2766 -rwxrwSrw- TEST_DIR/683/a
+Test 11 - qa_user, user-exec file falloc
2766 -rwxrwSrw- TEST_DIR/683/a
+766 -rwxrw-rw- TEST_DIR/683/a
Test 12 - qa_user, all-exec file falloc, only sgid
2777 -rwxrwsrwx TEST_DIR/683/a
@@ -30,6 +30,11 @@ verb=fpunch
_require_xfs_io_command $verb
_require_congruent_file_oplen $TEST_DIR 65536
+# Due to multiple security issues and potential for subtle bugs around setgid
+# inheritance the rules in the write and chmod/chown paths have been made
+# consistent and are enforced by the VFS since kernel 6.2.
+_fixed_in_kernel_version "v6.2"
+
junk_dir=$TEST_DIR/$seq
junk_file=$junk_dir/a
mkdir -p $junk_dir/
@@ -110,8 +115,8 @@ setup_testfile
chmod a+rwxs $junk_file
commit_and_check "" "$verb" 64k 64k
-# Commit to a non-exec file by an unprivileged user leaves sgid.
-echo "Test 9 - qa_user, non-exec file $verb, only sgid"
+# Commit to a non-exec file by an unprivileged user clears sgid.
+echo "Test 9 - qa_user, non-exec file $verb"
setup_testfile
chmod a+rw,g+rws $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
@@ -123,7 +128,7 @@ chmod a+rw,g+rwxs $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
# Commit to a user-exec file by an unprivileged user clears sgid
-echo "Test 11 - qa_user, user-exec file $verb, only sgid"
+echo "Test 11 - qa_user, user-exec file $verb"
setup_testfile
chmod a+rw,u+x,g+rws $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
@@ -31,17 +31,17 @@ Test 8 - root, all-exec file fpunch
6777 -rwsrwsrwx TEST_DIR/684/a
6777 -rwsrwsrwx TEST_DIR/684/a
-Test 9 - qa_user, non-exec file fpunch, only sgid
-2666 -rw-rwSrw- TEST_DIR/684/a
+Test 9 - qa_user, non-exec file fpunch
2666 -rw-rwSrw- TEST_DIR/684/a
+666 -rw-rw-rw- TEST_DIR/684/a
Test 10 - qa_user, group-exec file fpunch, only sgid
2676 -rw-rwsrw- TEST_DIR/684/a
676 -rw-rwxrw- TEST_DIR/684/a
-Test 11 - qa_user, user-exec file fpunch, only sgid
-2766 -rwxrwSrw- TEST_DIR/684/a
+Test 11 - qa_user, user-exec file fpunch
2766 -rwxrwSrw- TEST_DIR/684/a
+766 -rwxrw-rw- TEST_DIR/684/a
Test 12 - qa_user, all-exec file fpunch, only sgid
2777 -rwxrwsrwx TEST_DIR/684/a
@@ -30,6 +30,11 @@ verb=fzero
_require_xfs_io_command $verb
_require_congruent_file_oplen $TEST_DIR 65536
+# Due to multiple security issues and potential for subtle bugs around setgid
+# inheritance the rules in the write and chmod/chown paths have been made
+# consistent and are enforced by the VFS since kernel 6.2.
+_fixed_in_kernel_version "v6.2"
+
junk_dir=$TEST_DIR/$seq
junk_file=$junk_dir/a
mkdir -p $junk_dir/
@@ -110,8 +115,8 @@ setup_testfile
chmod a+rwxs $junk_file
commit_and_check "" "$verb" 64k 64k
-# Commit to a non-exec file by an unprivileged user leaves sgid.
-echo "Test 9 - qa_user, non-exec file $verb, only sgid"
+# Commit to a non-exec file by an unprivileged user clears sgid.
+echo "Test 9 - qa_user, non-exec file $verb"
setup_testfile
chmod a+rw,g+rws $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
@@ -123,7 +128,7 @@ chmod a+rw,g+rwxs $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
# Commit to a user-exec file by an unprivileged user clears sgid
-echo "Test 11 - qa_user, user-exec file $verb, only sgid"
+echo "Test 11 - qa_user, user-exec file $verb"
setup_testfile
chmod a+rw,u+x,g+rws $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
@@ -31,17 +31,17 @@ Test 8 - root, all-exec file fzero
6777 -rwsrwsrwx TEST_DIR/685/a
6777 -rwsrwsrwx TEST_DIR/685/a
-Test 9 - qa_user, non-exec file fzero, only sgid
-2666 -rw-rwSrw- TEST_DIR/685/a
+Test 9 - qa_user, non-exec file fzero
2666 -rw-rwSrw- TEST_DIR/685/a
+666 -rw-rw-rw- TEST_DIR/685/a
Test 10 - qa_user, group-exec file fzero, only sgid
2676 -rw-rwsrw- TEST_DIR/685/a
676 -rw-rwxrw- TEST_DIR/685/a
-Test 11 - qa_user, user-exec file fzero, only sgid
-2766 -rwxrwSrw- TEST_DIR/685/a
+Test 11 - qa_user, user-exec file fzero
2766 -rwxrwSrw- TEST_DIR/685/a
+766 -rwxrw-rw- TEST_DIR/685/a
Test 12 - qa_user, all-exec file fzero, only sgid
2777 -rwxrwsrwx TEST_DIR/685/a
@@ -30,6 +30,11 @@ verb=finsert
_require_xfs_io_command $verb
_require_congruent_file_oplen $TEST_DIR 65536
+# Due to multiple security issues and potential for subtle bugs around setgid
+# inheritance the rules in the write and chmod/chown paths have been made
+# consistent and are enforced by the VFS since kernel 6.2.
+_fixed_in_kernel_version "v6.2"
+
junk_dir=$TEST_DIR/$seq
junk_file=$junk_dir/a
mkdir -p $junk_dir/
@@ -110,8 +115,8 @@ setup_testfile
chmod a+rwxs $junk_file
commit_and_check "" "$verb" 64k 64k
-# Commit to a non-exec file by an unprivileged user leaves sgid.
-echo "Test 9 - qa_user, non-exec file $verb, only sgid"
+# Commit to a non-exec file by an unprivileged user clears sgid.
+echo "Test 9 - qa_user, non-exec file $verb"
setup_testfile
chmod a+rw,g+rws $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
@@ -123,7 +128,7 @@ chmod a+rw,g+rwxs $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
# Commit to a user-exec file by an unprivileged user clears sgid
-echo "Test 11 - qa_user, user-exec file $verb, only sgid"
+echo "Test 11 - qa_user, user-exec file $verb"
setup_testfile
chmod a+rw,u+x,g+rws $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
@@ -31,17 +31,17 @@ Test 8 - root, all-exec file finsert
6777 -rwsrwsrwx TEST_DIR/686/a
6777 -rwsrwsrwx TEST_DIR/686/a
-Test 9 - qa_user, non-exec file finsert, only sgid
-2666 -rw-rwSrw- TEST_DIR/686/a
+Test 9 - qa_user, non-exec file finsert
2666 -rw-rwSrw- TEST_DIR/686/a
+666 -rw-rw-rw- TEST_DIR/686/a
Test 10 - qa_user, group-exec file finsert, only sgid
2676 -rw-rwsrw- TEST_DIR/686/a
676 -rw-rwxrw- TEST_DIR/686/a
-Test 11 - qa_user, user-exec file finsert, only sgid
-2766 -rwxrwSrw- TEST_DIR/686/a
+Test 11 - qa_user, user-exec file finsert
2766 -rwxrwSrw- TEST_DIR/686/a
+766 -rwxrw-rw- TEST_DIR/686/a
Test 12 - qa_user, all-exec file finsert, only sgid
2777 -rwxrwsrwx TEST_DIR/686/a
@@ -110,8 +110,8 @@ setup_testfile
chmod a+rwxs $junk_file
commit_and_check "" "$verb" 64k 64k
-# Commit to a non-exec file by an unprivileged user leaves sgid.
-echo "Test 9 - qa_user, non-exec file $verb, only sgid"
+# Commit to a non-exec file by an unprivileged user clears sgid.
+echo "Test 9 - qa_user, non-exec file $verb"
setup_testfile
chmod a+rw,g+rws $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
@@ -123,7 +123,7 @@ chmod a+rw,g+rwxs $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
# Commit to a user-exec file by an unprivileged user clears sgid
-echo "Test 11 - qa_user, user-exec file $verb, only sgid"
+echo "Test 11 - qa_user, user-exec file $verb"
setup_testfile
chmod a+rw,u+x,g+rws $junk_file
commit_and_check "$qa_user" "$verb" 64k 64k
@@ -31,17 +31,17 @@ Test 8 - root, all-exec file fcollapse
6777 -rwsrwsrwx TEST_DIR/687/a
6777 -rwsrwsrwx TEST_DIR/687/a
-Test 9 - qa_user, non-exec file fcollapse, only sgid
-2666 -rw-rwSrw- TEST_DIR/687/a
+Test 9 - qa_user, non-exec file fcollapse
2666 -rw-rwSrw- TEST_DIR/687/a
+666 -rw-rw-rw- TEST_DIR/687/a
Test 10 - qa_user, group-exec file fcollapse, only sgid
2676 -rw-rwsrw- TEST_DIR/687/a
676 -rw-rwxrw- TEST_DIR/687/a
-Test 11 - qa_user, user-exec file fcollapse, only sgid
-2766 -rwxrwSrw- TEST_DIR/687/a
+Test 11 - qa_user, user-exec file fcollapse
2766 -rwxrwSrw- TEST_DIR/687/a
+766 -rwxrw-rw- TEST_DIR/687/a
Test 12 - qa_user, all-exec file fcollapse, only sgid
2777 -rwxrwsrwx TEST_DIR/687/a
Over mutiple kernel releases we have reworked setgid inheritance significantly due to long-standing security issues, security issues that were reintroduced after they were fixed, and the subtle and difficult inheritance rules that plagued individual filesystems. We have lifted setgid inheritance into the VFS proper in earlier patches. Starting with kernel v6.2 we have made setgid inheritance consistent between the write and setattr (ch{mod,own}) paths. The gist of the requirement is that in order to inherit the setgid bit the user needs to be in the group of the file or have CAP_FSETID in their user namespace. Otherwise the setgid bit will be dropped irregardless of the file's executability. Change the tests accordingly and annotate them with the commits that changed the behavior. Note, that only with v6.2 setgid inheritance works correctly for overlayfs in the write path. Before this the setgid bit was always retained. Link: https://lore.kernel.org/linux-ext4/CAOQ4uxhmCgyorYVtD6=n=khqwUc=MPbZs+y=sqt09XbGoNm_tA@mail.gmail.com Link: https://lore.kernel.org/linux-fsdevel/20221212112053.99208-1-brauner@kernel.org Link: https://lore.kernel.org/linux-fsdevel/20221122142010.zchf2jz2oymx55qi@wittgenstein Cc: Amir Goldstein <amir73il@gmail.com> Cc: Zorro Lang <zlang@redhat.com> Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org> --- Changes in v2: - Darrick J. Wong <djwong@kernel.org>: - Also update generic/686 and generic/687. - Link to v1: https://lore.kernel.org/r/20230103-fstests-setgid-v6-2-v1-1-b8972c303ebe@kernel.org --- tests/generic/673 | 12 +++++++++--- tests/generic/673.out | 8 ++++---- tests/generic/683 | 11 ++++++++--- tests/generic/683.out | 8 ++++---- tests/generic/684 | 11 ++++++++--- tests/generic/684.out | 8 ++++---- tests/generic/685 | 11 ++++++++--- tests/generic/685.out | 8 ++++---- tests/generic/686 | 11 ++++++++--- tests/generic/686.out | 8 ++++---- tests/generic/687 | 6 +++--- tests/generic/687.out | 8 ++++---- 12 files changed, 68 insertions(+), 42 deletions(-) --- base-commit: fbd489798b31e32f0eaefcd754326a06aa5b166f change-id: 20230103-fstests-setgid-v6-2-4ce5852d11e2