Re: [PATCH v2] btrfs: fix chattr failing on nodatasum files that have the compress flag

[Filipe Manana]

On Sun, Oct 11, 2026 at 4:58 PM Hengyu Liang <hengyul@xxxxxxxxxx> wrote:
>
> Commit cf1afec09e9f ("btrfs: validate properties before setting them")
> made btrfs_set_prop() validate a property before setting it. For the
> compression property, the validation fails with -EINVAL when the inode
> has NODATASUM.
>
> However, chattr writes back all the flags it read from the file, and
> btrfs_fileattr_set() sets the compression property whenever the new
> flags include FS_COMPR_FL, even when the compression flags do not
> change. A file created on a filesystem mounted with nodatasum has
> NODATASUM and still inherits the compress flag from its directory. As
> of now, chattr fails on such a file when it changes any other flag.
>
> The issue can be reproduced with:
>
>     mkfs.btrfs -f /dev/sdb
>     mount -o nodatasum /dev/sdb /mnt
>     mkdir /mnt/dir
>     chattr +c /mnt/dir
>     touch /mnt/dir/file
>     chattr +A /mnt/dir/file
>
> Before commit cf1afec09e9f ("btrfs: validate properties before setting
> them"), the last command succeeds.
>
> After that commit, it fails with EINVAL.
>
> This patch will make btrfs_fileattr_set() leave the compression flags
> and the compression property alone when the call changes neither
> FS_COMPR_FL nor FS_NOCOMP_FL.
>
> Fixes: cf1afec09e9f ("btrfs: validate properties before setting them")
> Cc: stable@xxxxxxxxxxxxxxx # 7.2+
> Suggested-by: Qu Wenruo <wqu@xxxxxxxx>
> Assisted-by: LLM
> Signed-off-by: Hengyu Liang <hengyul@xxxxxxxxxx>

Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>

Pushed to the for-next github branch, thanks.

> ---
> Changelog
> v2:
> - Do not test whether the inode can be compressed. Leave the compression
>   flags and the property alone when the call does not change them, as
>   suggested by Qu Wenruo. Setting the compress flag on a NODATASUM inode
>   still fails with -EINVAL.
>
> - In the failing call FS_COMPR_FL is set, because chattr writes back the
>   flags it read. A test for "neither FS_NOCOMP_FL nor FS_COMPR_FL is
>   set" does not catch it and would stop chattr -c from removing the
>   flag, so the check compares the old and the new flags.
>
> - Explain why the call fails.
>
> v1:
> https://lore.kernel.org/linux-btrfs/20261010105425.165104-1-hengyul@xxxxxxxxxx/
>
>  fs/btrfs/ioctl.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 52aab510aea0..43a945345cbe 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -375,6 +375,19 @@ int btrfs_fileattr_set(struct mnt_idmap *idmap,
>                 }
>         }
>
> +       /*
> +        * Callers write back the whole flag set, so only touch the compression
> +        * flags and the compression property if FS_COMPR_FL or FS_NOCOMP_FL
> +        * changes.
> +        */
> +       if (!((fsflags ^ old_fsflags) & (FS_COMPR_FL | FS_NOCOMP_FL))) {
> +               /* 1 item for the inode */
> +               trans = btrfs_start_transaction(root, 1);
> +               if (IS_ERR(trans))
> +                       return PTR_ERR(trans);
> +               goto update_flags;
> +       }
> +
>         /*
>          * The COMPRESS flag can only be changed by users, while the NOCOMPRESS
>          * flag may be changed automatically if compression code won't make
> --
> 2.53.0
>
>