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

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 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