On 8/18/22 16:09, Filipe Manana wrote:
On Thu, Aug 18, 2022 at 4:08 AM Anand Jain <anand.jain@xxxxxxxxxx> wrote:
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@xxxxxxxx>
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.
That was already pointed out in my previous review, wasn't it?
Oh right. I missed it.
+# 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.
That's a good idea. >
+# 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.
It's needed, getfattr prints a blank line at the end.
Ok.
Thanks.
Thanks.
