Message ID | f72207b27052afc7dcbdf4fb7cf956fc37105724.1663840534.git.fdmanana@suse.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v3] btrfs: test xattr changes for RO btrfs property | expand |
On Thu, Sep 22, 2022 at 10:56:39AM +0100, fdmanana@kernel.org wrote: > From: Goldwyn Rodrigues <rgoldwyn@suse.de> > > Test creation, modification and deletion of xattr for a BTRFS filesystem > which has the read-only property set to true. > > Re-test the same after read-only property is set to false. > > This tests the bug for "security.*" modifications which escape > xattr_permission(), because security parameters are usually let through > in xattr_permission, without checking > inode_permission()->btrfs_permission(). > > This resulted in being able to change a security xattr on a RO subvolume > until it got fixed by kernel commit b51111271b03 ("btrfs: check if root > is readonly while setting security xattr"). > > Signed-off-by: Filipe Manana <fdmanana@suse.com> > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> > --- > > V3: Use the _getfattr() helper. > Redirect mkfs output to the .full file and remove the _fail call, > which is an antipattern. > > Change the subject from: > > "btrfs: test security xattr changes for RO btrfs property" > > to: > > "btrfs: test xattr changes for RO btrfs property" > > Since it does not test only security xattrs. This version looks good to me, Reviewed-by: Zorro Lang <zlang@redhat.com> > > tests/btrfs/275 | 88 +++++++++++++++++++++++++++++++++++++++++++++ > tests/btrfs/275.out | 39 ++++++++++++++++++++ > 2 files changed, 127 insertions(+) > create mode 100755 tests/btrfs/275 > create mode 100644 tests/btrfs/275.out > > diff --git a/tests/btrfs/275 b/tests/btrfs/275 > new file mode 100755 > index 00000000..b34fbda0 > --- /dev/null > +++ b/tests/btrfs/275 > @@ -0,0 +1,88 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved. > +# > +# FS QA Test No. 275 > +# > +# Test that no xattr can be changed once btrfs property is set to RO. > +# > +. ./common/preamble > +_begin_fstest auto quick attr > + > +# Import common functions. > +. ./common/filter > +. ./common/attr > + > +# real QA test starts here > +_supported_fs btrfs > +_fixed_by_kernel_commit b51111271b03 \ > + "btrfs: check if root is readonly while setting security xattr" > +_require_attrs > +_require_btrfs_command "property" > +_require_scratch > + > +_scratch_mkfs >> $seqres.full 2>&1 > +_scratch_mount > + > +FILENAME=$SCRATCH_MNT/foo > + > +set_xattr() > +{ > + local value=$1 > + $SETFATTR_PROG -n "user.one" -v $value $FILENAME 2>&1 | _filter_scratch > + $SETFATTR_PROG -n "security.one" -v $value $FILENAME 2>&1 | _filter_scratch > + $SETFATTR_PROG -n "trusted.one" -v $value $FILENAME 2>&1 | _filter_scratch > +} > + > +get_xattr() > +{ > + _getfattr --absolute-names -n "user.one" $FILENAME 2>&1 | _filter_scratch > + _getfattr --absolute-names -n "security.one" $FILENAME 2>&1 | _filter_scratch > + _getfattr --absolute-names -n "trusted.one" $FILENAME 2>&1 | _filter_scratch > +} > + > +del_xattr() > +{ > + $SETFATTR_PROG -x "user.one" $FILENAME 2>&1 | _filter_scratch > + $SETFATTR_PROG -x "security.one" $FILENAME 2>&1 | _filter_scratch > + $SETFATTR_PROG -x "trusted.one" $FILENAME 2>&1 | _filter_scratch > +} > + > +# Create a test file. > +echo "hello world" > $FILENAME > + > +set_xattr 1 > + > +$BTRFS_UTIL_PROG property set $SCRATCH_MNT ro true > +$BTRFS_UTIL_PROG property get $SCRATCH_MNT ro > + > +# Attempt to change values of RO (property) filesystem. > +set_xattr 2 > + > +# Check the values of RO (property) filesystem are not changed. > +get_xattr > + > +# Attempt to remove xattr from RO (property) filesystem. > +del_xattr > + > +# Check if xattr still exist. > +get_xattr > + > +# Change filesystem property RO to false > +$BTRFS_UTIL_PROG property set $SCRATCH_MNT ro false > +$BTRFS_UTIL_PROG property get $SCRATCH_MNT ro > + > +# Change the xattrs after RO is false. > +set_xattr 2 > + > +# Get the changed values. > +get_xattr > + > +# Remove xattr. > +del_xattr > + > +# check if the xattrs are really deleted. > +get_xattr > + > +status=0 > +exit > diff --git a/tests/btrfs/275.out b/tests/btrfs/275.out > new file mode 100644 > index 00000000..fb8f02f8 > --- /dev/null > +++ b/tests/btrfs/275.out > @@ -0,0 +1,39 @@ > +QA output created by 275 > +ro=true > +setfattr: SCRATCH_MNT/foo: Read-only file system > +setfattr: SCRATCH_MNT/foo: Read-only file system > +setfattr: SCRATCH_MNT/foo: Read-only file system > +# file: SCRATCH_MNT/foo > +user.one="1" > + > +# file: SCRATCH_MNT/foo > +security.one="1" > + > +# file: SCRATCH_MNT/foo > +trusted.one="1" > + > +setfattr: SCRATCH_MNT/foo: Read-only file system > +setfattr: SCRATCH_MNT/foo: Read-only file system > +setfattr: SCRATCH_MNT/foo: Read-only file system > +# file: SCRATCH_MNT/foo > +user.one="1" > + > +# file: SCRATCH_MNT/foo > +security.one="1" > + > +# file: SCRATCH_MNT/foo > +trusted.one="1" > + > +ro=false > +# file: SCRATCH_MNT/foo > +user.one="2" > + > +# file: SCRATCH_MNT/foo > +security.one="2" > + > +# file: SCRATCH_MNT/foo > +trusted.one="2" > + > +SCRATCH_MNT/foo: user.one: No such attribute > +SCRATCH_MNT/foo: security.one: No such attribute > +SCRATCH_MNT/foo: trusted.one: No such attribute > -- > 2.35.1 >
diff --git a/tests/btrfs/275 b/tests/btrfs/275 new file mode 100755 index 00000000..b34fbda0 --- /dev/null +++ b/tests/btrfs/275 @@ -0,0 +1,88 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved. +# +# FS QA Test No. 275 +# +# Test that no xattr can be changed once btrfs property is set to RO. +# +. ./common/preamble +_begin_fstest auto quick attr + +# Import common functions. +. ./common/filter +. ./common/attr + +# real QA test starts here +_supported_fs btrfs +_fixed_by_kernel_commit b51111271b03 \ + "btrfs: check if root is readonly while setting security xattr" +_require_attrs +_require_btrfs_command "property" +_require_scratch + +_scratch_mkfs >> $seqres.full 2>&1 +_scratch_mount + +FILENAME=$SCRATCH_MNT/foo + +set_xattr() +{ + local value=$1 + $SETFATTR_PROG -n "user.one" -v $value $FILENAME 2>&1 | _filter_scratch + $SETFATTR_PROG -n "security.one" -v $value $FILENAME 2>&1 | _filter_scratch + $SETFATTR_PROG -n "trusted.one" -v $value $FILENAME 2>&1 | _filter_scratch +} + +get_xattr() +{ + _getfattr --absolute-names -n "user.one" $FILENAME 2>&1 | _filter_scratch + _getfattr --absolute-names -n "security.one" $FILENAME 2>&1 | _filter_scratch + _getfattr --absolute-names -n "trusted.one" $FILENAME 2>&1 | _filter_scratch +} + +del_xattr() +{ + $SETFATTR_PROG -x "user.one" $FILENAME 2>&1 | _filter_scratch + $SETFATTR_PROG -x "security.one" $FILENAME 2>&1 | _filter_scratch + $SETFATTR_PROG -x "trusted.one" $FILENAME 2>&1 | _filter_scratch +} + +# Create a test file. +echo "hello world" > $FILENAME + +set_xattr 1 + +$BTRFS_UTIL_PROG property set $SCRATCH_MNT ro true +$BTRFS_UTIL_PROG property get $SCRATCH_MNT ro + +# Attempt to change values of RO (property) filesystem. +set_xattr 2 + +# Check the values of RO (property) filesystem are not changed. +get_xattr + +# Attempt to remove xattr from RO (property) filesystem. +del_xattr + +# Check if xattr still exist. +get_xattr + +# Change filesystem property RO to false +$BTRFS_UTIL_PROG property set $SCRATCH_MNT ro false +$BTRFS_UTIL_PROG property get $SCRATCH_MNT ro + +# Change the xattrs after RO is false. +set_xattr 2 + +# Get the changed values. +get_xattr + +# Remove xattr. +del_xattr + +# check if the xattrs are really deleted. +get_xattr + +status=0 +exit diff --git a/tests/btrfs/275.out b/tests/btrfs/275.out new file mode 100644 index 00000000..fb8f02f8 --- /dev/null +++ b/tests/btrfs/275.out @@ -0,0 +1,39 @@ +QA output created by 275 +ro=true +setfattr: SCRATCH_MNT/foo: Read-only file system +setfattr: SCRATCH_MNT/foo: Read-only file system +setfattr: SCRATCH_MNT/foo: Read-only file system +# file: SCRATCH_MNT/foo +user.one="1" + +# file: SCRATCH_MNT/foo +security.one="1" + +# file: SCRATCH_MNT/foo +trusted.one="1" + +setfattr: SCRATCH_MNT/foo: Read-only file system +setfattr: SCRATCH_MNT/foo: Read-only file system +setfattr: SCRATCH_MNT/foo: Read-only file system +# file: SCRATCH_MNT/foo +user.one="1" + +# file: SCRATCH_MNT/foo +security.one="1" + +# file: SCRATCH_MNT/foo +trusted.one="1" + +ro=false +# file: SCRATCH_MNT/foo +user.one="2" + +# file: SCRATCH_MNT/foo +security.one="2" + +# file: SCRATCH_MNT/foo +trusted.one="2" + +SCRATCH_MNT/foo: user.one: No such attribute +SCRATCH_MNT/foo: security.one: No such attribute +SCRATCH_MNT/foo: trusted.one: No such attribute