Re: [PATCH 2/2] generic: ensure we drop suid after fallocate

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Mar 23, 2022 at 11:06:57AM +0800, Zorro Lang wrote:
> On Tue, Mar 15, 2022 at 08:30:40PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > fallocate changes file contents, so make sure that we drop privileges
> > and file capabilities after each fallocate operation.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/generic/834     |  129 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/834.out |   33 +++++++++++++
> >  tests/generic/835     |  129 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/835.out |   33 +++++++++++++
> >  tests/generic/836     |  129 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/836.out |   33 +++++++++++++
> >  tests/generic/837     |  129 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/837.out |   33 +++++++++++++
> >  tests/generic/838     |  129 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/838.out |   33 +++++++++++++
> >  tests/generic/839     |   79 ++++++++++++++++++++++++++++++
> >  tests/generic/839.out |   13 +++++
> >  12 files changed, 902 insertions(+)
> >  create mode 100755 tests/generic/834
> >  create mode 100644 tests/generic/834.out
> >  create mode 100755 tests/generic/835
> >  create mode 100644 tests/generic/835.out
> >  create mode 100755 tests/generic/836
> >  create mode 100644 tests/generic/836.out
> >  create mode 100755 tests/generic/837
> >  create mode 100644 tests/generic/837.out
> >  create mode 100755 tests/generic/838
> >  create mode 100644 tests/generic/838.out
> >  create mode 100755 tests/generic/839
> >  create mode 100755 tests/generic/839.out
> > 
> > 
> > diff --git a/tests/generic/834 b/tests/generic/834
> > new file mode 100755
> > index 00000000..27552221
> > --- /dev/null
> > +++ b/tests/generic/834
> > @@ -0,0 +1,129 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 Oracle.  All Rights Reserved.
> > +#
> > +# FS QA Test No. 834
> > +#
> > +# Functional test for dropping suid and sgid bits as part of a fallocate.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto clone quick
> > +
> > +# Override the default cleanup function.
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -r -f $tmp.* $junk_dir
> > +}
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +. ./common/reflink
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_require_user
> > +_require_test
> > +verb=falloc
> > +_require_xfs_io_command $verb
> > +
> > +_require_congruent_file_oplen $TEST_DIR 65536
>       ^^
> 
> Is it a Darrick's unpublic secret weapon:)

Not really, the latest djwong development branches are always at
https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/

(But yes, the _require_congruent_file_oplen helper prevents us from
running fallocate tests when the selected operation size isn't congruent
with the configured blocksize (e.g. blksz=65536 when the rt extent size
is 28k)

> > +
> > +junk_dir=$TEST_DIR/$seq
> > +junk_file=$junk_dir/a
> > +mkdir -p $junk_dir/
> > +chmod a+rw $junk_dir/
> > +
> 
> [snip] ...
> 
> > diff --git a/tests/generic/839 b/tests/generic/839
> > new file mode 100755
> > index 00000000..a1e23916
> > --- /dev/null
> > +++ b/tests/generic/839
> > @@ -0,0 +1,79 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 Oracle.  All Rights Reserved.
> > +#
> > +# FS QA Test No. 839
> > +#
> > +# Functional test for dropping capability bits as part of an fallocate.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto fiexchange swapext quick
>                         ^^         ^^
> 
> I think we don't have these two group names currently, this will cause failure.
> You can register them if you need :)

Oops.  Yeah, I'll fix these things before I resubmit.  Thanks for
pointing that out. :(

--D

> 
> Thanks,
> Zorro
> 
> > +
> > +# Override the default cleanup function.
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -r -f $tmp.* $junk_dir
> > +}
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_require_user
> > +_require_command "$GETCAP_PROG" getcap
> > +_require_command "$SETCAP_PROG" setcap
> > +_require_xfs_io_command falloc
> > +_require_test
> > +
> > +_require_congruent_file_oplen $TEST_DIR 65536
> > +
> > +junk_dir=$TEST_DIR/$seq
> > +junk_file=$junk_dir/a
> > +mkdir -p $junk_dir/
> > +chmod a+rw $junk_dir/
> > +
> > +setup_testfile() {
> > +	rm -f $junk_file
> > +	touch $junk_file
> > +	chmod a+rwx $junk_file
> > +	$SETCAP_PROG cap_setgid,cap_setuid+ep $junk_file
> > +	sync
> > +}
> > +
> > +commit_and_check() {
> > +	local user="$1"
> > +
> > +	stat -c '%a %A %n' $junk_file | _filter_test_dir
> > +	_getcap -v $junk_file | _filter_test_dir
> > +
> > +	local cmd="$XFS_IO_PROG -c 'falloc 0 64k' $junk_file"
> > +	if [ -n "$user" ]; then
> > +		su - "$user" -c "$cmd" >> $seqres.full
> > +	else
> > +		$SHELL -c "$cmd" >> $seqres.full
> > +	fi
> > +
> > +	stat -c '%a %A %n' $junk_file | _filter_test_dir
> > +	_getcap -v $junk_file | _filter_test_dir
> > +
> > +	# 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/839.out b/tests/generic/839.out
> > new file mode 100755
> > index 00000000..f571cd26
> > --- /dev/null
> > +++ b/tests/generic/839.out
> > @@ -0,0 +1,13 @@
> > +QA output created by 839
> > +Test 1 - qa_user
> > +777 -rwxrwxrwx TEST_DIR/839/a
> > +TEST_DIR/839/a cap_setgid,cap_setuid=ep
> > +777 -rwxrwxrwx TEST_DIR/839/a
> > +TEST_DIR/839/a
> > +
> > +Test 2 - root
> > +777 -rwxrwxrwx TEST_DIR/839/a
> > +TEST_DIR/839/a cap_setgid,cap_setuid=ep
> > +777 -rwxrwxrwx TEST_DIR/839/a
> > +TEST_DIR/839/a
> > +
> > 
>