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

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

[Hengyu Liang]

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@vger.kernel.org # 7.2+
Suggested-by: Qu Wenruo <wqu@suse.com>
Assisted-by: LLM
Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
---
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@cs.unc.edu/

 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

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

[Qu Wenruo]

在 2026/10/12 02:28, Hengyu Liang 写道:
> 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@vger.kernel.org # 7.2+
> Suggested-by: Qu Wenruo <wqu@suse.com>
> Assisted-by: LLM
> Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
> ---
> 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.

Oh, sorry I didn't notice the read-modify-write behavior for the flags.

Great that you find out a way to use the old and new flags to detect if 
the compress flag is changed.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> 
> - Explain why the call fails.
> 
> v1:
> https://lore.kernel.org/linux-btrfs/20261010105425.165104-1-hengyul@cs.unc.edu/
> 
>   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