Linux Btrfs filesystem development
 help / color / mirror / Atom feed
From: Sun YangKai <sunk67188@gmail.com>
To: linux-btrfs@vger.kernel.org, fstests@vger.kernel.org
Cc: Sun YangKai <sunyangkai@fnnas.com>
Subject: [PATCH v2] btrfs: test POSIX ACL changes for RO btrfs property
Date: Mon, 13 Jul 2026 17:44:45 +0800	[thread overview]
Message-ID: <20260713094611.19703-1-sunk67188@gmail.com> (raw)
In-Reply-To: <20260709082500.17907-2-sunk67188@gmail.com>

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


      parent reply	other threads:[~2026-07-13  9:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09  8:23 [PATCH v2] btrfs: check if root is readonly when setting posix acl Sun YangKai
2026-07-09  9:54 ` Johannes Thumshirn
2026-07-09 10:43 ` Filipe Manana
2026-07-10  9:03   ` Sun YangKai
2026-07-13  9:06 ` [PATCH] btrfs: test POSIX ACL changes for RO btrfs property Sun YangKai
2026-07-13  9:44 ` Sun YangKai [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260713094611.19703-1-sunk67188@gmail.com \
    --to=sunk67188@gmail.com \
    --cc=fstests@vger.kernel.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=sunyangkai@fnnas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox