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

On 2026/7/21 00:03, Filipe Manana wrote:
> On Thu, Jul 9, 2026 at 9:26 AM Sun YangKai <[email protected]> wrote:
>>
>> From: Sun YangKai <[email protected]>
>>
>> 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")
> 
> This can be split across multiple lines, the only place it shouldn't
> be is in the Fixes tag.

Got it.

>>
>> 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")
> 
> Please always leave a space between the commit hash and the opening parenthesis.

My bad

> I've amended that and pushed to for-next on github, thanks.
> 
> Reviewed-by: Filipe Manana <[email protected]>

Thanks a lot for your fix and review.

Sun YangKai

>>
>> 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 <[email protected]>
>> ---
>> 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
>>
>>