* [PATCHSET 0/2] fstests: new stuff for kernel 5.17
@ 2022-01-26 2:11 Darrick J. Wong
2022-01-26 2:11 ` [PATCH 1/2] generic: test suid/sgid behavior with reflink and dedupe Darrick J. Wong
2022-01-26 2:11 ` [PATCH 2/2] fstests: skip tests that require XFS_IOC_ALLOCSP Darrick J. Wong
0 siblings, 2 replies; 11+ messages in thread
From: Darrick J. Wong @ 2022-01-26 2:11 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: linux-xfs, fstests, guan
Hi all,
Add a new test to make sure that reflink actually clears the
setuid/setgid bits /and/ file capabilities correctly, and then port
fstests off the old ALLOCSP and FREESP ioctls since we've removed them
in 5.17.
If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.
This is an extraordinary way to destroy everything. Enjoy!
Comments and questions are, as always, welcome.
--D
kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=xfs-merge-5.17
fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=xfs-merge-5.17
---
common/rc | 4 +-
ltp/fsstress.c | 4 ++
tests/generic/950 | 108 +++++++++++++++++++++++++++++++++++++++++++++
tests/generic/950.out | 49 ++++++++++++++++++++
tests/generic/951 | 118 +++++++++++++++++++++++++++++++++++++++++++++++++
tests/generic/951.out | 49 ++++++++++++++++++++
tests/generic/952 | 71 +++++++++++++++++++++++++++++
tests/generic/952.out | 13 +++++
tests/xfs/107 | 1
9 files changed, 415 insertions(+), 2 deletions(-)
create mode 100755 tests/generic/950
create mode 100644 tests/generic/950.out
create mode 100755 tests/generic/951
create mode 100644 tests/generic/951.out
create mode 100755 tests/generic/952
create mode 100644 tests/generic/952.out
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] generic: test suid/sgid behavior with reflink and dedupe
2022-01-26 2:11 [PATCHSET 0/2] fstests: new stuff for kernel 5.17 Darrick J. Wong
@ 2022-01-26 2:11 ` Darrick J. Wong
2022-01-26 5:56 ` Zorro Lang
2022-01-27 1:27 ` [PATCH v1.1 " Darrick J. Wong
2022-01-26 2:11 ` [PATCH 2/2] fstests: skip tests that require XFS_IOC_ALLOCSP Darrick J. Wong
1 sibling, 2 replies; 11+ messages in thread
From: Darrick J. Wong @ 2022-01-26 2:11 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: linux-xfs, fstests, guan
From: Darrick J. Wong <djwong@kernel.org>
Make sure that we drop the setuid and setgid bits any time reflink or
dedupe change the file contents.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
tests/generic/950 | 108 +++++++++++++++++++++++++++++++++++++++++++++
tests/generic/950.out | 49 ++++++++++++++++++++
tests/generic/951 | 118 +++++++++++++++++++++++++++++++++++++++++++++++++
tests/generic/951.out | 49 ++++++++++++++++++++
tests/generic/952 | 71 +++++++++++++++++++++++++++++
tests/generic/952.out | 13 +++++
6 files changed, 408 insertions(+)
create mode 100755 tests/generic/950
create mode 100644 tests/generic/950.out
create mode 100755 tests/generic/951
create mode 100644 tests/generic/951.out
create mode 100755 tests/generic/952
create mode 100644 tests/generic/952.out
diff --git a/tests/generic/950 b/tests/generic/950
new file mode 100755
index 00000000..84f1d1f0
--- /dev/null
+++ b/tests/generic/950
@@ -0,0 +1,108 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 Oracle. All Rights Reserved.
+#
+# FS QA Test 950
+#
+# Functional test for dropping suid and sgid bits as part of a reflink.
+#
+. ./common/preamble
+_begin_fstest auto clone quick
+
+# Import common functions.
+. ./common/filter
+. ./common/reflink
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_require_user
+_require_scratch_reflink
+
+_scratch_mkfs >> $seqres.full
+_scratch_mount
+_require_congruent_file_oplen $SCRATCH_MNT 1048576
+chmod a+rw $SCRATCH_MNT/
+
+setup_testfile() {
+ rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
+ _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
+ _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
+ chmod a+r $SCRATCH_MNT/b
+ sync
+}
+
+commit_and_check() {
+ local user="$1"
+
+ md5sum $SCRATCH_MNT/a | _filter_scratch
+ stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
+
+ local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
+ if [ -n "$user" ]; then
+ su - "$user" -c "$cmd" >> $seqres.full
+ else
+ $SHELL -c "$cmd" >> $seqres.full
+ fi
+
+ _scratch_cycle_mount
+ md5sum $SCRATCH_MNT/a | _filter_scratch
+ stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
+
+ # Blank line in output
+ echo
+}
+
+# Commit to a non-exec file by an unprivileged user clears suid but leaves
+# sgid.
+echo "Test 1 - qa_user, non-exec file"
+setup_testfile
+chmod a+rws $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a group-exec file by an unprivileged user clears suid and sgid.
+echo "Test 2 - qa_user, group-exec file"
+setup_testfile
+chmod g+x,a+rws $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
+echo "Test 3 - qa_user, user-exec file"
+setup_testfile
+chmod u+x,a+rws,g-x $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a all-exec file by an unprivileged user clears suid and sgid.
+echo "Test 4 - qa_user, all-exec file"
+setup_testfile
+chmod a+rwxs $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a non-exec file by root clears suid but leaves sgid.
+echo "Test 5 - root, non-exec file"
+setup_testfile
+chmod a+rws $SCRATCH_MNT/a
+commit_and_check
+
+# Commit to a group-exec file by root clears suid and sgid.
+echo "Test 6 - root, group-exec file"
+setup_testfile
+chmod g+x,a+rws $SCRATCH_MNT/a
+commit_and_check
+
+# Commit to a user-exec file by root clears suid but not sgid.
+echo "Test 7 - root, user-exec file"
+setup_testfile
+chmod u+x,a+rws,g-x $SCRATCH_MNT/a
+commit_and_check
+
+# Commit to a all-exec file by root clears suid and sgid.
+echo "Test 8 - root, all-exec file"
+setup_testfile
+chmod a+rwxs $SCRATCH_MNT/a
+commit_and_check
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/950.out b/tests/generic/950.out
new file mode 100644
index 00000000..b42e4931
--- /dev/null
+++ b/tests/generic/950.out
@@ -0,0 +1,49 @@
+QA output created by 950
+Test 1 - qa_user, non-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+2666 -rw-rwSrw- SCRATCH_MNT/a
+
+Test 2 - qa_user, group-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+676 -rw-rwxrw- SCRATCH_MNT/a
+
+Test 3 - qa_user, user-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+2766 -rwxrwSrw- SCRATCH_MNT/a
+
+Test 4 - qa_user, all-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+777 -rwxrwxrwx SCRATCH_MNT/a
+
+Test 5 - root, non-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+
+Test 6 - root, group-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+
+Test 7 - root, user-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+
+Test 8 - root, all-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+
diff --git a/tests/generic/951 b/tests/generic/951
new file mode 100755
index 00000000..99f67ab7
--- /dev/null
+++ b/tests/generic/951
@@ -0,0 +1,118 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 Oracle. All Rights Reserved.
+#
+# FS QA Test 951
+#
+# Functional test for dropping suid and sgid bits as part of a deduplication.
+#
+. ./common/preamble
+_begin_fstest auto clone quick
+
+# Import common functions.
+. ./common/filter
+. ./common/reflink
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_require_user
+_require_scratch_reflink
+_require_xfs_io_command dedupe
+
+_scratch_mkfs >> $seqres.full
+_scratch_mount
+_require_congruent_file_oplen $SCRATCH_MNT 1048576
+chmod a+rw $SCRATCH_MNT/
+
+setup_testfile() {
+ rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
+ _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
+ _pwrite_byte 0x58 0 1m $SCRATCH_MNT/b >> $seqres.full
+ chmod a+r $SCRATCH_MNT/b
+ sync
+}
+
+commit_and_check() {
+ local user="$1"
+
+ md5sum $SCRATCH_MNT/a | _filter_scratch
+ stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
+
+ local before_freesp=$(_get_available_space $SCRATCH_MNT)
+
+ local cmd="$XFS_IO_PROG -c 'dedupe $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
+ if [ -n "$user" ]; then
+ su - "$user" -c "$cmd" >> $seqres.full
+ else
+ $SHELL -c "$cmd" >> $seqres.full
+ fi
+
+ _scratch_cycle_mount
+ md5sum $SCRATCH_MNT/a | _filter_scratch
+ stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
+
+ local after_freesp=$(_get_available_space $SCRATCH_MNT)
+
+ echo "before: $before_freesp; after: $after_freesp" >> $seqres.full
+ if [ $after_freesp -le $before_freesp ]; then
+ echo "expected more free space after dedupe"
+ fi
+
+ # Blank line in output
+ echo
+}
+
+# Commit to a non-exec file by an unprivileged user clears suid but leaves
+# sgid.
+echo "Test 1 - qa_user, non-exec file"
+setup_testfile
+chmod a+rws $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a group-exec file by an unprivileged user clears suid and sgid.
+echo "Test 2 - qa_user, group-exec file"
+setup_testfile
+chmod g+x,a+rws $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
+echo "Test 3 - qa_user, user-exec file"
+setup_testfile
+chmod u+x,a+rws,g-x $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a all-exec file by an unprivileged user clears suid and sgid.
+echo "Test 4 - qa_user, all-exec file"
+setup_testfile
+chmod a+rwxs $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a non-exec file by root clears suid but leaves sgid.
+echo "Test 5 - root, non-exec file"
+setup_testfile
+chmod a+rws $SCRATCH_MNT/a
+commit_and_check
+
+# Commit to a group-exec file by root clears suid and sgid.
+echo "Test 6 - root, group-exec file"
+setup_testfile
+chmod g+x,a+rws $SCRATCH_MNT/a
+commit_and_check
+
+# Commit to a user-exec file by root clears suid but not sgid.
+echo "Test 7 - root, user-exec file"
+setup_testfile
+chmod u+x,a+rws,g-x $SCRATCH_MNT/a
+commit_and_check
+
+# Commit to a all-exec file by root clears suid and sgid.
+echo "Test 8 - root, all-exec file"
+setup_testfile
+chmod a+rwxs $SCRATCH_MNT/a
+commit_and_check
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/951.out b/tests/generic/951.out
new file mode 100644
index 00000000..f7099ea2
--- /dev/null
+++ b/tests/generic/951.out
@@ -0,0 +1,49 @@
+QA output created by 951
+Test 1 - qa_user, non-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+
+Test 2 - qa_user, group-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+
+Test 3 - qa_user, user-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+
+Test 4 - qa_user, all-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+
+Test 5 - root, non-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+
+Test 6 - root, group-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+
+Test 7 - root, user-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+
+Test 8 - root, all-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+
diff --git a/tests/generic/952 b/tests/generic/952
new file mode 100755
index 00000000..470d73bd
--- /dev/null
+++ b/tests/generic/952
@@ -0,0 +1,71 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 Oracle. All Rights Reserved.
+#
+# FS QA Test 952
+#
+# Functional test for dropping suid and sgid capabilities as part of a reflink.
+#
+. ./common/preamble
+_begin_fstest auto clone quick
+
+# Import common functions.
+. ./common/filter
+. ./common/reflink
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_require_user
+_require_command "$GETCAP_PROG" getcap
+_require_command "$SETCAP_PROG" setcap
+_require_scratch_reflink
+
+_scratch_mkfs >> $seqres.full
+_scratch_mount
+_require_congruent_file_oplen $SCRATCH_MNT 1048576
+chmod a+rw $SCRATCH_MNT/
+
+setup_testfile() {
+ rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
+ _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
+ _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
+ chmod a+rwx $SCRATCH_MNT/a $SCRATCH_MNT/b
+ $SETCAP_PROG cap_setgid,cap_setuid+ep $SCRATCH_MNT/a
+ sync
+}
+
+commit_and_check() {
+ local user="$1"
+
+ stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
+ _getcap -v $SCRATCH_MNT/a | _filter_scratch
+
+ local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
+ if [ -n "$user" ]; then
+ su - "$user" -c "$cmd" >> $seqres.full
+ else
+ $SHELL -c "$cmd" >> $seqres.full
+ fi
+
+ stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
+ _getcap -v $SCRATCH_MNT/a | _filter_scratch
+
+ # Blank line in output
+ echo
+}
+
+# Commit by an unprivileged user clears capability bits.
+echo "Test 1 - qa_user"
+setup_testfile
+commit_and_check "$qa_user"
+
+# Commit by root leaves capability bits.
+echo "Test 2 - root"
+setup_testfile
+commit_and_check
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/952.out b/tests/generic/952.out
new file mode 100644
index 00000000..eac9e76a
--- /dev/null
+++ b/tests/generic/952.out
@@ -0,0 +1,13 @@
+QA output created by 952
+Test 1 - qa_user
+777 -rwxrwxrwx SCRATCH_MNT/a
+SCRATCH_MNT/a cap_setgid,cap_setuid=ep
+777 -rwxrwxrwx SCRATCH_MNT/a
+SCRATCH_MNT/a
+
+Test 2 - root
+777 -rwxrwxrwx SCRATCH_MNT/a
+SCRATCH_MNT/a cap_setgid,cap_setuid=ep
+777 -rwxrwxrwx SCRATCH_MNT/a
+SCRATCH_MNT/a
+
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] fstests: skip tests that require XFS_IOC_ALLOCSP
2022-01-26 2:11 [PATCHSET 0/2] fstests: new stuff for kernel 5.17 Darrick J. Wong
2022-01-26 2:11 ` [PATCH 1/2] generic: test suid/sgid behavior with reflink and dedupe Darrick J. Wong
@ 2022-01-26 2:11 ` Darrick J. Wong
2022-01-26 6:13 ` Zorro Lang
1 sibling, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2022-01-26 2:11 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: linux-xfs, fstests, guan
From: Darrick J. Wong <djwong@kernel.org>
Deprecating this, so turn off the tests that require it.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
common/rc | 4 ++--
ltp/fsstress.c | 4 ++++
tests/xfs/107 | 1 +
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/common/rc b/common/rc
index b3289de9..6a0648ad 100644
--- a/common/rc
+++ b/common/rc
@@ -2507,8 +2507,8 @@ _require_xfs_io_command()
rm -f $testcopy > /dev/null 2>&1
param_checked="$param"
;;
- "falloc" )
- testio=`$XFS_IO_PROG -F -f -c "falloc $param 0 1m" $testfile 2>&1`
+ "falloc"|"allocsp")
+ testio=`$XFS_IO_PROG -F -f -c "$command $param 0 1m" $testfile 2>&1`
param_checked="$param"
;;
"fpunch" | "fcollapse" | "zero" | "fzero" | "finsert" | "funshare")
diff --git a/ltp/fsstress.c b/ltp/fsstress.c
index 5f3126e6..23188467 100644
--- a/ltp/fsstress.c
+++ b/ltp/fsstress.c
@@ -2045,6 +2045,7 @@ afsync_f(opnum_t opno, long r)
void
allocsp_f(opnum_t opno, long r)
{
+#ifdef XFS_IOC_ALLOCSP64
int e;
pathname_t f;
int fd;
@@ -2094,6 +2095,7 @@ allocsp_f(opnum_t opno, long r)
}
free_pathname(&f);
close(fd);
+#endif
}
#ifdef AIO
@@ -3733,6 +3735,7 @@ fiemap_f(opnum_t opno, long r)
void
freesp_f(opnum_t opno, long r)
{
+#ifdef XFS_IOC_FREESP64
int e;
pathname_t f;
int fd;
@@ -3781,6 +3784,7 @@ freesp_f(opnum_t opno, long r)
procid, opno, f.path, st, (long long)off, e);
free_pathname(&f);
close(fd);
+#endif
}
void
diff --git a/tests/xfs/107 b/tests/xfs/107
index 577094b2..1ea9c492 100755
--- a/tests/xfs/107
+++ b/tests/xfs/107
@@ -20,6 +20,7 @@ _begin_fstest auto quick prealloc
_supported_fs xfs
_require_test
_require_scratch
+_require_xfs_io_command allocsp # detect presence of ALLOCSP ioctl
_require_test_program allocstale
# Create a 256MB filesystem to avoid running into mkfs problems with too-small
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] generic: test suid/sgid behavior with reflink and dedupe
2022-01-26 2:11 ` [PATCH 1/2] generic: test suid/sgid behavior with reflink and dedupe Darrick J. Wong
@ 2022-01-26 5:56 ` Zorro Lang
2022-01-27 0:55 ` Darrick J. Wong
2022-01-27 1:27 ` [PATCH v1.1 " Darrick J. Wong
1 sibling, 1 reply; 11+ messages in thread
From: Zorro Lang @ 2022-01-26 5:56 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: guaneryu, linux-xfs, fstests, guan
On Tue, Jan 25, 2022 at 06:11:49PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Make sure that we drop the setuid and setgid bits any time reflink or
> dedupe change the file contents.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> tests/generic/950 | 108 +++++++++++++++++++++++++++++++++++++++++++++
> tests/generic/950.out | 49 ++++++++++++++++++++
> tests/generic/951 | 118 +++++++++++++++++++++++++++++++++++++++++++++++++
> tests/generic/951.out | 49 ++++++++++++++++++++
> tests/generic/952 | 71 +++++++++++++++++++++++++++++
> tests/generic/952.out | 13 +++++
> 6 files changed, 408 insertions(+)
> create mode 100755 tests/generic/950
> create mode 100644 tests/generic/950.out
> create mode 100755 tests/generic/951
> create mode 100644 tests/generic/951.out
> create mode 100755 tests/generic/952
> create mode 100644 tests/generic/952.out
>
>
> diff --git a/tests/generic/950 b/tests/generic/950
> new file mode 100755
> index 00000000..84f1d1f0
> --- /dev/null
> +++ b/tests/generic/950
> @@ -0,0 +1,108 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 950
> +#
> +# Functional test for dropping suid and sgid bits as part of a reflink.
> +#
> +. ./common/preamble
> +_begin_fstest auto clone quick
> +
> +# Import common functions.
> +. ./common/filter
> +. ./common/reflink
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_require_user
> +_require_scratch_reflink
> +
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount
> +_require_congruent_file_oplen $SCRATCH_MNT 1048576
Is this a Darrick's secret helper ? :)
Thanks,
Zorro
> +chmod a+rw $SCRATCH_MNT/
> +
> +setup_testfile() {
> + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> + _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
> + chmod a+r $SCRATCH_MNT/b
> + sync
> +}
> +
> +commit_and_check() {
> + local user="$1"
> +
> + md5sum $SCRATCH_MNT/a | _filter_scratch
> + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> +
> + local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> + if [ -n "$user" ]; then
> + su - "$user" -c "$cmd" >> $seqres.full
> + else
> + $SHELL -c "$cmd" >> $seqres.full
> + fi
> +
> + _scratch_cycle_mount
> + md5sum $SCRATCH_MNT/a | _filter_scratch
> + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> +
> + # Blank line in output
> + echo
> +}
> +
> +# Commit to a non-exec file by an unprivileged user clears suid but leaves
> +# sgid.
> +echo "Test 1 - qa_user, non-exec file"
> +setup_testfile
> +chmod a+rws $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a group-exec file by an unprivileged user clears suid and sgid.
> +echo "Test 2 - qa_user, group-exec file"
> +setup_testfile
> +chmod g+x,a+rws $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
> +echo "Test 3 - qa_user, user-exec file"
> +setup_testfile
> +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a all-exec file by an unprivileged user clears suid and sgid.
> +echo "Test 4 - qa_user, all-exec file"
> +setup_testfile
> +chmod a+rwxs $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a non-exec file by root clears suid but leaves sgid.
> +echo "Test 5 - root, non-exec file"
> +setup_testfile
> +chmod a+rws $SCRATCH_MNT/a
> +commit_and_check
> +
> +# Commit to a group-exec file by root clears suid and sgid.
> +echo "Test 6 - root, group-exec file"
> +setup_testfile
> +chmod g+x,a+rws $SCRATCH_MNT/a
> +commit_and_check
> +
> +# Commit to a user-exec file by root clears suid but not sgid.
> +echo "Test 7 - root, user-exec file"
> +setup_testfile
> +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> +commit_and_check
> +
> +# Commit to a all-exec file by root clears suid and sgid.
> +echo "Test 8 - root, all-exec file"
> +setup_testfile
> +chmod a+rwxs $SCRATCH_MNT/a
> +commit_and_check
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/950.out b/tests/generic/950.out
> new file mode 100644
> index 00000000..b42e4931
> --- /dev/null
> +++ b/tests/generic/950.out
> @@ -0,0 +1,49 @@
> +QA output created by 950
> +Test 1 - qa_user, non-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +2666 -rw-rwSrw- SCRATCH_MNT/a
> +
> +Test 2 - qa_user, group-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +676 -rw-rwxrw- SCRATCH_MNT/a
> +
> +Test 3 - qa_user, user-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +2766 -rwxrwSrw- SCRATCH_MNT/a
> +
> +Test 4 - qa_user, all-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +777 -rwxrwxrwx SCRATCH_MNT/a
> +
> +Test 5 - root, non-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +
> +Test 6 - root, group-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +
> +Test 7 - root, user-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +
> +Test 8 - root, all-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +
> diff --git a/tests/generic/951 b/tests/generic/951
> new file mode 100755
> index 00000000..99f67ab7
> --- /dev/null
> +++ b/tests/generic/951
> @@ -0,0 +1,118 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 951
> +#
> +# Functional test for dropping suid and sgid bits as part of a deduplication.
> +#
> +. ./common/preamble
> +_begin_fstest auto clone quick
> +
> +# Import common functions.
> +. ./common/filter
> +. ./common/reflink
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_require_user
> +_require_scratch_reflink
> +_require_xfs_io_command dedupe
> +
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount
> +_require_congruent_file_oplen $SCRATCH_MNT 1048576
> +chmod a+rw $SCRATCH_MNT/
> +
> +setup_testfile() {
> + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/b >> $seqres.full
> + chmod a+r $SCRATCH_MNT/b
> + sync
> +}
> +
> +commit_and_check() {
> + local user="$1"
> +
> + md5sum $SCRATCH_MNT/a | _filter_scratch
> + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> +
> + local before_freesp=$(_get_available_space $SCRATCH_MNT)
> +
> + local cmd="$XFS_IO_PROG -c 'dedupe $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> + if [ -n "$user" ]; then
> + su - "$user" -c "$cmd" >> $seqres.full
> + else
> + $SHELL -c "$cmd" >> $seqres.full
> + fi
> +
> + _scratch_cycle_mount
> + md5sum $SCRATCH_MNT/a | _filter_scratch
> + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> +
> + local after_freesp=$(_get_available_space $SCRATCH_MNT)
> +
> + echo "before: $before_freesp; after: $after_freesp" >> $seqres.full
> + if [ $after_freesp -le $before_freesp ]; then
> + echo "expected more free space after dedupe"
> + fi
> +
> + # Blank line in output
> + echo
> +}
> +
> +# Commit to a non-exec file by an unprivileged user clears suid but leaves
> +# sgid.
> +echo "Test 1 - qa_user, non-exec file"
> +setup_testfile
> +chmod a+rws $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a group-exec file by an unprivileged user clears suid and sgid.
> +echo "Test 2 - qa_user, group-exec file"
> +setup_testfile
> +chmod g+x,a+rws $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
> +echo "Test 3 - qa_user, user-exec file"
> +setup_testfile
> +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a all-exec file by an unprivileged user clears suid and sgid.
> +echo "Test 4 - qa_user, all-exec file"
> +setup_testfile
> +chmod a+rwxs $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a non-exec file by root clears suid but leaves sgid.
> +echo "Test 5 - root, non-exec file"
> +setup_testfile
> +chmod a+rws $SCRATCH_MNT/a
> +commit_and_check
> +
> +# Commit to a group-exec file by root clears suid and sgid.
> +echo "Test 6 - root, group-exec file"
> +setup_testfile
> +chmod g+x,a+rws $SCRATCH_MNT/a
> +commit_and_check
> +
> +# Commit to a user-exec file by root clears suid but not sgid.
> +echo "Test 7 - root, user-exec file"
> +setup_testfile
> +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> +commit_and_check
> +
> +# Commit to a all-exec file by root clears suid and sgid.
> +echo "Test 8 - root, all-exec file"
> +setup_testfile
> +chmod a+rwxs $SCRATCH_MNT/a
> +commit_and_check
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/951.out b/tests/generic/951.out
> new file mode 100644
> index 00000000..f7099ea2
> --- /dev/null
> +++ b/tests/generic/951.out
> @@ -0,0 +1,49 @@
> +QA output created by 951
> +Test 1 - qa_user, non-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +
> +Test 2 - qa_user, group-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +
> +Test 3 - qa_user, user-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +
> +Test 4 - qa_user, all-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +
> +Test 5 - root, non-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +
> +Test 6 - root, group-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +
> +Test 7 - root, user-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +
> +Test 8 - root, all-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +
> diff --git a/tests/generic/952 b/tests/generic/952
> new file mode 100755
> index 00000000..470d73bd
> --- /dev/null
> +++ b/tests/generic/952
> @@ -0,0 +1,71 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 952
> +#
> +# Functional test for dropping suid and sgid capabilities as part of a reflink.
> +#
> +. ./common/preamble
> +_begin_fstest auto clone quick
> +
> +# Import common functions.
> +. ./common/filter
> +. ./common/reflink
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_require_user
> +_require_command "$GETCAP_PROG" getcap
> +_require_command "$SETCAP_PROG" setcap
> +_require_scratch_reflink
> +
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount
> +_require_congruent_file_oplen $SCRATCH_MNT 1048576
> +chmod a+rw $SCRATCH_MNT/
> +
> +setup_testfile() {
> + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> + _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
> + chmod a+rwx $SCRATCH_MNT/a $SCRATCH_MNT/b
> + $SETCAP_PROG cap_setgid,cap_setuid+ep $SCRATCH_MNT/a
> + sync
> +}
> +
> +commit_and_check() {
> + local user="$1"
> +
> + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> + _getcap -v $SCRATCH_MNT/a | _filter_scratch
> +
> + local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> + if [ -n "$user" ]; then
> + su - "$user" -c "$cmd" >> $seqres.full
> + else
> + $SHELL -c "$cmd" >> $seqres.full
> + fi
> +
> + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> + _getcap -v $SCRATCH_MNT/a | _filter_scratch
> +
> + # Blank line in output
> + echo
> +}
> +
> +# Commit by an unprivileged user clears capability bits.
> +echo "Test 1 - qa_user"
> +setup_testfile
> +commit_and_check "$qa_user"
> +
> +# Commit by root leaves capability bits.
> +echo "Test 2 - root"
> +setup_testfile
> +commit_and_check
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/952.out b/tests/generic/952.out
> new file mode 100644
> index 00000000..eac9e76a
> --- /dev/null
> +++ b/tests/generic/952.out
> @@ -0,0 +1,13 @@
> +QA output created by 952
> +Test 1 - qa_user
> +777 -rwxrwxrwx SCRATCH_MNT/a
> +SCRATCH_MNT/a cap_setgid,cap_setuid=ep
> +777 -rwxrwxrwx SCRATCH_MNT/a
> +SCRATCH_MNT/a
> +
> +Test 2 - root
> +777 -rwxrwxrwx SCRATCH_MNT/a
> +SCRATCH_MNT/a cap_setgid,cap_setuid=ep
> +777 -rwxrwxrwx SCRATCH_MNT/a
> +SCRATCH_MNT/a
> +
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] fstests: skip tests that require XFS_IOC_ALLOCSP
2022-01-26 2:11 ` [PATCH 2/2] fstests: skip tests that require XFS_IOC_ALLOCSP Darrick J. Wong
@ 2022-01-26 6:13 ` Zorro Lang
0 siblings, 0 replies; 11+ messages in thread
From: Zorro Lang @ 2022-01-26 6:13 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: guaneryu, linux-xfs, fstests, guan
On Tue, Jan 25, 2022 at 06:11:54PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Deprecating this, so turn off the tests that require it.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
Looks good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>
> common/rc | 4 ++--
> ltp/fsstress.c | 4 ++++
> tests/xfs/107 | 1 +
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
>
> diff --git a/common/rc b/common/rc
> index b3289de9..6a0648ad 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2507,8 +2507,8 @@ _require_xfs_io_command()
> rm -f $testcopy > /dev/null 2>&1
> param_checked="$param"
> ;;
> - "falloc" )
> - testio=`$XFS_IO_PROG -F -f -c "falloc $param 0 1m" $testfile 2>&1`
> + "falloc"|"allocsp")
> + testio=`$XFS_IO_PROG -F -f -c "$command $param 0 1m" $testfile 2>&1`
> param_checked="$param"
> ;;
> "fpunch" | "fcollapse" | "zero" | "fzero" | "finsert" | "funshare")
> diff --git a/ltp/fsstress.c b/ltp/fsstress.c
> index 5f3126e6..23188467 100644
> --- a/ltp/fsstress.c
> +++ b/ltp/fsstress.c
> @@ -2045,6 +2045,7 @@ afsync_f(opnum_t opno, long r)
> void
> allocsp_f(opnum_t opno, long r)
> {
> +#ifdef XFS_IOC_ALLOCSP64
> int e;
> pathname_t f;
> int fd;
> @@ -2094,6 +2095,7 @@ allocsp_f(opnum_t opno, long r)
> }
> free_pathname(&f);
> close(fd);
> +#endif
> }
>
> #ifdef AIO
> @@ -3733,6 +3735,7 @@ fiemap_f(opnum_t opno, long r)
> void
> freesp_f(opnum_t opno, long r)
> {
> +#ifdef XFS_IOC_FREESP64
> int e;
> pathname_t f;
> int fd;
> @@ -3781,6 +3784,7 @@ freesp_f(opnum_t opno, long r)
> procid, opno, f.path, st, (long long)off, e);
> free_pathname(&f);
> close(fd);
> +#endif
> }
>
> void
> diff --git a/tests/xfs/107 b/tests/xfs/107
> index 577094b2..1ea9c492 100755
> --- a/tests/xfs/107
> +++ b/tests/xfs/107
> @@ -20,6 +20,7 @@ _begin_fstest auto quick prealloc
> _supported_fs xfs
> _require_test
> _require_scratch
> +_require_xfs_io_command allocsp # detect presence of ALLOCSP ioctl
> _require_test_program allocstale
>
> # Create a 256MB filesystem to avoid running into mkfs problems with too-small
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] generic: test suid/sgid behavior with reflink and dedupe
2022-01-26 5:56 ` Zorro Lang
@ 2022-01-27 0:55 ` Darrick J. Wong
2022-01-27 2:52 ` Zorro Lang
0 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2022-01-27 0:55 UTC (permalink / raw)
To: guaneryu, linux-xfs, fstests, guan
On Wed, Jan 26, 2022 at 01:56:41PM +0800, Zorro Lang wrote:
> On Tue, Jan 25, 2022 at 06:11:49PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > Make sure that we drop the setuid and setgid bits any time reflink or
> > dedupe change the file contents.
> >
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> > tests/generic/950 | 108 +++++++++++++++++++++++++++++++++++++++++++++
> > tests/generic/950.out | 49 ++++++++++++++++++++
> > tests/generic/951 | 118 +++++++++++++++++++++++++++++++++++++++++++++++++
> > tests/generic/951.out | 49 ++++++++++++++++++++
> > tests/generic/952 | 71 +++++++++++++++++++++++++++++
> > tests/generic/952.out | 13 +++++
> > 6 files changed, 408 insertions(+)
> > create mode 100755 tests/generic/950
> > create mode 100644 tests/generic/950.out
> > create mode 100755 tests/generic/951
> > create mode 100644 tests/generic/951.out
> > create mode 100755 tests/generic/952
> > create mode 100644 tests/generic/952.out
> >
> >
> > diff --git a/tests/generic/950 b/tests/generic/950
> > new file mode 100755
> > index 00000000..84f1d1f0
> > --- /dev/null
> > +++ b/tests/generic/950
> > @@ -0,0 +1,108 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > +#
> > +# FS QA Test 950
> > +#
> > +# Functional test for dropping suid and sgid bits as part of a reflink.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto clone quick
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +. ./common/reflink
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_require_user
> > +_require_scratch_reflink
> > +
> > +_scratch_mkfs >> $seqres.full
> > +_scratch_mount
> > +_require_congruent_file_oplen $SCRATCH_MNT 1048576
>
> Is this a Darrick's secret helper ? :)
Oops, yes. Will resend.
There's a big old patchset in djwong-dev that teaches all the reflink
tests to _notrun if the file allocation unit is not congruent (i.e. an
integer multiple or factor) with the operation sizes used in the test.
I should probably move that up in the branch, but with Lunar New Year
starting next week there probably isn't much point... ;)
--D
> Thanks,
> Zorro
>
> > +chmod a+rw $SCRATCH_MNT/
> > +
> > +setup_testfile() {
> > + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> > + _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
> > + chmod a+r $SCRATCH_MNT/b
> > + sync
> > +}
> > +
> > +commit_and_check() {
> > + local user="$1"
> > +
> > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > +
> > + local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> > + if [ -n "$user" ]; then
> > + su - "$user" -c "$cmd" >> $seqres.full
> > + else
> > + $SHELL -c "$cmd" >> $seqres.full
> > + fi
> > +
> > + _scratch_cycle_mount
> > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > +
> > + # Blank line in output
> > + echo
> > +}
> > +
> > +# Commit to a non-exec file by an unprivileged user clears suid but leaves
> > +# sgid.
> > +echo "Test 1 - qa_user, non-exec file"
> > +setup_testfile
> > +chmod a+rws $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a group-exec file by an unprivileged user clears suid and sgid.
> > +echo "Test 2 - qa_user, group-exec file"
> > +setup_testfile
> > +chmod g+x,a+rws $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
> > +echo "Test 3 - qa_user, user-exec file"
> > +setup_testfile
> > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a all-exec file by an unprivileged user clears suid and sgid.
> > +echo "Test 4 - qa_user, all-exec file"
> > +setup_testfile
> > +chmod a+rwxs $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a non-exec file by root clears suid but leaves sgid.
> > +echo "Test 5 - root, non-exec file"
> > +setup_testfile
> > +chmod a+rws $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# Commit to a group-exec file by root clears suid and sgid.
> > +echo "Test 6 - root, group-exec file"
> > +setup_testfile
> > +chmod g+x,a+rws $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# Commit to a user-exec file by root clears suid but not sgid.
> > +echo "Test 7 - root, user-exec file"
> > +setup_testfile
> > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# Commit to a all-exec file by root clears suid and sgid.
> > +echo "Test 8 - root, all-exec file"
> > +setup_testfile
> > +chmod a+rwxs $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/generic/950.out b/tests/generic/950.out
> > new file mode 100644
> > index 00000000..b42e4931
> > --- /dev/null
> > +++ b/tests/generic/950.out
> > @@ -0,0 +1,49 @@
> > +QA output created by 950
> > +Test 1 - qa_user, non-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +2666 -rw-rwSrw- SCRATCH_MNT/a
> > +
> > +Test 2 - qa_user, group-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +676 -rw-rwxrw- SCRATCH_MNT/a
> > +
> > +Test 3 - qa_user, user-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +2766 -rwxrwSrw- SCRATCH_MNT/a
> > +
> > +Test 4 - qa_user, all-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +777 -rwxrwxrwx SCRATCH_MNT/a
> > +
> > +Test 5 - root, non-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +
> > +Test 6 - root, group-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +
> > +Test 7 - root, user-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +
> > +Test 8 - root, all-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +
> > diff --git a/tests/generic/951 b/tests/generic/951
> > new file mode 100755
> > index 00000000..99f67ab7
> > --- /dev/null
> > +++ b/tests/generic/951
> > @@ -0,0 +1,118 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > +#
> > +# FS QA Test 951
> > +#
> > +# Functional test for dropping suid and sgid bits as part of a deduplication.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto clone quick
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +. ./common/reflink
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_require_user
> > +_require_scratch_reflink
> > +_require_xfs_io_command dedupe
> > +
> > +_scratch_mkfs >> $seqres.full
> > +_scratch_mount
> > +_require_congruent_file_oplen $SCRATCH_MNT 1048576
> > +chmod a+rw $SCRATCH_MNT/
> > +
> > +setup_testfile() {
> > + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/b >> $seqres.full
> > + chmod a+r $SCRATCH_MNT/b
> > + sync
> > +}
> > +
> > +commit_and_check() {
> > + local user="$1"
> > +
> > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > +
> > + local before_freesp=$(_get_available_space $SCRATCH_MNT)
> > +
> > + local cmd="$XFS_IO_PROG -c 'dedupe $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> > + if [ -n "$user" ]; then
> > + su - "$user" -c "$cmd" >> $seqres.full
> > + else
> > + $SHELL -c "$cmd" >> $seqres.full
> > + fi
> > +
> > + _scratch_cycle_mount
> > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > +
> > + local after_freesp=$(_get_available_space $SCRATCH_MNT)
> > +
> > + echo "before: $before_freesp; after: $after_freesp" >> $seqres.full
> > + if [ $after_freesp -le $before_freesp ]; then
> > + echo "expected more free space after dedupe"
> > + fi
> > +
> > + # Blank line in output
> > + echo
> > +}
> > +
> > +# Commit to a non-exec file by an unprivileged user clears suid but leaves
> > +# sgid.
> > +echo "Test 1 - qa_user, non-exec file"
> > +setup_testfile
> > +chmod a+rws $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a group-exec file by an unprivileged user clears suid and sgid.
> > +echo "Test 2 - qa_user, group-exec file"
> > +setup_testfile
> > +chmod g+x,a+rws $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
> > +echo "Test 3 - qa_user, user-exec file"
> > +setup_testfile
> > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a all-exec file by an unprivileged user clears suid and sgid.
> > +echo "Test 4 - qa_user, all-exec file"
> > +setup_testfile
> > +chmod a+rwxs $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a non-exec file by root clears suid but leaves sgid.
> > +echo "Test 5 - root, non-exec file"
> > +setup_testfile
> > +chmod a+rws $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# Commit to a group-exec file by root clears suid and sgid.
> > +echo "Test 6 - root, group-exec file"
> > +setup_testfile
> > +chmod g+x,a+rws $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# Commit to a user-exec file by root clears suid but not sgid.
> > +echo "Test 7 - root, user-exec file"
> > +setup_testfile
> > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# Commit to a all-exec file by root clears suid and sgid.
> > +echo "Test 8 - root, all-exec file"
> > +setup_testfile
> > +chmod a+rwxs $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/generic/951.out b/tests/generic/951.out
> > new file mode 100644
> > index 00000000..f7099ea2
> > --- /dev/null
> > +++ b/tests/generic/951.out
> > @@ -0,0 +1,49 @@
> > +QA output created by 951
> > +Test 1 - qa_user, non-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +
> > +Test 2 - qa_user, group-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +
> > +Test 3 - qa_user, user-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +
> > +Test 4 - qa_user, all-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +
> > +Test 5 - root, non-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +
> > +Test 6 - root, group-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +
> > +Test 7 - root, user-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +
> > +Test 8 - root, all-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +
> > diff --git a/tests/generic/952 b/tests/generic/952
> > new file mode 100755
> > index 00000000..470d73bd
> > --- /dev/null
> > +++ b/tests/generic/952
> > @@ -0,0 +1,71 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > +#
> > +# FS QA Test 952
> > +#
> > +# Functional test for dropping suid and sgid capabilities as part of a reflink.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto clone quick
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +. ./common/reflink
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_require_user
> > +_require_command "$GETCAP_PROG" getcap
> > +_require_command "$SETCAP_PROG" setcap
> > +_require_scratch_reflink
> > +
> > +_scratch_mkfs >> $seqres.full
> > +_scratch_mount
> > +_require_congruent_file_oplen $SCRATCH_MNT 1048576
> > +chmod a+rw $SCRATCH_MNT/
> > +
> > +setup_testfile() {
> > + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> > + _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
> > + chmod a+rwx $SCRATCH_MNT/a $SCRATCH_MNT/b
> > + $SETCAP_PROG cap_setgid,cap_setuid+ep $SCRATCH_MNT/a
> > + sync
> > +}
> > +
> > +commit_and_check() {
> > + local user="$1"
> > +
> > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > + _getcap -v $SCRATCH_MNT/a | _filter_scratch
> > +
> > + local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> > + if [ -n "$user" ]; then
> > + su - "$user" -c "$cmd" >> $seqres.full
> > + else
> > + $SHELL -c "$cmd" >> $seqres.full
> > + fi
> > +
> > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > + _getcap -v $SCRATCH_MNT/a | _filter_scratch
> > +
> > + # Blank line in output
> > + echo
> > +}
> > +
> > +# Commit by an unprivileged user clears capability bits.
> > +echo "Test 1 - qa_user"
> > +setup_testfile
> > +commit_and_check "$qa_user"
> > +
> > +# Commit by root leaves capability bits.
> > +echo "Test 2 - root"
> > +setup_testfile
> > +commit_and_check
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/generic/952.out b/tests/generic/952.out
> > new file mode 100644
> > index 00000000..eac9e76a
> > --- /dev/null
> > +++ b/tests/generic/952.out
> > @@ -0,0 +1,13 @@
> > +QA output created by 952
> > +Test 1 - qa_user
> > +777 -rwxrwxrwx SCRATCH_MNT/a
> > +SCRATCH_MNT/a cap_setgid,cap_setuid=ep
> > +777 -rwxrwxrwx SCRATCH_MNT/a
> > +SCRATCH_MNT/a
> > +
> > +Test 2 - root
> > +777 -rwxrwxrwx SCRATCH_MNT/a
> > +SCRATCH_MNT/a cap_setgid,cap_setuid=ep
> > +777 -rwxrwxrwx SCRATCH_MNT/a
> > +SCRATCH_MNT/a
> > +
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1.1 1/2] generic: test suid/sgid behavior with reflink and dedupe
2022-01-26 2:11 ` [PATCH 1/2] generic: test suid/sgid behavior with reflink and dedupe Darrick J. Wong
2022-01-26 5:56 ` Zorro Lang
@ 2022-01-27 1:27 ` Darrick J. Wong
2022-02-16 15:09 ` Filipe Manana
1 sibling, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2022-01-27 1:27 UTC (permalink / raw)
To: guaneryu; +Cc: linux-xfs, fstests, guan
From: Darrick J. Wong <djwong@kernel.org>
Make sure that we drop the setuid and setgid bits any time reflink or
dedupe change the file contents.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
v2: drop the congruent oplen checks, that was a mismerge
---
tests/generic/950 | 107 +++++++++++++++++++++++++++++++++++++++++++++
tests/generic/950.out | 49 +++++++++++++++++++++
tests/generic/951 | 117 +++++++++++++++++++++++++++++++++++++++++++++++++
tests/generic/951.out | 49 +++++++++++++++++++++
tests/generic/952 | 70 +++++++++++++++++++++++++++++
tests/generic/952.out | 13 +++++
6 files changed, 405 insertions(+)
create mode 100755 tests/generic/950
create mode 100644 tests/generic/950.out
create mode 100755 tests/generic/951
create mode 100644 tests/generic/951.out
create mode 100755 tests/generic/952
create mode 100644 tests/generic/952.out
diff --git a/tests/generic/950 b/tests/generic/950
new file mode 100755
index 00000000..a7398cb5
--- /dev/null
+++ b/tests/generic/950
@@ -0,0 +1,107 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 Oracle. All Rights Reserved.
+#
+# FS QA Test 950
+#
+# Functional test for dropping suid and sgid bits as part of a reflink.
+#
+. ./common/preamble
+_begin_fstest auto clone quick
+
+# Import common functions.
+. ./common/filter
+. ./common/reflink
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_require_user
+_require_scratch_reflink
+
+_scratch_mkfs >> $seqres.full
+_scratch_mount
+chmod a+rw $SCRATCH_MNT/
+
+setup_testfile() {
+ rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
+ _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
+ _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
+ chmod a+r $SCRATCH_MNT/b
+ sync
+}
+
+commit_and_check() {
+ local user="$1"
+
+ md5sum $SCRATCH_MNT/a | _filter_scratch
+ stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
+
+ local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
+ if [ -n "$user" ]; then
+ su - "$user" -c "$cmd" >> $seqres.full
+ else
+ $SHELL -c "$cmd" >> $seqres.full
+ fi
+
+ _scratch_cycle_mount
+ md5sum $SCRATCH_MNT/a | _filter_scratch
+ stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
+
+ # Blank line in output
+ echo
+}
+
+# Commit to a non-exec file by an unprivileged user clears suid but leaves
+# sgid.
+echo "Test 1 - qa_user, non-exec file"
+setup_testfile
+chmod a+rws $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a group-exec file by an unprivileged user clears suid and sgid.
+echo "Test 2 - qa_user, group-exec file"
+setup_testfile
+chmod g+x,a+rws $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
+echo "Test 3 - qa_user, user-exec file"
+setup_testfile
+chmod u+x,a+rws,g-x $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a all-exec file by an unprivileged user clears suid and sgid.
+echo "Test 4 - qa_user, all-exec file"
+setup_testfile
+chmod a+rwxs $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a non-exec file by root clears suid but leaves sgid.
+echo "Test 5 - root, non-exec file"
+setup_testfile
+chmod a+rws $SCRATCH_MNT/a
+commit_and_check
+
+# Commit to a group-exec file by root clears suid and sgid.
+echo "Test 6 - root, group-exec file"
+setup_testfile
+chmod g+x,a+rws $SCRATCH_MNT/a
+commit_and_check
+
+# Commit to a user-exec file by root clears suid but not sgid.
+echo "Test 7 - root, user-exec file"
+setup_testfile
+chmod u+x,a+rws,g-x $SCRATCH_MNT/a
+commit_and_check
+
+# Commit to a all-exec file by root clears suid and sgid.
+echo "Test 8 - root, all-exec file"
+setup_testfile
+chmod a+rwxs $SCRATCH_MNT/a
+commit_and_check
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/950.out b/tests/generic/950.out
new file mode 100644
index 00000000..b42e4931
--- /dev/null
+++ b/tests/generic/950.out
@@ -0,0 +1,49 @@
+QA output created by 950
+Test 1 - qa_user, non-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+2666 -rw-rwSrw- SCRATCH_MNT/a
+
+Test 2 - qa_user, group-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+676 -rw-rwxrw- SCRATCH_MNT/a
+
+Test 3 - qa_user, user-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+2766 -rwxrwSrw- SCRATCH_MNT/a
+
+Test 4 - qa_user, all-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+777 -rwxrwxrwx SCRATCH_MNT/a
+
+Test 5 - root, non-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+
+Test 6 - root, group-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+
+Test 7 - root, user-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+
+Test 8 - root, all-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+
diff --git a/tests/generic/951 b/tests/generic/951
new file mode 100755
index 00000000..8484f225
--- /dev/null
+++ b/tests/generic/951
@@ -0,0 +1,117 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 Oracle. All Rights Reserved.
+#
+# FS QA Test 951
+#
+# Functional test for dropping suid and sgid bits as part of a deduplication.
+#
+. ./common/preamble
+_begin_fstest auto clone quick
+
+# Import common functions.
+. ./common/filter
+. ./common/reflink
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_require_user
+_require_scratch_reflink
+_require_xfs_io_command dedupe
+
+_scratch_mkfs >> $seqres.full
+_scratch_mount
+chmod a+rw $SCRATCH_MNT/
+
+setup_testfile() {
+ rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
+ _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
+ _pwrite_byte 0x58 0 1m $SCRATCH_MNT/b >> $seqres.full
+ chmod a+r $SCRATCH_MNT/b
+ sync
+}
+
+commit_and_check() {
+ local user="$1"
+
+ md5sum $SCRATCH_MNT/a | _filter_scratch
+ stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
+
+ local before_freesp=$(_get_available_space $SCRATCH_MNT)
+
+ local cmd="$XFS_IO_PROG -c 'dedupe $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
+ if [ -n "$user" ]; then
+ su - "$user" -c "$cmd" >> $seqres.full
+ else
+ $SHELL -c "$cmd" >> $seqres.full
+ fi
+
+ _scratch_cycle_mount
+ md5sum $SCRATCH_MNT/a | _filter_scratch
+ stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
+
+ local after_freesp=$(_get_available_space $SCRATCH_MNT)
+
+ echo "before: $before_freesp; after: $after_freesp" >> $seqres.full
+ if [ $after_freesp -le $before_freesp ]; then
+ echo "expected more free space after dedupe"
+ fi
+
+ # Blank line in output
+ echo
+}
+
+# Commit to a non-exec file by an unprivileged user clears suid but leaves
+# sgid.
+echo "Test 1 - qa_user, non-exec file"
+setup_testfile
+chmod a+rws $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a group-exec file by an unprivileged user clears suid and sgid.
+echo "Test 2 - qa_user, group-exec file"
+setup_testfile
+chmod g+x,a+rws $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
+echo "Test 3 - qa_user, user-exec file"
+setup_testfile
+chmod u+x,a+rws,g-x $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a all-exec file by an unprivileged user clears suid and sgid.
+echo "Test 4 - qa_user, all-exec file"
+setup_testfile
+chmod a+rwxs $SCRATCH_MNT/a
+commit_and_check "$qa_user"
+
+# Commit to a non-exec file by root clears suid but leaves sgid.
+echo "Test 5 - root, non-exec file"
+setup_testfile
+chmod a+rws $SCRATCH_MNT/a
+commit_and_check
+
+# Commit to a group-exec file by root clears suid and sgid.
+echo "Test 6 - root, group-exec file"
+setup_testfile
+chmod g+x,a+rws $SCRATCH_MNT/a
+commit_and_check
+
+# Commit to a user-exec file by root clears suid but not sgid.
+echo "Test 7 - root, user-exec file"
+setup_testfile
+chmod u+x,a+rws,g-x $SCRATCH_MNT/a
+commit_and_check
+
+# Commit to a all-exec file by root clears suid and sgid.
+echo "Test 8 - root, all-exec file"
+setup_testfile
+chmod a+rwxs $SCRATCH_MNT/a
+commit_and_check
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/951.out b/tests/generic/951.out
new file mode 100644
index 00000000..f7099ea2
--- /dev/null
+++ b/tests/generic/951.out
@@ -0,0 +1,49 @@
+QA output created by 951
+Test 1 - qa_user, non-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+
+Test 2 - qa_user, group-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+
+Test 3 - qa_user, user-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+
+Test 4 - qa_user, all-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+
+Test 5 - root, non-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6666 -rwSrwSrw- SCRATCH_MNT/a
+
+Test 6 - root, group-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6676 -rwSrwsrw- SCRATCH_MNT/a
+
+Test 7 - root, user-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6766 -rwsrwSrw- SCRATCH_MNT/a
+
+Test 8 - root, all-exec file
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
+6777 -rwsrwsrwx SCRATCH_MNT/a
+
diff --git a/tests/generic/952 b/tests/generic/952
new file mode 100755
index 00000000..86443dcc
--- /dev/null
+++ b/tests/generic/952
@@ -0,0 +1,70 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 Oracle. All Rights Reserved.
+#
+# FS QA Test 952
+#
+# Functional test for dropping suid and sgid capabilities as part of a reflink.
+#
+. ./common/preamble
+_begin_fstest auto clone quick
+
+# Import common functions.
+. ./common/filter
+. ./common/reflink
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_require_user
+_require_command "$GETCAP_PROG" getcap
+_require_command "$SETCAP_PROG" setcap
+_require_scratch_reflink
+
+_scratch_mkfs >> $seqres.full
+_scratch_mount
+chmod a+rw $SCRATCH_MNT/
+
+setup_testfile() {
+ rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
+ _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
+ _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
+ chmod a+rwx $SCRATCH_MNT/a $SCRATCH_MNT/b
+ $SETCAP_PROG cap_setgid,cap_setuid+ep $SCRATCH_MNT/a
+ sync
+}
+
+commit_and_check() {
+ local user="$1"
+
+ stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
+ _getcap -v $SCRATCH_MNT/a | _filter_scratch
+
+ local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
+ if [ -n "$user" ]; then
+ su - "$user" -c "$cmd" >> $seqres.full
+ else
+ $SHELL -c "$cmd" >> $seqres.full
+ fi
+
+ stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
+ _getcap -v $SCRATCH_MNT/a | _filter_scratch
+
+ # Blank line in output
+ echo
+}
+
+# Commit by an unprivileged user clears capability bits.
+echo "Test 1 - qa_user"
+setup_testfile
+commit_and_check "$qa_user"
+
+# Commit by root leaves capability bits.
+echo "Test 2 - root"
+setup_testfile
+commit_and_check
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/952.out b/tests/generic/952.out
new file mode 100644
index 00000000..eac9e76a
--- /dev/null
+++ b/tests/generic/952.out
@@ -0,0 +1,13 @@
+QA output created by 952
+Test 1 - qa_user
+777 -rwxrwxrwx SCRATCH_MNT/a
+SCRATCH_MNT/a cap_setgid,cap_setuid=ep
+777 -rwxrwxrwx SCRATCH_MNT/a
+SCRATCH_MNT/a
+
+Test 2 - root
+777 -rwxrwxrwx SCRATCH_MNT/a
+SCRATCH_MNT/a cap_setgid,cap_setuid=ep
+777 -rwxrwxrwx SCRATCH_MNT/a
+SCRATCH_MNT/a
+
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] generic: test suid/sgid behavior with reflink and dedupe
2022-01-27 0:55 ` Darrick J. Wong
@ 2022-01-27 2:52 ` Zorro Lang
0 siblings, 0 replies; 11+ messages in thread
From: Zorro Lang @ 2022-01-27 2:52 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, fstests
On Wed, Jan 26, 2022 at 04:55:56PM -0800, Darrick J. Wong wrote:
> On Wed, Jan 26, 2022 at 01:56:41PM +0800, Zorro Lang wrote:
> > On Tue, Jan 25, 2022 at 06:11:49PM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > >
> > > Make sure that we drop the setuid and setgid bits any time reflink or
> > > dedupe change the file contents.
> > >
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > > tests/generic/950 | 108 +++++++++++++++++++++++++++++++++++++++++++++
> > > tests/generic/950.out | 49 ++++++++++++++++++++
> > > tests/generic/951 | 118 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > tests/generic/951.out | 49 ++++++++++++++++++++
> > > tests/generic/952 | 71 +++++++++++++++++++++++++++++
> > > tests/generic/952.out | 13 +++++
> > > 6 files changed, 408 insertions(+)
> > > create mode 100755 tests/generic/950
> > > create mode 100644 tests/generic/950.out
> > > create mode 100755 tests/generic/951
> > > create mode 100644 tests/generic/951.out
> > > create mode 100755 tests/generic/952
> > > create mode 100644 tests/generic/952.out
> > >
> > >
> > > diff --git a/tests/generic/950 b/tests/generic/950
> > > new file mode 100755
> > > index 00000000..84f1d1f0
> > > --- /dev/null
> > > +++ b/tests/generic/950
> > > @@ -0,0 +1,108 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > > +#
> > > +# FS QA Test 950
> > > +#
> > > +# Functional test for dropping suid and sgid bits as part of a reflink.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto clone quick
> > > +
> > > +# Import common functions.
> > > +. ./common/filter
> > > +. ./common/reflink
> > > +
> > > +# real QA test starts here
> > > +
> > > +# Modify as appropriate.
> > > +_supported_fs generic
> > > +_require_user
> > > +_require_scratch_reflink
> > > +
> > > +_scratch_mkfs >> $seqres.full
> > > +_scratch_mount
> > > +_require_congruent_file_oplen $SCRATCH_MNT 1048576
> >
> > Is this a Darrick's secret helper ? :)
>
> Oops, yes. Will resend.
>
> There's a big old patchset in djwong-dev that teaches all the reflink
> tests to _notrun if the file allocation unit is not congruent (i.e. an
> integer multiple or factor) with the operation sizes used in the test.
>
> I should probably move that up in the branch, but with Lunar New Year
> starting next week there probably isn't much point... ;)
Sure, Happy Lunar New Year ("新春快乐") in advance :)
>
> --D
>
> > Thanks,
> > Zorro
> >
> > > +chmod a+rw $SCRATCH_MNT/
> > > +
> > > +setup_testfile() {
> > > + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> > > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> > > + _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
> > > + chmod a+r $SCRATCH_MNT/b
> > > + sync
> > > +}
> > > +
> > > +commit_and_check() {
> > > + local user="$1"
> > > +
> > > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > > +
> > > + local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> > > + if [ -n "$user" ]; then
> > > + su - "$user" -c "$cmd" >> $seqres.full
> > > + else
> > > + $SHELL -c "$cmd" >> $seqres.full
> > > + fi
> > > +
> > > + _scratch_cycle_mount
> > > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > > +
> > > + # Blank line in output
> > > + echo
> > > +}
> > > +
> > > +# Commit to a non-exec file by an unprivileged user clears suid but leaves
> > > +# sgid.
> > > +echo "Test 1 - qa_user, non-exec file"
> > > +setup_testfile
> > > +chmod a+rws $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a group-exec file by an unprivileged user clears suid and sgid.
> > > +echo "Test 2 - qa_user, group-exec file"
> > > +setup_testfile
> > > +chmod g+x,a+rws $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
> > > +echo "Test 3 - qa_user, user-exec file"
> > > +setup_testfile
> > > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a all-exec file by an unprivileged user clears suid and sgid.
> > > +echo "Test 4 - qa_user, all-exec file"
> > > +setup_testfile
> > > +chmod a+rwxs $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a non-exec file by root clears suid but leaves sgid.
> > > +echo "Test 5 - root, non-exec file"
> > > +setup_testfile
> > > +chmod a+rws $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# Commit to a group-exec file by root clears suid and sgid.
> > > +echo "Test 6 - root, group-exec file"
> > > +setup_testfile
> > > +chmod g+x,a+rws $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# Commit to a user-exec file by root clears suid but not sgid.
> > > +echo "Test 7 - root, user-exec file"
> > > +setup_testfile
> > > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# Commit to a all-exec file by root clears suid and sgid.
> > > +echo "Test 8 - root, all-exec file"
> > > +setup_testfile
> > > +chmod a+rwxs $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# success, all done
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/950.out b/tests/generic/950.out
> > > new file mode 100644
> > > index 00000000..b42e4931
> > > --- /dev/null
> > > +++ b/tests/generic/950.out
> > > @@ -0,0 +1,49 @@
> > > +QA output created by 950
> > > +Test 1 - qa_user, non-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +2666 -rw-rwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 2 - qa_user, group-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +676 -rw-rwxrw- SCRATCH_MNT/a
> > > +
> > > +Test 3 - qa_user, user-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +2766 -rwxrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 4 - qa_user, all-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +777 -rwxrwxrwx SCRATCH_MNT/a
> > > +
> > > +Test 5 - root, non-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 6 - root, group-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +
> > > +Test 7 - root, user-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 8 - root, all-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +
> > > diff --git a/tests/generic/951 b/tests/generic/951
> > > new file mode 100755
> > > index 00000000..99f67ab7
> > > --- /dev/null
> > > +++ b/tests/generic/951
> > > @@ -0,0 +1,118 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > > +#
> > > +# FS QA Test 951
> > > +#
> > > +# Functional test for dropping suid and sgid bits as part of a deduplication.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto clone quick
> > > +
> > > +# Import common functions.
> > > +. ./common/filter
> > > +. ./common/reflink
> > > +
> > > +# real QA test starts here
> > > +
> > > +# Modify as appropriate.
> > > +_supported_fs generic
> > > +_require_user
> > > +_require_scratch_reflink
> > > +_require_xfs_io_command dedupe
> > > +
> > > +_scratch_mkfs >> $seqres.full
> > > +_scratch_mount
> > > +_require_congruent_file_oplen $SCRATCH_MNT 1048576
> > > +chmod a+rw $SCRATCH_MNT/
> > > +
> > > +setup_testfile() {
> > > + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> > > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> > > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/b >> $seqres.full
> > > + chmod a+r $SCRATCH_MNT/b
> > > + sync
> > > +}
> > > +
> > > +commit_and_check() {
> > > + local user="$1"
> > > +
> > > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > > +
> > > + local before_freesp=$(_get_available_space $SCRATCH_MNT)
> > > +
> > > + local cmd="$XFS_IO_PROG -c 'dedupe $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> > > + if [ -n "$user" ]; then
> > > + su - "$user" -c "$cmd" >> $seqres.full
> > > + else
> > > + $SHELL -c "$cmd" >> $seqres.full
> > > + fi
> > > +
> > > + _scratch_cycle_mount
> > > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > > +
> > > + local after_freesp=$(_get_available_space $SCRATCH_MNT)
> > > +
> > > + echo "before: $before_freesp; after: $after_freesp" >> $seqres.full
> > > + if [ $after_freesp -le $before_freesp ]; then
> > > + echo "expected more free space after dedupe"
> > > + fi
> > > +
> > > + # Blank line in output
> > > + echo
> > > +}
> > > +
> > > +# Commit to a non-exec file by an unprivileged user clears suid but leaves
> > > +# sgid.
> > > +echo "Test 1 - qa_user, non-exec file"
> > > +setup_testfile
> > > +chmod a+rws $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a group-exec file by an unprivileged user clears suid and sgid.
> > > +echo "Test 2 - qa_user, group-exec file"
> > > +setup_testfile
> > > +chmod g+x,a+rws $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
> > > +echo "Test 3 - qa_user, user-exec file"
> > > +setup_testfile
> > > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a all-exec file by an unprivileged user clears suid and sgid.
> > > +echo "Test 4 - qa_user, all-exec file"
> > > +setup_testfile
> > > +chmod a+rwxs $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a non-exec file by root clears suid but leaves sgid.
> > > +echo "Test 5 - root, non-exec file"
> > > +setup_testfile
> > > +chmod a+rws $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# Commit to a group-exec file by root clears suid and sgid.
> > > +echo "Test 6 - root, group-exec file"
> > > +setup_testfile
> > > +chmod g+x,a+rws $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# Commit to a user-exec file by root clears suid but not sgid.
> > > +echo "Test 7 - root, user-exec file"
> > > +setup_testfile
> > > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# Commit to a all-exec file by root clears suid and sgid.
> > > +echo "Test 8 - root, all-exec file"
> > > +setup_testfile
> > > +chmod a+rwxs $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# success, all done
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/951.out b/tests/generic/951.out
> > > new file mode 100644
> > > index 00000000..f7099ea2
> > > --- /dev/null
> > > +++ b/tests/generic/951.out
> > > @@ -0,0 +1,49 @@
> > > +QA output created by 951
> > > +Test 1 - qa_user, non-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 2 - qa_user, group-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +
> > > +Test 3 - qa_user, user-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 4 - qa_user, all-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +
> > > +Test 5 - root, non-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 6 - root, group-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +
> > > +Test 7 - root, user-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 8 - root, all-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +
> > > diff --git a/tests/generic/952 b/tests/generic/952
> > > new file mode 100755
> > > index 00000000..470d73bd
> > > --- /dev/null
> > > +++ b/tests/generic/952
> > > @@ -0,0 +1,71 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > > +#
> > > +# FS QA Test 952
> > > +#
> > > +# Functional test for dropping suid and sgid capabilities as part of a reflink.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto clone quick
> > > +
> > > +# Import common functions.
> > > +. ./common/filter
> > > +. ./common/reflink
> > > +
> > > +# real QA test starts here
> > > +
> > > +# Modify as appropriate.
> > > +_supported_fs generic
> > > +_require_user
> > > +_require_command "$GETCAP_PROG" getcap
> > > +_require_command "$SETCAP_PROG" setcap
> > > +_require_scratch_reflink
> > > +
> > > +_scratch_mkfs >> $seqres.full
> > > +_scratch_mount
> > > +_require_congruent_file_oplen $SCRATCH_MNT 1048576
> > > +chmod a+rw $SCRATCH_MNT/
> > > +
> > > +setup_testfile() {
> > > + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> > > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> > > + _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
> > > + chmod a+rwx $SCRATCH_MNT/a $SCRATCH_MNT/b
> > > + $SETCAP_PROG cap_setgid,cap_setuid+ep $SCRATCH_MNT/a
> > > + sync
> > > +}
> > > +
> > > +commit_and_check() {
> > > + local user="$1"
> > > +
> > > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > > + _getcap -v $SCRATCH_MNT/a | _filter_scratch
> > > +
> > > + local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> > > + if [ -n "$user" ]; then
> > > + su - "$user" -c "$cmd" >> $seqres.full
> > > + else
> > > + $SHELL -c "$cmd" >> $seqres.full
> > > + fi
> > > +
> > > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > > + _getcap -v $SCRATCH_MNT/a | _filter_scratch
> > > +
> > > + # Blank line in output
> > > + echo
> > > +}
> > > +
> > > +# Commit by an unprivileged user clears capability bits.
> > > +echo "Test 1 - qa_user"
> > > +setup_testfile
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit by root leaves capability bits.
> > > +echo "Test 2 - root"
> > > +setup_testfile
> > > +commit_and_check
> > > +
> > > +# success, all done
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/952.out b/tests/generic/952.out
> > > new file mode 100644
> > > index 00000000..eac9e76a
> > > --- /dev/null
> > > +++ b/tests/generic/952.out
> > > @@ -0,0 +1,13 @@
> > > +QA output created by 952
> > > +Test 1 - qa_user
> > > +777 -rwxrwxrwx SCRATCH_MNT/a
> > > +SCRATCH_MNT/a cap_setgid,cap_setuid=ep
> > > +777 -rwxrwxrwx SCRATCH_MNT/a
> > > +SCRATCH_MNT/a
> > > +
> > > +Test 2 - root
> > > +777 -rwxrwxrwx SCRATCH_MNT/a
> > > +SCRATCH_MNT/a cap_setgid,cap_setuid=ep
> > > +777 -rwxrwxrwx SCRATCH_MNT/a
> > > +SCRATCH_MNT/a
> > > +
> > >
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1.1 1/2] generic: test suid/sgid behavior with reflink and dedupe
2022-01-27 1:27 ` [PATCH v1.1 " Darrick J. Wong
@ 2022-02-16 15:09 ` Filipe Manana
2022-02-18 5:26 ` Darrick J. Wong
0 siblings, 1 reply; 11+ messages in thread
From: Filipe Manana @ 2022-02-16 15:09 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Eryu Guan, xfs, fstests, Eryu Guan, linux-btrfs
On Thu, Jan 27, 2022 at 9:01 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> From: Darrick J. Wong <djwong@kernel.org>
>
> Make sure that we drop the setuid and setgid bits any time reflink or
> dedupe change the file contents.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> v2: drop the congruent oplen checks, that was a mismerge
> ---
> tests/generic/950 | 107 +++++++++++++++++++++++++++++++++++++++++++++
> tests/generic/950.out | 49 +++++++++++++++++++++
> tests/generic/951 | 117 +++++++++++++++++++++++++++++++++++++++++++++++++
> tests/generic/951.out | 49 +++++++++++++++++++++
> tests/generic/952 | 70 +++++++++++++++++++++++++++++
> tests/generic/952.out | 13 +++++
> 6 files changed, 405 insertions(+)
> create mode 100755 tests/generic/950
> create mode 100644 tests/generic/950.out
> create mode 100755 tests/generic/951
> create mode 100644 tests/generic/951.out
> create mode 100755 tests/generic/952
> create mode 100644 tests/generic/952.out
>
> diff --git a/tests/generic/950 b/tests/generic/950
> new file mode 100755
> index 00000000..a7398cb5
> --- /dev/null
> +++ b/tests/generic/950
> @@ -0,0 +1,107 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 950
> +#
> +# Functional test for dropping suid and sgid bits as part of a reflink.
> +#
> +. ./common/preamble
> +_begin_fstest auto clone quick
> +
> +# Import common functions.
> +. ./common/filter
> +. ./common/reflink
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_require_user
> +_require_scratch_reflink
> +
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount
> +chmod a+rw $SCRATCH_MNT/
> +
> +setup_testfile() {
> + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> + _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
> + chmod a+r $SCRATCH_MNT/b
> + sync
> +}
> +
> +commit_and_check() {
> + local user="$1"
> +
> + md5sum $SCRATCH_MNT/a | _filter_scratch
> + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> +
> + local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> + if [ -n "$user" ]; then
> + su - "$user" -c "$cmd" >> $seqres.full
> + else
> + $SHELL -c "$cmd" >> $seqres.full
> + fi
> +
> + _scratch_cycle_mount
> + md5sum $SCRATCH_MNT/a | _filter_scratch
> + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> +
> + # Blank line in output
> + echo
> +}
> +
Hi Darrick,
> +# Commit to a non-exec file by an unprivileged user clears suid but leaves
> +# sgid.
> +echo "Test 1 - qa_user, non-exec file"
> +setup_testfile
> +chmod a+rws $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
So this test fails on btrfs, and after taking a look at it, I'm not
sure if this expected result is xfs specific or not.
On btrfs we get sgid cleared:
310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
6666 -rwSrwSrw- SCRATCH_MNT/a
3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
-2666 -rw-rwSrw- SCRATCH_MNT/a
+666 -rw-rw-rw- SCRATCH_MNT/a
This happens because at btrfs_setattr() we use setattr_copy() to
change the inode's ->i_mode.
I see that xfs does not use it, instead it manipulates ->i_mode
directly with xfs_setattr_mode().
At setattr_copy() we get the sgid removed:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/attr.c?h=v5.17-rc4#n241
Testing this with buffered writes instead of reflink, to see how xfs,
btrfs and ext4 behave:
#!/bin/bash
DEV=/dev/sdj
MNT=/mnt/sdj
XFS_IO=/home/fdmanana/xfsprogs/sbin/xfs_io
mkfs.btrfs -f $DEV >/dev/null
#mkfs.ext4 -F $DEV >/dev/null
#mkfs.xfs -f $DEV >/dev/null
mount $DEV $MNT
$XFS_IO -f -c "pwrite -S 0xab 0 128K" $MNT/foo >/dev/null
chmod a+rws $MNT/foo
echo "Inode mode before: $(stat -c '%a %A %n' $MNT/foo)"
cmd="$XFS_IO -c 'pwrite -S 0xcd 64K 4K' $MNT/foo"
su - fsgqa -c "$cmd" >/dev/null
echo "Inode mode after: $(stat -c '%a %A %n' $MNT/foo)"
umount $DEV
btrfs and ext4 give the same result, as both use the setattr_copy()
from generic code, where sgid gets removed:
Inode mode before: 6666 -rwSrwSrw- /mnt/sdj/foo
Inode mode after: 666 -rw-rw-rw- /mnt/sdj/foo
xfs gives the same behaviour as this test expects for reflinks, sgid
is preserved:
Inode mode before: 6666 -rwSrwSrw- /mnt/sdj/foo
Inode mode after: 2666 -rw-rwSrw- /mnt/sdj/foo
So I wonder if this is a behaviour that can be fs specific, if xfs is
behaving incorrectly, or the generic code, setattr_copy(), is not
correct.
Any thoughts?
Thanks.
> +
> +# Commit to a group-exec file by an unprivileged user clears suid and sgid.
> +echo "Test 2 - qa_user, group-exec file"
> +setup_testfile
> +chmod g+x,a+rws $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
> +echo "Test 3 - qa_user, user-exec file"
> +setup_testfile
> +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a all-exec file by an unprivileged user clears suid and sgid.
> +echo "Test 4 - qa_user, all-exec file"
> +setup_testfile
> +chmod a+rwxs $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a non-exec file by root clears suid but leaves sgid.
> +echo "Test 5 - root, non-exec file"
> +setup_testfile
> +chmod a+rws $SCRATCH_MNT/a
> +commit_and_check
> +
> +# Commit to a group-exec file by root clears suid and sgid.
> +echo "Test 6 - root, group-exec file"
> +setup_testfile
> +chmod g+x,a+rws $SCRATCH_MNT/a
> +commit_and_check
> +
> +# Commit to a user-exec file by root clears suid but not sgid.
> +echo "Test 7 - root, user-exec file"
> +setup_testfile
> +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> +commit_and_check
> +
> +# Commit to a all-exec file by root clears suid and sgid.
> +echo "Test 8 - root, all-exec file"
> +setup_testfile
> +chmod a+rwxs $SCRATCH_MNT/a
> +commit_and_check
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/950.out b/tests/generic/950.out
> new file mode 100644
> index 00000000..b42e4931
> --- /dev/null
> +++ b/tests/generic/950.out
> @@ -0,0 +1,49 @@
> +QA output created by 950
> +Test 1 - qa_user, non-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +2666 -rw-rwSrw- SCRATCH_MNT/a
> +
> +Test 2 - qa_user, group-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +676 -rw-rwxrw- SCRATCH_MNT/a
> +
> +Test 3 - qa_user, user-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +2766 -rwxrwSrw- SCRATCH_MNT/a
> +
> +Test 4 - qa_user, all-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +777 -rwxrwxrwx SCRATCH_MNT/a
> +
> +Test 5 - root, non-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +
> +Test 6 - root, group-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +
> +Test 7 - root, user-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +
> +Test 8 - root, all-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +
> diff --git a/tests/generic/951 b/tests/generic/951
> new file mode 100755
> index 00000000..8484f225
> --- /dev/null
> +++ b/tests/generic/951
> @@ -0,0 +1,117 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 951
> +#
> +# Functional test for dropping suid and sgid bits as part of a deduplication.
> +#
> +. ./common/preamble
> +_begin_fstest auto clone quick
> +
> +# Import common functions.
> +. ./common/filter
> +. ./common/reflink
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_require_user
> +_require_scratch_reflink
> +_require_xfs_io_command dedupe
> +
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount
> +chmod a+rw $SCRATCH_MNT/
> +
> +setup_testfile() {
> + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/b >> $seqres.full
> + chmod a+r $SCRATCH_MNT/b
> + sync
> +}
> +
> +commit_and_check() {
> + local user="$1"
> +
> + md5sum $SCRATCH_MNT/a | _filter_scratch
> + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> +
> + local before_freesp=$(_get_available_space $SCRATCH_MNT)
> +
> + local cmd="$XFS_IO_PROG -c 'dedupe $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> + if [ -n "$user" ]; then
> + su - "$user" -c "$cmd" >> $seqres.full
> + else
> + $SHELL -c "$cmd" >> $seqres.full
> + fi
> +
> + _scratch_cycle_mount
> + md5sum $SCRATCH_MNT/a | _filter_scratch
> + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> +
> + local after_freesp=$(_get_available_space $SCRATCH_MNT)
> +
> + echo "before: $before_freesp; after: $after_freesp" >> $seqres.full
> + if [ $after_freesp -le $before_freesp ]; then
> + echo "expected more free space after dedupe"
> + fi
> +
> + # Blank line in output
> + echo
> +}
> +
> +# Commit to a non-exec file by an unprivileged user clears suid but leaves
> +# sgid.
> +echo "Test 1 - qa_user, non-exec file"
> +setup_testfile
> +chmod a+rws $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a group-exec file by an unprivileged user clears suid and sgid.
> +echo "Test 2 - qa_user, group-exec file"
> +setup_testfile
> +chmod g+x,a+rws $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
> +echo "Test 3 - qa_user, user-exec file"
> +setup_testfile
> +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a all-exec file by an unprivileged user clears suid and sgid.
> +echo "Test 4 - qa_user, all-exec file"
> +setup_testfile
> +chmod a+rwxs $SCRATCH_MNT/a
> +commit_and_check "$qa_user"
> +
> +# Commit to a non-exec file by root clears suid but leaves sgid.
> +echo "Test 5 - root, non-exec file"
> +setup_testfile
> +chmod a+rws $SCRATCH_MNT/a
> +commit_and_check
> +
> +# Commit to a group-exec file by root clears suid and sgid.
> +echo "Test 6 - root, group-exec file"
> +setup_testfile
> +chmod g+x,a+rws $SCRATCH_MNT/a
> +commit_and_check
> +
> +# Commit to a user-exec file by root clears suid but not sgid.
> +echo "Test 7 - root, user-exec file"
> +setup_testfile
> +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> +commit_and_check
> +
> +# Commit to a all-exec file by root clears suid and sgid.
> +echo "Test 8 - root, all-exec file"
> +setup_testfile
> +chmod a+rwxs $SCRATCH_MNT/a
> +commit_and_check
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/951.out b/tests/generic/951.out
> new file mode 100644
> index 00000000..f7099ea2
> --- /dev/null
> +++ b/tests/generic/951.out
> @@ -0,0 +1,49 @@
> +QA output created by 951
> +Test 1 - qa_user, non-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +
> +Test 2 - qa_user, group-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +
> +Test 3 - qa_user, user-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +
> +Test 4 - qa_user, all-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +
> +Test 5 - root, non-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6666 -rwSrwSrw- SCRATCH_MNT/a
> +
> +Test 6 - root, group-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6676 -rwSrwsrw- SCRATCH_MNT/a
> +
> +Test 7 - root, user-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6766 -rwsrwSrw- SCRATCH_MNT/a
> +
> +Test 8 - root, all-exec file
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> +6777 -rwsrwsrwx SCRATCH_MNT/a
> +
> diff --git a/tests/generic/952 b/tests/generic/952
> new file mode 100755
> index 00000000..86443dcc
> --- /dev/null
> +++ b/tests/generic/952
> @@ -0,0 +1,70 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 952
> +#
> +# Functional test for dropping suid and sgid capabilities as part of a reflink.
> +#
> +. ./common/preamble
> +_begin_fstest auto clone quick
> +
> +# Import common functions.
> +. ./common/filter
> +. ./common/reflink
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_require_user
> +_require_command "$GETCAP_PROG" getcap
> +_require_command "$SETCAP_PROG" setcap
> +_require_scratch_reflink
> +
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount
> +chmod a+rw $SCRATCH_MNT/
> +
> +setup_testfile() {
> + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> + _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
> + chmod a+rwx $SCRATCH_MNT/a $SCRATCH_MNT/b
> + $SETCAP_PROG cap_setgid,cap_setuid+ep $SCRATCH_MNT/a
> + sync
> +}
> +
> +commit_and_check() {
> + local user="$1"
> +
> + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> + _getcap -v $SCRATCH_MNT/a | _filter_scratch
> +
> + local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> + if [ -n "$user" ]; then
> + su - "$user" -c "$cmd" >> $seqres.full
> + else
> + $SHELL -c "$cmd" >> $seqres.full
> + fi
> +
> + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> + _getcap -v $SCRATCH_MNT/a | _filter_scratch
> +
> + # Blank line in output
> + echo
> +}
> +
> +# Commit by an unprivileged user clears capability bits.
> +echo "Test 1 - qa_user"
> +setup_testfile
> +commit_and_check "$qa_user"
> +
> +# Commit by root leaves capability bits.
> +echo "Test 2 - root"
> +setup_testfile
> +commit_and_check
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/952.out b/tests/generic/952.out
> new file mode 100644
> index 00000000..eac9e76a
> --- /dev/null
> +++ b/tests/generic/952.out
> @@ -0,0 +1,13 @@
> +QA output created by 952
> +Test 1 - qa_user
> +777 -rwxrwxrwx SCRATCH_MNT/a
> +SCRATCH_MNT/a cap_setgid,cap_setuid=ep
> +777 -rwxrwxrwx SCRATCH_MNT/a
> +SCRATCH_MNT/a
> +
> +Test 2 - root
> +777 -rwxrwxrwx SCRATCH_MNT/a
> +SCRATCH_MNT/a cap_setgid,cap_setuid=ep
> +777 -rwxrwxrwx SCRATCH_MNT/a
> +SCRATCH_MNT/a
> +
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1.1 1/2] generic: test suid/sgid behavior with reflink and dedupe
2022-02-16 15:09 ` Filipe Manana
@ 2022-02-18 5:26 ` Darrick J. Wong
2022-03-10 17:48 ` Darrick J. Wong
0 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2022-02-18 5:26 UTC (permalink / raw)
To: Filipe Manana; +Cc: Eryu Guan, xfs, fstests, Eryu Guan, linux-btrfs
On Wed, Feb 16, 2022 at 03:09:20PM +0000, Filipe Manana wrote:
> On Thu, Jan 27, 2022 at 9:01 AM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > Make sure that we drop the setuid and setgid bits any time reflink or
> > dedupe change the file contents.
> >
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> > v2: drop the congruent oplen checks, that was a mismerge
> > ---
> > tests/generic/950 | 107 +++++++++++++++++++++++++++++++++++++++++++++
> > tests/generic/950.out | 49 +++++++++++++++++++++
> > tests/generic/951 | 117 +++++++++++++++++++++++++++++++++++++++++++++++++
> > tests/generic/951.out | 49 +++++++++++++++++++++
> > tests/generic/952 | 70 +++++++++++++++++++++++++++++
> > tests/generic/952.out | 13 +++++
> > 6 files changed, 405 insertions(+)
> > create mode 100755 tests/generic/950
> > create mode 100644 tests/generic/950.out
> > create mode 100755 tests/generic/951
> > create mode 100644 tests/generic/951.out
> > create mode 100755 tests/generic/952
> > create mode 100644 tests/generic/952.out
> >
> > diff --git a/tests/generic/950 b/tests/generic/950
> > new file mode 100755
> > index 00000000..a7398cb5
> > --- /dev/null
> > +++ b/tests/generic/950
> > @@ -0,0 +1,107 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > +#
> > +# FS QA Test 950
> > +#
> > +# Functional test for dropping suid and sgid bits as part of a reflink.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto clone quick
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +. ./common/reflink
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_require_user
> > +_require_scratch_reflink
> > +
> > +_scratch_mkfs >> $seqres.full
> > +_scratch_mount
> > +chmod a+rw $SCRATCH_MNT/
> > +
> > +setup_testfile() {
> > + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> > + _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
> > + chmod a+r $SCRATCH_MNT/b
> > + sync
> > +}
> > +
> > +commit_and_check() {
> > + local user="$1"
> > +
> > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > +
> > + local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> > + if [ -n "$user" ]; then
> > + su - "$user" -c "$cmd" >> $seqres.full
> > + else
> > + $SHELL -c "$cmd" >> $seqres.full
> > + fi
> > +
> > + _scratch_cycle_mount
> > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > +
> > + # Blank line in output
> > + echo
> > +}
> > +
>
> Hi Darrick,
>
> > +# Commit to a non-exec file by an unprivileged user clears suid but leaves
> > +# sgid.
> > +echo "Test 1 - qa_user, non-exec file"
> > +setup_testfile
> > +chmod a+rws $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
>
> So this test fails on btrfs, and after taking a look at it, I'm not
> sure if this expected result is xfs specific or not.
It's.... complicated. g+s without g+x is some holdover from System V
mandatory file locks, wherein:
"Note that the group-id bit is usually automatically cleared by the
kernel when a setgid file is written to. This is a security measure. The
kernel has been modified to recognize the special case of a mandatory
lock candidate and to refrain from clearing this bit. Similarly the
kernel has been modified not to run mandatory lock candidates with
setgid privileges."
https://www.kernel.org/doc/html/v5.14/filesystems/mandatory-locking.html#marking-a-file-for-mandatory-locking
Which, I think, is why XFS preserves the setgid bit. Of course, with
mandatory locking now deprecated, it's perhaps time to get rid of that
behavior?
The funny part is, XFS itself doesn't make this decision;
should_remove_suid in the VFS does:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/inode.c?h=v5.17-rc4#n1967
So XFS has this behavior because generic_remap_file_range_prep calls
file_modified -> file_remove_privs -> dentry_needs_remove_privs ->
should_remove_suid...
> On btrfs we get sgid cleared:
>
> 310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> 6666 -rwSrwSrw- SCRATCH_MNT/a
> 3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> -2666 -rw-rwSrw- SCRATCH_MNT/a
> +666 -rw-rw-rw- SCRATCH_MNT/a
>
> This happens because at btrfs_setattr() we use setattr_copy() to
> change the inode's ->i_mode.
> I see that xfs does not use it, instead it manipulates ->i_mode
> directly with xfs_setattr_mode().
>
> At setattr_copy() we get the sgid removed:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/attr.c?h=v5.17-rc4#n241
...OH, now I get it. notify_change() sees that ATTR_KILL_SUID is set,
and sets ATTR_MODE. If you use setattr_copy, will always clear the
setgid bit. XFS doesn't (as you point out) which is why the behavior is
different. xfs_setattr_nonsize looks like it open-codes a lot of what
setattr_copy does, but without the gid-killing quirk.
I'm not sure what to do here -- the VFS code is clearly conflicted about
this. Given that the weird behavior only exists to support a dying
feature, we might as well just make everyone clear sgid.
Digging around in xfs_setattr_nonsize a bit more, I suspect the reason
why we handle ATTR_[UG]ID separately is because of the xfs quota
subsystem, but I don't see any reason why xfs couldn't adopt
setattr_copy for the rest of the ATTR_* bits.
Sorry about injecting a new fail test at you all. :/
> Testing this with buffered writes instead of reflink, to see how xfs,
> btrfs and ext4 behave:
>
> #!/bin/bash
>
> DEV=/dev/sdj
> MNT=/mnt/sdj
> XFS_IO=/home/fdmanana/xfsprogs/sbin/xfs_io
>
> mkfs.btrfs -f $DEV >/dev/null
> #mkfs.ext4 -F $DEV >/dev/null
> #mkfs.xfs -f $DEV >/dev/null
> mount $DEV $MNT
>
> $XFS_IO -f -c "pwrite -S 0xab 0 128K" $MNT/foo >/dev/null
> chmod a+rws $MNT/foo
>
> echo "Inode mode before: $(stat -c '%a %A %n' $MNT/foo)"
>
> cmd="$XFS_IO -c 'pwrite -S 0xcd 64K 4K' $MNT/foo"
> su - fsgqa -c "$cmd" >/dev/null
>
> echo "Inode mode after: $(stat -c '%a %A %n' $MNT/foo)"
>
> umount $DEV
>
> btrfs and ext4 give the same result, as both use the setattr_copy()
> from generic code, where sgid gets removed:
>
> Inode mode before: 6666 -rwSrwSrw- /mnt/sdj/foo
> Inode mode after: 666 -rw-rw-rw- /mnt/sdj/foo
>
> xfs gives the same behaviour as this test expects for reflinks, sgid
> is preserved:
>
> Inode mode before: 6666 -rwSrwSrw- /mnt/sdj/foo
> Inode mode after: 2666 -rw-rwSrw- /mnt/sdj/foo
>
> So I wonder if this is a behaviour that can be fs specific, if xfs is
> behaving incorrectly, or the generic code, setattr_copy(), is not
> correct.
> Any thoughts?
"Ow, my brain hurts." and "I wonder if Dave has better context about
this"...
--D
>
> Thanks.
>
>
>
>
>
> > +
> > +# Commit to a group-exec file by an unprivileged user clears suid and sgid.
> > +echo "Test 2 - qa_user, group-exec file"
> > +setup_testfile
> > +chmod g+x,a+rws $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
> > +echo "Test 3 - qa_user, user-exec file"
> > +setup_testfile
> > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a all-exec file by an unprivileged user clears suid and sgid.
> > +echo "Test 4 - qa_user, all-exec file"
> > +setup_testfile
> > +chmod a+rwxs $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a non-exec file by root clears suid but leaves sgid.
> > +echo "Test 5 - root, non-exec file"
> > +setup_testfile
> > +chmod a+rws $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# Commit to a group-exec file by root clears suid and sgid.
> > +echo "Test 6 - root, group-exec file"
> > +setup_testfile
> > +chmod g+x,a+rws $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# Commit to a user-exec file by root clears suid but not sgid.
> > +echo "Test 7 - root, user-exec file"
> > +setup_testfile
> > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# Commit to a all-exec file by root clears suid and sgid.
> > +echo "Test 8 - root, all-exec file"
> > +setup_testfile
> > +chmod a+rwxs $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/generic/950.out b/tests/generic/950.out
> > new file mode 100644
> > index 00000000..b42e4931
> > --- /dev/null
> > +++ b/tests/generic/950.out
> > @@ -0,0 +1,49 @@
> > +QA output created by 950
> > +Test 1 - qa_user, non-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +2666 -rw-rwSrw- SCRATCH_MNT/a
> > +
> > +Test 2 - qa_user, group-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +676 -rw-rwxrw- SCRATCH_MNT/a
> > +
> > +Test 3 - qa_user, user-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +2766 -rwxrwSrw- SCRATCH_MNT/a
> > +
> > +Test 4 - qa_user, all-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +777 -rwxrwxrwx SCRATCH_MNT/a
> > +
> > +Test 5 - root, non-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +
> > +Test 6 - root, group-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +
> > +Test 7 - root, user-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +
> > +Test 8 - root, all-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +
> > diff --git a/tests/generic/951 b/tests/generic/951
> > new file mode 100755
> > index 00000000..8484f225
> > --- /dev/null
> > +++ b/tests/generic/951
> > @@ -0,0 +1,117 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > +#
> > +# FS QA Test 951
> > +#
> > +# Functional test for dropping suid and sgid bits as part of a deduplication.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto clone quick
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +. ./common/reflink
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_require_user
> > +_require_scratch_reflink
> > +_require_xfs_io_command dedupe
> > +
> > +_scratch_mkfs >> $seqres.full
> > +_scratch_mount
> > +chmod a+rw $SCRATCH_MNT/
> > +
> > +setup_testfile() {
> > + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/b >> $seqres.full
> > + chmod a+r $SCRATCH_MNT/b
> > + sync
> > +}
> > +
> > +commit_and_check() {
> > + local user="$1"
> > +
> > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > +
> > + local before_freesp=$(_get_available_space $SCRATCH_MNT)
> > +
> > + local cmd="$XFS_IO_PROG -c 'dedupe $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> > + if [ -n "$user" ]; then
> > + su - "$user" -c "$cmd" >> $seqres.full
> > + else
> > + $SHELL -c "$cmd" >> $seqres.full
> > + fi
> > +
> > + _scratch_cycle_mount
> > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > +
> > + local after_freesp=$(_get_available_space $SCRATCH_MNT)
> > +
> > + echo "before: $before_freesp; after: $after_freesp" >> $seqres.full
> > + if [ $after_freesp -le $before_freesp ]; then
> > + echo "expected more free space after dedupe"
> > + fi
> > +
> > + # Blank line in output
> > + echo
> > +}
> > +
> > +# Commit to a non-exec file by an unprivileged user clears suid but leaves
> > +# sgid.
> > +echo "Test 1 - qa_user, non-exec file"
> > +setup_testfile
> > +chmod a+rws $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a group-exec file by an unprivileged user clears suid and sgid.
> > +echo "Test 2 - qa_user, group-exec file"
> > +setup_testfile
> > +chmod g+x,a+rws $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
> > +echo "Test 3 - qa_user, user-exec file"
> > +setup_testfile
> > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a all-exec file by an unprivileged user clears suid and sgid.
> > +echo "Test 4 - qa_user, all-exec file"
> > +setup_testfile
> > +chmod a+rwxs $SCRATCH_MNT/a
> > +commit_and_check "$qa_user"
> > +
> > +# Commit to a non-exec file by root clears suid but leaves sgid.
> > +echo "Test 5 - root, non-exec file"
> > +setup_testfile
> > +chmod a+rws $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# Commit to a group-exec file by root clears suid and sgid.
> > +echo "Test 6 - root, group-exec file"
> > +setup_testfile
> > +chmod g+x,a+rws $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# Commit to a user-exec file by root clears suid but not sgid.
> > +echo "Test 7 - root, user-exec file"
> > +setup_testfile
> > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# Commit to a all-exec file by root clears suid and sgid.
> > +echo "Test 8 - root, all-exec file"
> > +setup_testfile
> > +chmod a+rwxs $SCRATCH_MNT/a
> > +commit_and_check
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/generic/951.out b/tests/generic/951.out
> > new file mode 100644
> > index 00000000..f7099ea2
> > --- /dev/null
> > +++ b/tests/generic/951.out
> > @@ -0,0 +1,49 @@
> > +QA output created by 951
> > +Test 1 - qa_user, non-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +
> > +Test 2 - qa_user, group-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +
> > +Test 3 - qa_user, user-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +
> > +Test 4 - qa_user, all-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +
> > +Test 5 - root, non-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > +
> > +Test 6 - root, group-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > +
> > +Test 7 - root, user-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > +
> > +Test 8 - root, all-exec file
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > +
> > diff --git a/tests/generic/952 b/tests/generic/952
> > new file mode 100755
> > index 00000000..86443dcc
> > --- /dev/null
> > +++ b/tests/generic/952
> > @@ -0,0 +1,70 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > +#
> > +# FS QA Test 952
> > +#
> > +# Functional test for dropping suid and sgid capabilities as part of a reflink.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto clone quick
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +. ./common/reflink
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_require_user
> > +_require_command "$GETCAP_PROG" getcap
> > +_require_command "$SETCAP_PROG" setcap
> > +_require_scratch_reflink
> > +
> > +_scratch_mkfs >> $seqres.full
> > +_scratch_mount
> > +chmod a+rw $SCRATCH_MNT/
> > +
> > +setup_testfile() {
> > + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> > + _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
> > + chmod a+rwx $SCRATCH_MNT/a $SCRATCH_MNT/b
> > + $SETCAP_PROG cap_setgid,cap_setuid+ep $SCRATCH_MNT/a
> > + sync
> > +}
> > +
> > +commit_and_check() {
> > + local user="$1"
> > +
> > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > + _getcap -v $SCRATCH_MNT/a | _filter_scratch
> > +
> > + local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> > + if [ -n "$user" ]; then
> > + su - "$user" -c "$cmd" >> $seqres.full
> > + else
> > + $SHELL -c "$cmd" >> $seqres.full
> > + fi
> > +
> > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > + _getcap -v $SCRATCH_MNT/a | _filter_scratch
> > +
> > + # Blank line in output
> > + echo
> > +}
> > +
> > +# Commit by an unprivileged user clears capability bits.
> > +echo "Test 1 - qa_user"
> > +setup_testfile
> > +commit_and_check "$qa_user"
> > +
> > +# Commit by root leaves capability bits.
> > +echo "Test 2 - root"
> > +setup_testfile
> > +commit_and_check
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/generic/952.out b/tests/generic/952.out
> > new file mode 100644
> > index 00000000..eac9e76a
> > --- /dev/null
> > +++ b/tests/generic/952.out
> > @@ -0,0 +1,13 @@
> > +QA output created by 952
> > +Test 1 - qa_user
> > +777 -rwxrwxrwx SCRATCH_MNT/a
> > +SCRATCH_MNT/a cap_setgid,cap_setuid=ep
> > +777 -rwxrwxrwx SCRATCH_MNT/a
> > +SCRATCH_MNT/a
> > +
> > +Test 2 - root
> > +777 -rwxrwxrwx SCRATCH_MNT/a
> > +SCRATCH_MNT/a cap_setgid,cap_setuid=ep
> > +777 -rwxrwxrwx SCRATCH_MNT/a
> > +SCRATCH_MNT/a
> > +
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1.1 1/2] generic: test suid/sgid behavior with reflink and dedupe
2022-02-18 5:26 ` Darrick J. Wong
@ 2022-03-10 17:48 ` Darrick J. Wong
0 siblings, 0 replies; 11+ messages in thread
From: Darrick J. Wong @ 2022-03-10 17:48 UTC (permalink / raw)
To: Filipe Manana; +Cc: Eryu Guan, xfs, fstests, Eryu Guan, linux-btrfs
On Thu, Feb 17, 2022 at 09:26:15PM -0800, Darrick J. Wong wrote:
> On Wed, Feb 16, 2022 at 03:09:20PM +0000, Filipe Manana wrote:
> > On Thu, Jan 27, 2022 at 9:01 AM Darrick J. Wong <djwong@kernel.org> wrote:
> > >
> > > From: Darrick J. Wong <djwong@kernel.org>
> > >
> > > Make sure that we drop the setuid and setgid bits any time reflink or
> > > dedupe change the file contents.
> > >
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > > v2: drop the congruent oplen checks, that was a mismerge
> > > ---
> > > tests/generic/950 | 107 +++++++++++++++++++++++++++++++++++++++++++++
> > > tests/generic/950.out | 49 +++++++++++++++++++++
> > > tests/generic/951 | 117 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > tests/generic/951.out | 49 +++++++++++++++++++++
> > > tests/generic/952 | 70 +++++++++++++++++++++++++++++
> > > tests/generic/952.out | 13 +++++
> > > 6 files changed, 405 insertions(+)
> > > create mode 100755 tests/generic/950
> > > create mode 100644 tests/generic/950.out
> > > create mode 100755 tests/generic/951
> > > create mode 100644 tests/generic/951.out
> > > create mode 100755 tests/generic/952
> > > create mode 100644 tests/generic/952.out
> > >
> > > diff --git a/tests/generic/950 b/tests/generic/950
> > > new file mode 100755
> > > index 00000000..a7398cb5
> > > --- /dev/null
> > > +++ b/tests/generic/950
> > > @@ -0,0 +1,107 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > > +#
> > > +# FS QA Test 950
> > > +#
> > > +# Functional test for dropping suid and sgid bits as part of a reflink.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto clone quick
> > > +
> > > +# Import common functions.
> > > +. ./common/filter
> > > +. ./common/reflink
> > > +
> > > +# real QA test starts here
> > > +
> > > +# Modify as appropriate.
> > > +_supported_fs generic
> > > +_require_user
> > > +_require_scratch_reflink
> > > +
> > > +_scratch_mkfs >> $seqres.full
> > > +_scratch_mount
> > > +chmod a+rw $SCRATCH_MNT/
> > > +
> > > +setup_testfile() {
> > > + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> > > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> > > + _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
> > > + chmod a+r $SCRATCH_MNT/b
> > > + sync
> > > +}
> > > +
> > > +commit_and_check() {
> > > + local user="$1"
> > > +
> > > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > > +
> > > + local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> > > + if [ -n "$user" ]; then
> > > + su - "$user" -c "$cmd" >> $seqres.full
> > > + else
> > > + $SHELL -c "$cmd" >> $seqres.full
> > > + fi
> > > +
> > > + _scratch_cycle_mount
> > > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > > +
> > > + # Blank line in output
> > > + echo
> > > +}
> > > +
> >
> > Hi Darrick,
> >
> > > +# Commit to a non-exec file by an unprivileged user clears suid but leaves
> > > +# sgid.
> > > +echo "Test 1 - qa_user, non-exec file"
> > > +setup_testfile
> > > +chmod a+rws $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> >
> > So this test fails on btrfs, and after taking a look at it, I'm not
> > sure if this expected result is xfs specific or not.
>
> It's.... complicated. g+s without g+x is some holdover from System V
> mandatory file locks, wherein:
>
> "Note that the group-id bit is usually automatically cleared by the
> kernel when a setgid file is written to. This is a security measure. The
> kernel has been modified to recognize the special case of a mandatory
> lock candidate and to refrain from clearing this bit. Similarly the
> kernel has been modified not to run mandatory lock candidates with
> setgid privileges."
>
> https://www.kernel.org/doc/html/v5.14/filesystems/mandatory-locking.html#marking-a-file-for-mandatory-locking
>
> Which, I think, is why XFS preserves the setgid bit. Of course, with
> mandatory locking now deprecated, it's perhaps time to get rid of that
> behavior?
>
> The funny part is, XFS itself doesn't make this decision;
> should_remove_suid in the VFS does:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/inode.c?h=v5.17-rc4#n1967
>
> So XFS has this behavior because generic_remap_file_range_prep calls
> file_modified -> file_remove_privs -> dentry_needs_remove_privs ->
> should_remove_suid...
>
> > On btrfs we get sgid cleared:
> >
> > 310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > 6666 -rwSrwSrw- SCRATCH_MNT/a
> > 3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > -2666 -rw-rwSrw- SCRATCH_MNT/a
> > +666 -rw-rw-rw- SCRATCH_MNT/a
> >
> > This happens because at btrfs_setattr() we use setattr_copy() to
> > change the inode's ->i_mode.
> > I see that xfs does not use it, instead it manipulates ->i_mode
> > directly with xfs_setattr_mode().
> >
> > At setattr_copy() we get the sgid removed:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/attr.c?h=v5.17-rc4#n241
>
> ...OH, now I get it. notify_change() sees that ATTR_KILL_SUID is set,
> and sets ATTR_MODE. If you use setattr_copy, will always clear the
> setgid bit. XFS doesn't (as you point out) which is why the behavior is
> different. xfs_setattr_nonsize looks like it open-codes a lot of what
> setattr_copy does, but without the gid-killing quirk.
>
> I'm not sure what to do here -- the VFS code is clearly conflicted about
> this. Given that the weird behavior only exists to support a dying
> feature, we might as well just make everyone clear sgid.
>
> Digging around in xfs_setattr_nonsize a bit more, I suspect the reason
> why we handle ATTR_[UG]ID separately is because of the xfs quota
> subsystem, but I don't see any reason why xfs couldn't adopt
> setattr_copy for the rest of the ATTR_* bits.
For anyone still following this thread: I ported XFS to setattr_copy and
will send a patch to Eryu fixing generic/673:
https://lore.kernel.org/linux-xfs/8aa8a6d8-109e-99dd-d904-268df6058447@virtuozzo.com/T/#t
Apparently XFS has simply been out of date since 7fa294c8991c ("userns:
Allow chown and setgid preservation")
--D
> Sorry about injecting a new fail test at you all. :/
>
> > Testing this with buffered writes instead of reflink, to see how xfs,
> > btrfs and ext4 behave:
> >
> > #!/bin/bash
> >
> > DEV=/dev/sdj
> > MNT=/mnt/sdj
> > XFS_IO=/home/fdmanana/xfsprogs/sbin/xfs_io
> >
> > mkfs.btrfs -f $DEV >/dev/null
> > #mkfs.ext4 -F $DEV >/dev/null
> > #mkfs.xfs -f $DEV >/dev/null
> > mount $DEV $MNT
> >
> > $XFS_IO -f -c "pwrite -S 0xab 0 128K" $MNT/foo >/dev/null
> > chmod a+rws $MNT/foo
> >
> > echo "Inode mode before: $(stat -c '%a %A %n' $MNT/foo)"
> >
> > cmd="$XFS_IO -c 'pwrite -S 0xcd 64K 4K' $MNT/foo"
> > su - fsgqa -c "$cmd" >/dev/null
> >
> > echo "Inode mode after: $(stat -c '%a %A %n' $MNT/foo)"
> >
> > umount $DEV
> >
> > btrfs and ext4 give the same result, as both use the setattr_copy()
> > from generic code, where sgid gets removed:
> >
> > Inode mode before: 6666 -rwSrwSrw- /mnt/sdj/foo
> > Inode mode after: 666 -rw-rw-rw- /mnt/sdj/foo
> >
> > xfs gives the same behaviour as this test expects for reflinks, sgid
> > is preserved:
> >
> > Inode mode before: 6666 -rwSrwSrw- /mnt/sdj/foo
> > Inode mode after: 2666 -rw-rwSrw- /mnt/sdj/foo
> >
> > So I wonder if this is a behaviour that can be fs specific, if xfs is
> > behaving incorrectly, or the generic code, setattr_copy(), is not
> > correct.
> > Any thoughts?
>
> "Ow, my brain hurts." and "I wonder if Dave has better context about
> this"...
>
> --D
>
> >
> > Thanks.
> >
> >
> >
> >
> >
> > > +
> > > +# Commit to a group-exec file by an unprivileged user clears suid and sgid.
> > > +echo "Test 2 - qa_user, group-exec file"
> > > +setup_testfile
> > > +chmod g+x,a+rws $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
> > > +echo "Test 3 - qa_user, user-exec file"
> > > +setup_testfile
> > > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a all-exec file by an unprivileged user clears suid and sgid.
> > > +echo "Test 4 - qa_user, all-exec file"
> > > +setup_testfile
> > > +chmod a+rwxs $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a non-exec file by root clears suid but leaves sgid.
> > > +echo "Test 5 - root, non-exec file"
> > > +setup_testfile
> > > +chmod a+rws $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# Commit to a group-exec file by root clears suid and sgid.
> > > +echo "Test 6 - root, group-exec file"
> > > +setup_testfile
> > > +chmod g+x,a+rws $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# Commit to a user-exec file by root clears suid but not sgid.
> > > +echo "Test 7 - root, user-exec file"
> > > +setup_testfile
> > > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# Commit to a all-exec file by root clears suid and sgid.
> > > +echo "Test 8 - root, all-exec file"
> > > +setup_testfile
> > > +chmod a+rwxs $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# success, all done
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/950.out b/tests/generic/950.out
> > > new file mode 100644
> > > index 00000000..b42e4931
> > > --- /dev/null
> > > +++ b/tests/generic/950.out
> > > @@ -0,0 +1,49 @@
> > > +QA output created by 950
> > > +Test 1 - qa_user, non-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +2666 -rw-rwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 2 - qa_user, group-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +676 -rw-rwxrw- SCRATCH_MNT/a
> > > +
> > > +Test 3 - qa_user, user-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +2766 -rwxrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 4 - qa_user, all-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +777 -rwxrwxrwx SCRATCH_MNT/a
> > > +
> > > +Test 5 - root, non-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 6 - root, group-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +
> > > +Test 7 - root, user-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 8 - root, all-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +3784de23efab7a2074c9ec66901e39e5 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +
> > > diff --git a/tests/generic/951 b/tests/generic/951
> > > new file mode 100755
> > > index 00000000..8484f225
> > > --- /dev/null
> > > +++ b/tests/generic/951
> > > @@ -0,0 +1,117 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > > +#
> > > +# FS QA Test 951
> > > +#
> > > +# Functional test for dropping suid and sgid bits as part of a deduplication.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto clone quick
> > > +
> > > +# Import common functions.
> > > +. ./common/filter
> > > +. ./common/reflink
> > > +
> > > +# real QA test starts here
> > > +
> > > +# Modify as appropriate.
> > > +_supported_fs generic
> > > +_require_user
> > > +_require_scratch_reflink
> > > +_require_xfs_io_command dedupe
> > > +
> > > +_scratch_mkfs >> $seqres.full
> > > +_scratch_mount
> > > +chmod a+rw $SCRATCH_MNT/
> > > +
> > > +setup_testfile() {
> > > + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> > > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> > > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/b >> $seqres.full
> > > + chmod a+r $SCRATCH_MNT/b
> > > + sync
> > > +}
> > > +
> > > +commit_and_check() {
> > > + local user="$1"
> > > +
> > > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > > +
> > > + local before_freesp=$(_get_available_space $SCRATCH_MNT)
> > > +
> > > + local cmd="$XFS_IO_PROG -c 'dedupe $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> > > + if [ -n "$user" ]; then
> > > + su - "$user" -c "$cmd" >> $seqres.full
> > > + else
> > > + $SHELL -c "$cmd" >> $seqres.full
> > > + fi
> > > +
> > > + _scratch_cycle_mount
> > > + md5sum $SCRATCH_MNT/a | _filter_scratch
> > > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > > +
> > > + local after_freesp=$(_get_available_space $SCRATCH_MNT)
> > > +
> > > + echo "before: $before_freesp; after: $after_freesp" >> $seqres.full
> > > + if [ $after_freesp -le $before_freesp ]; then
> > > + echo "expected more free space after dedupe"
> > > + fi
> > > +
> > > + # Blank line in output
> > > + echo
> > > +}
> > > +
> > > +# Commit to a non-exec file by an unprivileged user clears suid but leaves
> > > +# sgid.
> > > +echo "Test 1 - qa_user, non-exec file"
> > > +setup_testfile
> > > +chmod a+rws $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a group-exec file by an unprivileged user clears suid and sgid.
> > > +echo "Test 2 - qa_user, group-exec file"
> > > +setup_testfile
> > > +chmod g+x,a+rws $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a user-exec file by an unprivileged user clears suid but not sgid.
> > > +echo "Test 3 - qa_user, user-exec file"
> > > +setup_testfile
> > > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a all-exec file by an unprivileged user clears suid and sgid.
> > > +echo "Test 4 - qa_user, all-exec file"
> > > +setup_testfile
> > > +chmod a+rwxs $SCRATCH_MNT/a
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit to a non-exec file by root clears suid but leaves sgid.
> > > +echo "Test 5 - root, non-exec file"
> > > +setup_testfile
> > > +chmod a+rws $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# Commit to a group-exec file by root clears suid and sgid.
> > > +echo "Test 6 - root, group-exec file"
> > > +setup_testfile
> > > +chmod g+x,a+rws $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# Commit to a user-exec file by root clears suid but not sgid.
> > > +echo "Test 7 - root, user-exec file"
> > > +setup_testfile
> > > +chmod u+x,a+rws,g-x $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# Commit to a all-exec file by root clears suid and sgid.
> > > +echo "Test 8 - root, all-exec file"
> > > +setup_testfile
> > > +chmod a+rwxs $SCRATCH_MNT/a
> > > +commit_and_check
> > > +
> > > +# success, all done
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/951.out b/tests/generic/951.out
> > > new file mode 100644
> > > index 00000000..f7099ea2
> > > --- /dev/null
> > > +++ b/tests/generic/951.out
> > > @@ -0,0 +1,49 @@
> > > +QA output created by 951
> > > +Test 1 - qa_user, non-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 2 - qa_user, group-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +
> > > +Test 3 - qa_user, user-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 4 - qa_user, all-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +
> > > +Test 5 - root, non-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6666 -rwSrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 6 - root, group-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6676 -rwSrwsrw- SCRATCH_MNT/a
> > > +
> > > +Test 7 - root, user-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6766 -rwsrwSrw- SCRATCH_MNT/a
> > > +
> > > +Test 8 - root, all-exec file
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +310f146ce52077fcd3308dcbe7632bb2 SCRATCH_MNT/a
> > > +6777 -rwsrwsrwx SCRATCH_MNT/a
> > > +
> > > diff --git a/tests/generic/952 b/tests/generic/952
> > > new file mode 100755
> > > index 00000000..86443dcc
> > > --- /dev/null
> > > +++ b/tests/generic/952
> > > @@ -0,0 +1,70 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > > +#
> > > +# FS QA Test 952
> > > +#
> > > +# Functional test for dropping suid and sgid capabilities as part of a reflink.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto clone quick
> > > +
> > > +# Import common functions.
> > > +. ./common/filter
> > > +. ./common/reflink
> > > +
> > > +# real QA test starts here
> > > +
> > > +# Modify as appropriate.
> > > +_supported_fs generic
> > > +_require_user
> > > +_require_command "$GETCAP_PROG" getcap
> > > +_require_command "$SETCAP_PROG" setcap
> > > +_require_scratch_reflink
> > > +
> > > +_scratch_mkfs >> $seqres.full
> > > +_scratch_mount
> > > +chmod a+rw $SCRATCH_MNT/
> > > +
> > > +setup_testfile() {
> > > + rm -f $SCRATCH_MNT/a $SCRATCH_MNT/b
> > > + _pwrite_byte 0x58 0 1m $SCRATCH_MNT/a >> $seqres.full
> > > + _pwrite_byte 0x57 0 1m $SCRATCH_MNT/b >> $seqres.full
> > > + chmod a+rwx $SCRATCH_MNT/a $SCRATCH_MNT/b
> > > + $SETCAP_PROG cap_setgid,cap_setuid+ep $SCRATCH_MNT/a
> > > + sync
> > > +}
> > > +
> > > +commit_and_check() {
> > > + local user="$1"
> > > +
> > > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > > + _getcap -v $SCRATCH_MNT/a | _filter_scratch
> > > +
> > > + local cmd="$XFS_IO_PROG -c 'reflink $SCRATCH_MNT/b 0 0 1m' $SCRATCH_MNT/a"
> > > + if [ -n "$user" ]; then
> > > + su - "$user" -c "$cmd" >> $seqres.full
> > > + else
> > > + $SHELL -c "$cmd" >> $seqres.full
> > > + fi
> > > +
> > > + stat -c '%a %A %n' $SCRATCH_MNT/a | _filter_scratch
> > > + _getcap -v $SCRATCH_MNT/a | _filter_scratch
> > > +
> > > + # Blank line in output
> > > + echo
> > > +}
> > > +
> > > +# Commit by an unprivileged user clears capability bits.
> > > +echo "Test 1 - qa_user"
> > > +setup_testfile
> > > +commit_and_check "$qa_user"
> > > +
> > > +# Commit by root leaves capability bits.
> > > +echo "Test 2 - root"
> > > +setup_testfile
> > > +commit_and_check
> > > +
> > > +# success, all done
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/952.out b/tests/generic/952.out
> > > new file mode 100644
> > > index 00000000..eac9e76a
> > > --- /dev/null
> > > +++ b/tests/generic/952.out
> > > @@ -0,0 +1,13 @@
> > > +QA output created by 952
> > > +Test 1 - qa_user
> > > +777 -rwxrwxrwx SCRATCH_MNT/a
> > > +SCRATCH_MNT/a cap_setgid,cap_setuid=ep
> > > +777 -rwxrwxrwx SCRATCH_MNT/a
> > > +SCRATCH_MNT/a
> > > +
> > > +Test 2 - root
> > > +777 -rwxrwxrwx SCRATCH_MNT/a
> > > +SCRATCH_MNT/a cap_setgid,cap_setuid=ep
> > > +777 -rwxrwxrwx SCRATCH_MNT/a
> > > +SCRATCH_MNT/a
> > > +
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-03-10 17:48 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-26 2:11 [PATCHSET 0/2] fstests: new stuff for kernel 5.17 Darrick J. Wong
2022-01-26 2:11 ` [PATCH 1/2] generic: test suid/sgid behavior with reflink and dedupe Darrick J. Wong
2022-01-26 5:56 ` Zorro Lang
2022-01-27 0:55 ` Darrick J. Wong
2022-01-27 2:52 ` Zorro Lang
2022-01-27 1:27 ` [PATCH v1.1 " Darrick J. Wong
2022-02-16 15:09 ` Filipe Manana
2022-02-18 5:26 ` Darrick J. Wong
2022-03-10 17:48 ` Darrick J. Wong
2022-01-26 2:11 ` [PATCH 2/2] fstests: skip tests that require XFS_IOC_ALLOCSP Darrick J. Wong
2022-01-26 6:13 ` Zorro Lang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).