Re: [PATCH] btrfs: Test xattr changes for read-only btrfs property

[Anand Jain <anand.jain@oracle.com>]

On 8/17/22 05:40, Goldwyn Rodrigues wrote:
> 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 BTRFS read-only property is set to false.
> 
> This tests the bug for "security.*" modifications which escape
> xattr_permission(), because security parameters are let through
> in xattr_permission(), without checks from
> inode_permission()->btrfs_permission(). There is no restriction on
> security.* from VFS and decision is left to the underlying filesystem.
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> diff --git a/tests/btrfs/273 b/tests/btrfs/273
> new file mode 100755
> index 00000000..ec7d264d
> --- /dev/null
> +++ b/tests/btrfs/273
> @@ -0,0 +1,78 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FS QA Test No. 273
> +#
> +# 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
> +_require_attrs
> +_require_btrfs_command "property"
> +_require_scratch
> +
> +_scratch_mkfs > /dev/null 2>&1 || _fail "mkfs failed"
> +_scratch_mount
> +
> +FILENAME=$SCRATCH_MNT/foo
> +
> +set_xattr() {
> +	local value=$1
> +	$SETFATTR_PROG -n "user.one" -v $value $FILENAME
> +	$SETFATTR_PROG -n "security.one" -v $value $FILENAME
> +	$SETFATTR_PROG -n "trusted.one" -v $value $FILENAME
> +}
> +
> +get_xattr() {
> +	$GETFATTR_PROG -n "user.one" $FILENAME
> +	$GETFATTR_PROG -n "security.one" $FILENAME
> +	$GETFATTR_PROG -n "trusted.one" $FILENAME
> +}
> +
> +del_xattr() {
> +	$SETFATTR_PROG -x "user.one" $FILENAME
> +	$SETFATTR_PROG -x "security.one" $FILENAME
> +	$SETFATTR_PROG -x "trusted.one" $FILENAME
> +}
> +

This output contains mnt references that need to be filtered.
Filter _filter_scratch should help.



> +# 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 is not changed
> +get_xattr
> +
> +# Attempt to remove xattr from RO (property) filesystem
> +del_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
> +

  Nit:  We are reusing the value "2" and changing it to "3"  makes it
  unique and so the debugging easier.


> +# Get the changed values
> +get_xattr
> +
> +# Remove xattr
> +del_xattr
> +
> +status=0
> +exit
> diff --git a/tests/btrfs/273.out b/tests/btrfs/273.out
> new file mode 100644
> index 00000000..f6fca029
> --- /dev/null
> +++ b/tests/btrfs/273.out
> @@ -0,0 +1,33 @@
> +QA output created by 273
> +ro=true
> +setfattr: /scratch/foo: Read-only file system
> +setfattr: /scratch/foo: Read-only file system
> +setfattr: /scratch/foo: Read-only file system
> +getfattr: Removing leading '/' from absolute path names
> +# file: scratch/foo
> +user.one="1"
> +
> +getfattr: Removing leading '/' from absolute path names
> +# file: scratch/foo
> +security.one="1"
> +
> +getfattr: Removing leading '/' from absolute path names
> +# file: scratch/foo
> +trusted.one="1"
> +
> +setfattr: /scratch/foo: Read-only file system
> +setfattr: /scratch/foo: Read-only file system
> +setfattr: /scratch/foo: Read-only file system
> +ro=false
> +getfattr: Removing leading '/' from absolute path names
> +# file: scratch/foo
> +user.one="2"
> +
> +getfattr: Removing leading '/' from absolute path names
> +# file: scratch/foo
> +security.one="2"
> +
> +getfattr: Removing leading '/' from absolute path names
> +# file: scratch/foo
> +trusted.one="2"


> +

Nit: A whitespace.

Thanks.