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

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.

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

I've amended that and pushed to for-next on github, thanks.

Reviewed-by: Filipe Manana <[email protected]>

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