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

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

[Sun YangKai]

From: Sun YangKai <sunyangkai@fnnas.com>

Test creation, modification and deletion of POSIX ACLs for a btrfs
filesystem which has the read-only property set to true.

Re-test the same after the read-only property is set to false.

This complements btrfs/275, which covers the generic ->setxattr path.
POSIX ACLs are set and removed through the ->set_acl inode operation
(btrfs_set_acl), which is a separate code path that is not covered by
the RO check added for security xattrs in commit b51111271b03 ("btrfs:
check if root is readonly while setting security xattr").  As a result,
ACLs could still be modified on a read-only subvolume, bypassing the RO
protection that applies to every other xattr.

The test exercises the set, get and remove cycle of an access ACL on a
regular file, and expects all modifications to fail with EROFS while the
subvolume is read-only.

Signed-off-by: Sun YangKai <sunyangkai@fnnas.com>
---
 tests/btrfs/353     | 97 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/353.out | 39 ++++++++++++++++++
 2 files changed, 136 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..60059d4a
--- /dev/null
+++ b/tests/btrfs/353
@@ -0,0 +1,97 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2026 YOUR NAME HERE. 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
+
+# TODO: fill in the kernel commit hash that fixes the ->set_acl path once
+# it is merged.
+# _fixed_by_kernel_commit <HASH> \
+#	"btrfs: check if root is readonly when setting posix acl"
+
+_require_acls
+_require_btrfs_command "property"
+_require_scratch
+
+_scratch_mkfs >> $seqres.full 2>&1
+_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

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

[Sun YangKai]

From: Sun YangKai <sunyangkai@fnnas.com>

Test creation, modification and deletion of POSIX ACLs on a btrfs
filesystem that has the read-only property set to true, and re-test the
same after the property is cleared.

Commit 353c2ea735e4 ("btrfs: remove redundant readonly root check in
btrfs_setxattr_trans") removed the read-only root check from
btrfs_setxattr_trans() under the assumption that none of its callers can
run on a read-only root.  That assumption does not hold for the ->set_acl
path: btrfs_set_acl() -> __btrfs_set_acl() reaches
btrfs_setxattr_trans() directly, bypassing the handler-level read-only
checks that were later (re)added by commit b51111271b03 ("btrfs: check
if root is readonly while setting security xattr").  As a result, ACLs
could still be set or removed on a read-only subvolume, bypassing the RO
protection that applies to every other xattr.

The restoring kernel fix adds the read-only check directly to
btrfs_set_acl().  This test complements btrfs/275, which covers the
generic ->setxattr path.

The test exercises the set, get and remove cycle of an access ACL on a
regular file, and expects all modifications to fail with EROFS while the
subvolume is read-only, then succeed once it is writable again.

Signed-off-by: Sun YangKai <sunyangkai@fnnas.com>
---
Changes from v1:

- fix copywrite header
- better wording for commit message

Sorry for bothering.

---
 tests/btrfs/353     | 97 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/353.out | 39 ++++++++++++++++++
 2 files changed, 136 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..586f722a
--- /dev/null
+++ b/tests/btrfs/353
@@ -0,0 +1,97 @@
+#! /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
+
+# TODO: fill in the kernel commit hash that fixes the ->set_acl path once
+# it is merged.
+# _fixed_by_kernel_commit <HASH> \
+#	"btrfs: check if root is readonly when setting posix acl"
+
+_require_acls
+_require_btrfs_command "property"
+_require_scratch
+
+_scratch_mkfs >> $seqres.full 2>&1
+_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

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

[Filipe Manana]

On Mon, Jul 13, 2026 at 10:50 AM Sun YangKai <sunk67188@gmail.com> wrote:
>
> From: Sun YangKai <sunyangkai@fnnas.com>
>
> Test creation, modification and deletion of POSIX ACLs on a btrfs
> filesystem that has the read-only property set to true, and re-test the
> same after the property is cleared.
>
> Commit 353c2ea735e4 ("btrfs: remove redundant readonly root check in
> btrfs_setxattr_trans") removed the read-only root check from
> btrfs_setxattr_trans() under the assumption that none of its callers can
> run on a read-only root.  That assumption does not hold for the ->set_acl
> path: btrfs_set_acl() -> __btrfs_set_acl() reaches
> btrfs_setxattr_trans() directly, bypassing the handler-level read-only
> checks that were later (re)added by commit b51111271b03 ("btrfs: check
> if root is readonly while setting security xattr").  As a result, ACLs
> could still be set or removed on a read-only subvolume, bypassing the RO
> protection that applies to every other xattr.

You don't need to explain this in a test case, those are details for
the kernel patch...

Just say what the test does and that it exercises a bug fixed by a
kernel patch that has the subject:

     "btrfs: check if root is readonly when setting posix acl"

>
> The restoring kernel fix adds the read-only check directly to
> btrfs_set_acl().  This test complements btrfs/275, which covers the
> generic ->setxattr path.
>
> The test exercises the set, get and remove cycle of an access ACL on a
> regular file, and expects all modifications to fail with EROFS while the
> subvolume is read-only, then succeed once it is writable again.
>
> Signed-off-by: Sun YangKai <sunyangkai@fnnas.com>
> ---
> Changes from v1:
>
> - fix copywrite header
> - better wording for commit message
>
> Sorry for bothering.
>
> ---
>  tests/btrfs/353     | 97 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/353.out | 39 ++++++++++++++++++
>  2 files changed, 136 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..586f722a
> --- /dev/null
> +++ b/tests/btrfs/353
> @@ -0,0 +1,97 @@
> +#! /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
> +
> +# TODO: fill in the kernel commit hash that fixes the ->set_acl path once
> +# it is merged.
> +# _fixed_by_kernel_commit <HASH> \
> +#      "btrfs: check if root is readonly when setting posix acl"

That's not how we handle patches not yet in Linus' tree.
First don't comment it, second use xxxxxxxxxxxx for the hash.
Like this:

_fixed_by_kernel_commit xxxxxxxxxxxx \
     "btrfs: check if root is readonly when setting posix acl"

We normally find and update tests that need the hash by greping with
something like:  egrep -ri 'commit xxxxx' tests/



> +
> +_require_acls
> +_require_btrfs_command "property"
> +_require_scratch
> +
> +_scratch_mkfs >> $seqres.full 2>&1

Nowadays we have the practice of also adding:   || _fail "mkfs failed"

> +_scratch_mount
> +
> +FILENAME=$SCRATCH_MNT/foo

Don't use upper case variable names.

With that fixed:

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.


> +
> +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
>
>

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

[Sun YangKai]

This is my first time to contribute to fstests project so thanks a lot 
for pointing those things out and it really helps a lot. I'll fix them 
and send a v2 version later.

Thanks,
Sun YangKai

On 2026/7/21 00:16, Filipe Manana wrote:
> On Mon, Jul 13, 2026 at 10:50 AM Sun YangKai <sunk67188@gmail.com> wrote:
>>
>> From: Sun YangKai <sunyangkai@fnnas.com>
>>
>> Test creation, modification and deletion of POSIX ACLs on a btrfs
>> filesystem that has the read-only property set to true, and re-test the
>> same after the property is cleared.
>>
>> Commit 353c2ea735e4 ("btrfs: remove redundant readonly root check in
>> btrfs_setxattr_trans") removed the read-only root check from
>> btrfs_setxattr_trans() under the assumption that none of its callers can
>> run on a read-only root.  That assumption does not hold for the ->set_acl
>> path: btrfs_set_acl() -> __btrfs_set_acl() reaches
>> btrfs_setxattr_trans() directly, bypassing the handler-level read-only
>> checks that were later (re)added by commit b51111271b03 ("btrfs: check
>> if root is readonly while setting security xattr").  As a result, ACLs
>> could still be set or removed on a read-only subvolume, bypassing the RO
>> protection that applies to every other xattr.
> 
> You don't need to explain this in a test case, those are details for
> the kernel patch...
> 
> Just say what the test does and that it exercises a bug fixed by a
> kernel patch that has the subject:
> 
>       "btrfs: check if root is readonly when setting posix acl"

>>
>> The restoring kernel fix adds the read-only check directly to
>> btrfs_set_acl().  This test complements btrfs/275, which covers the
>> generic ->setxattr path.
>>
>> The test exercises the set, get and remove cycle of an access ACL on a
>> regular file, and expects all modifications to fail with EROFS while the
>> subvolume is read-only, then succeed once it is writable again.
>>
>> Signed-off-by: Sun YangKai <sunyangkai@fnnas.com>
>> ---
>> Changes from v1:
>>
>> - fix copywrite header
>> - better wording for commit message
>>
>> Sorry for bothering.
>>
>> ---
>>   tests/btrfs/353     | 97 +++++++++++++++++++++++++++++++++++++++++++++
>>   tests/btrfs/353.out | 39 ++++++++++++++++++
>>   2 files changed, 136 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..586f722a
>> --- /dev/null
>> +++ b/tests/btrfs/353
>> @@ -0,0 +1,97 @@
>> +#! /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
>> +
>> +# TODO: fill in the kernel commit hash that fixes the ->set_acl path once
>> +# it is merged.
>> +# _fixed_by_kernel_commit <HASH> \
>> +#      "btrfs: check if root is readonly when setting posix acl"
> 
> That's not how we handle patches not yet in Linus' tree.
> First don't comment it, second use xxxxxxxxxxxx for the hash.
> Like this:
> 
> _fixed_by_kernel_commit xxxxxxxxxxxx \
>       "btrfs: check if root is readonly when setting posix acl"
> 
> We normally find and update tests that need the hash by greping with
> something like:  egrep -ri 'commit xxxxx' tests/
> 
> 
> 
>> +
>> +_require_acls
>> +_require_btrfs_command "property"
>> +_require_scratch
>> +
>> +_scratch_mkfs >> $seqres.full 2>&1
> 
> Nowadays we have the practice of also adding:   || _fail "mkfs failed"
> 
>> +_scratch_mount
>> +
>> +FILENAME=$SCRATCH_MNT/foo
> 
> Don't use upper case variable names.
> 
> With that fixed:
> 
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
> 
> Thanks.
> 
> 
>> +
>> +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
>>
>>

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

[Sun YangKai]

From: Sun YangKai <sunyangkai@fygo.io>

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 <sunyangkai@fygo.io>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
---
Changes since v2:

- updated commit message
- proper format for fixed_by_kernel_commit
- lower case variable name
- handle mkfs failure

Suggested by Filipe Manana
---
 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