Re: [PATCH v3] btrfs: test POSIX ACL changes for RO btrfs property

On Wed, Jul 22, 2026 at 03:52:47PM +0800, Sun YangKai wrote:
> From: Sun YangKai <[email protected]>
> 
> Test creation, modification and deletion of POSIX ACLs on a btrfs
> filesystem that has the read-only property set to true.
> 
> This exercises a bug fixed by the kernel patch with subject:
> "btrfs: check if root is readonly when setting posix acl"
> 
> Signed-off-by: Sun YangKai <[email protected]>
> Reviewed-by: Filipe Manana <[email protected]>
> ---
> Changes since v2:
> 
> - updated commit message
> - proper format for fixed_by_kernel_commit
> - lower case variable name
> - handle mkfs failure
> 
> Suggested by Filipe Manana
> ---

This version is good to me, if no more review points form btrfs list, I'll
merge it.

Reviewed-by: Zorro Lang <[email protected]>

>  tests/btrfs/353     | 95 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/353.out | 39 +++++++++++++++++++
>  2 files changed, 134 insertions(+)
>  create mode 100755 tests/btrfs/353
>  create mode 100644 tests/btrfs/353.out
> 
> diff --git a/tests/btrfs/353 b/tests/btrfs/353
> new file mode 100755
> index 00000000..63ba4681
> --- /dev/null
> +++ b/tests/btrfs/353
> @@ -0,0 +1,95 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2026 Fygo OS. All Rights Reserved.
> +#
> +# FS QA Test No. 353
> +#
> +# Test that POSIX ACLs cannot be changed once a btrfs subvolume has the
> +# read-only property set.
> +#
> +# Setting or removing a POSIX ACL goes through the ->set_acl inode
> +# operation, which is a different code path from the generic ->setxattr
> +# one covered by btrfs/275.  It used to be allowed on a read-only
> +# subvolume and thus bypassed the RO protection.  Such modifications must
> +# fail with EROFS, just like any other xattr.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick acl attr
> +
> +. ./common/filter
> +. ./common/attr
> +
> +_fixed_by_kernel_commit xxxxxxxxxxxx \
> +	"btrfs: check if root is readonly when setting posix acl"
> +
> +_require_acls
> +_require_btrfs_command "property"
> +_require_scratch
> +
> +_scratch_mkfs >> $seqres.full 2>&1 || _fail "mkfs failed"
> +_scratch_mount
> +
> +filename=$SCRATCH_MNT/foo
> +
> +set_acl()
> +{
> +	local perm=$1
> +
> +	# Use -n so setfacl does not recalculate the mask, keeping the
> +	# golden output deterministic regardless of the named user's
> +	# permissions.
> +	setfacl -n -m u:$acl2:$perm,m::rwx $filename 2>&1 | _filter_scratch
> +}
> +
> +get_acl()
> +{
> +	getfacl --absolute-names -n $filename | _filter_scratch | _getfacl_filter_id
> +}
> +
> +del_acl()
> +{
> +	setfacl -b $filename 2>&1 | _filter_scratch
> +}
> +
> +_acl_setup_ids
> +
> +# Create a test file.
> +echo "hello world" > $filename
> +
> +# Set an initial ACL while the subvolume is writable.
> +set_acl rwx
> +
> +# Attempt to change the ACL once the subvolume is read-only.  This must
> +# fail with EROFS.
> +$BTRFS_UTIL_PROG property set $SCRATCH_MNT ro true
> +$BTRFS_UTIL_PROG property get $SCRATCH_MNT ro
> +
> +set_acl r--
> +
> +# The ACL must not have changed.
> +get_acl
> +
> +# Attempt to remove the ACL from the read-only subvolume.  This must
> +# fail with EROFS as well.
> +del_acl
> +
> +# The ACL must still be present.
> +get_acl
> +
> +# Make the subvolume writable again.
> +$BTRFS_UTIL_PROG property set $SCRATCH_MNT ro false
> +$BTRFS_UTIL_PROG property get $SCRATCH_MNT ro
> +
> +# Now changing the ACL must succeed.
> +set_acl r--
> +
> +get_acl
> +
> +# And removing it must succeed too.
> +del_acl
> +
> +# Check the ACL is really gone.
> +get_acl
> +
> +status=0
> +exit
> diff --git a/tests/btrfs/353.out b/tests/btrfs/353.out
> new file mode 100644
> index 00000000..66f4a0e5
> --- /dev/null
> +++ b/tests/btrfs/353.out
> @@ -0,0 +1,39 @@
> +QA output created by 353
> +ro=true
> +setfacl: SCRATCH_MNT/foo: Read-only file system
> +# file: SCRATCH_MNT/foo
> +# owner: 0
> +# group: 0
> +user::rw-
> +user:id2:rwx
> +group::r--
> +mask::rwx
> +other::r--
> +
> +setfacl: SCRATCH_MNT/foo: Read-only file system
> +# file: SCRATCH_MNT/foo
> +# owner: 0
> +# group: 0
> +user::rw-
> +user:id2:rwx
> +group::r--
> +mask::rwx
> +other::r--
> +
> +ro=false
> +# file: SCRATCH_MNT/foo
> +# owner: 0
> +# group: 0
> +user::rw-
> +user:id2:r--
> +group::r--
> +mask::rwx
> +other::r--
> +
> +# file: SCRATCH_MNT/foo
> +# owner: 0
> +# group: 0
> +user::rw-
> +group::r--
> +other::r--
> +
> -- 
> 2.54.0
> 
>