[PATCH v2] btrfs: check if root is readonly when setting posix acl

[PATCH v2] btrfs: check if root is readonly when setting posix acl

[Sun YangKai]

From: Sun YangKai <sunyangkai@fnnas.com>

For a filesystem which has btrfs read-only property set to true, all
write operations including acl and xattr should be denied. However,
acl can still be set even if btrfs ro property is true.

This happens because no function on the set_acl code path checks the root
is readonly or not. It was checked in btrfs_setxattr_trans() but got
removed in
commit 353c2ea735e4 ("btrfs: remove redundant readonly root check in btrfs_setxattr_trans")

That commit didn't check if all the callers properly check the root's
read-only flag. A previous fix is
commit b51111271b03("btrfs: check if root is readonly while setting security xattr")

Always check if the root is read-only before performing the set acl
operation.

Fixes: 353c2ea735e4 ("btrfs: remove redundant readonly root check in btrfs_setxattr_trans")
Signed-off-by: Sun YangKai <sunyangkai@fnnas.com>
---
Changes to v1:
- add the missing header file

---
 fs/btrfs/acl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
index e55b686fe1ab..662cdd1cbdef 100644
--- a/fs/btrfs/acl.c
+++ b/fs/btrfs/acl.c
@@ -15,6 +15,7 @@
 #include "xattr.h"
 #include "acl.h"
 #include "misc.h"
+#include "btrfs_inode.h"
 
 struct posix_acl *btrfs_get_acl(struct inode *inode, int type, bool rcu)
 {
@@ -107,6 +108,9 @@ int btrfs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
 	struct inode *inode = d_inode(dentry);
 	umode_t old_mode = inode->i_mode;
 
+	if (btrfs_root_readonly(BTRFS_I(inode)->root))
+		return -EROFS;
+
 	if (type == ACL_TYPE_ACCESS && acl) {
 		ret = posix_acl_update_mode(idmap, inode,
 					    &inode->i_mode, &acl);
-- 
2.54.0

Re: [PATCH v2] btrfs: check if root is readonly when setting posix acl

[Johannes Thumshirn]

Looks good,

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH v2] btrfs: check if root is readonly when setting posix acl

[Filipe Manana]

On Thu, Jul 9, 2026 at 9:26 AM Sun YangKai <sunk67188@gmail.com> wrote:
>
> From: Sun YangKai <sunyangkai@fnnas.com>
>
> For a filesystem which has btrfs read-only property set to true, all
> write operations including acl and xattr should be denied. However,
> acl can still be set even if btrfs ro property is true.
>
> This happens because no function on the set_acl code path checks the root
> is readonly or not. It was checked in btrfs_setxattr_trans() but got
> removed in
> commit 353c2ea735e4 ("btrfs: remove redundant readonly root check in btrfs_setxattr_trans")
>
> That commit didn't check if all the callers properly check the root's
> read-only flag. A previous fix is
> commit b51111271b03("btrfs: check if root is readonly while setting security xattr")
>
> Always check if the root is read-only before performing the set acl
> operation.
>
> Fixes: 353c2ea735e4 ("btrfs: remove redundant readonly root check in btrfs_setxattr_trans")
> Signed-off-by: Sun YangKai <sunyangkai@fnnas.com>

Test case please (fstests).

> ---
> Changes to v1:
> - add the missing header file
>
> ---
>  fs/btrfs/acl.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
> index e55b686fe1ab..662cdd1cbdef 100644
> --- a/fs/btrfs/acl.c
> +++ b/fs/btrfs/acl.c
> @@ -15,6 +15,7 @@
>  #include "xattr.h"
>  #include "acl.h"
>  #include "misc.h"
> +#include "btrfs_inode.h"
>
>  struct posix_acl *btrfs_get_acl(struct inode *inode, int type, bool rcu)
>  {
> @@ -107,6 +108,9 @@ int btrfs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>         struct inode *inode = d_inode(dentry);
>         umode_t old_mode = inode->i_mode;
>
> +       if (btrfs_root_readonly(BTRFS_I(inode)->root))
> +               return -EROFS;
> +
>         if (type == ACL_TYPE_ACCESS && acl) {
>                 ret = posix_acl_update_mode(idmap, inode,
>                                             &inode->i_mode, &acl);
> --
> 2.54.0
>
>

Re: [PATCH v2] btrfs: check if root is readonly when setting posix acl

[Sun YangKai]

On 2026/7/9 18:43, Filipe Manana wrote:
> On Thu, Jul 9, 2026 at 9:26 AM Sun YangKai <sunk67188@gmail.com> wrote:
>>
>> From: Sun YangKai <sunyangkai@fnnas.com>
>>
>> For a filesystem which has btrfs read-only property set to true, all
>> write operations including acl and xattr should be denied. However,
>> acl can still be set even if btrfs ro property is true.
>>
>> This happens because no function on the set_acl code path checks the root
>> is readonly or not. It was checked in btrfs_setxattr_trans() but got
>> removed in
>> commit 353c2ea735e4 ("btrfs: remove redundant readonly root check in btrfs_setxattr_trans")
>>
>> That commit didn't check if all the callers properly check the root's
>> read-only flag. A previous fix is
>> commit b51111271b03("btrfs: check if root is readonly while setting security xattr")
>>
>> Always check if the root is read-only before performing the set acl
>> operation.
>>
>> Fixes: 353c2ea735e4 ("btrfs: remove redundant readonly root check in btrfs_setxattr_trans")
>> Signed-off-by: Sun YangKai <sunyangkai@fnnas.com>
> 
> Test case please (fstests).

Ok, I'll work on that later.

I've noticed the test case btrfs/275 for b51111271b03 and I'll take that 
as reference.

Thanks,
Sun YangKai

> 
>> ---
>> Changes to v1:
>> - add the missing header file
>>
>> ---
>>   fs/btrfs/acl.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
>> index e55b686fe1ab..662cdd1cbdef 100644
>> --- a/fs/btrfs/acl.c
>> +++ b/fs/btrfs/acl.c
>> @@ -15,6 +15,7 @@
>>   #include "xattr.h"
>>   #include "acl.h"
>>   #include "misc.h"
>> +#include "btrfs_inode.h"
>>
>>   struct posix_acl *btrfs_get_acl(struct inode *inode, int type, bool rcu)
>>   {
>> @@ -107,6 +108,9 @@ int btrfs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>>          struct inode *inode = d_inode(dentry);
>>          umode_t old_mode = inode->i_mode;
>>
>> +       if (btrfs_root_readonly(BTRFS_I(inode)->root))
>> +               return -EROFS;
>> +
>>          if (type == ACL_TYPE_ACCESS && acl) {
>>                  ret = posix_acl_update_mode(idmap, inode,
>>                                              &inode->i_mode, &acl);
>> --
>> 2.54.0
>>
>>