Re: [PATCH 2/2] btrfs: do not allow compression on nodatacow files

[Nikolay Borisov <nborisov@suse.com>]

On 15.04.22 г. 11:04 ч., Chung-Chiang Cheng wrote:
> Compression and nodatacow are mutually exclusive. A similar issue was
> fixed by commit f37c563bab429 ("btrfs: add missing check for nocow and
> compression inode flags"). Besides ioctl, there is another way to
> enable/disable/reset compression directly via xattr. The following
> steps will result in a invalid combination.
> 
>    $ touch bar
>    $ chattr +C bar
>    $ lsattr bar
>    ---------------C-- bar
>    $ setfattr -n btrfs.compression -v zstd bar
>    $ lsattr bar
>    --------c------C-- bar
> 
> To align with the logic in check_fsflags, nocompress will also be
> unacceptable after this patch, to prevent mix any compression-related
> options with nodatacow.
> 
>    $ touch bar
>    $ chattr +C bar
>    $ lsattr bar
>    ---------------C-- bar
>    $ setfattr -n btrfs.compression -v zstd bar
>    setfattr: bar: Invalid argument
>    $ setfattr -n btrfs.compression -v no bar
>    setfattr: bar: Invalid argument

Also send a test to verify we don't regress this behavior in the future.

> 
> Reported-by: Jayce Lin <jaycelin@synology.com>
> Signed-off-by: Chung-Chiang Cheng <cccheng@synology.com>
> ---
>   fs/btrfs/props.c | 16 +++++++++++-----
>   fs/btrfs/props.h |  3 ++-
>   fs/btrfs/xattr.c |  2 +-
>   3 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
> index 1a6d2d5b4b33..5a6f87744c28 100644
> --- a/fs/btrfs/props.c
> +++ b/fs/btrfs/props.c
> @@ -17,7 +17,8 @@ static DEFINE_HASHTABLE(prop_handlers_ht, BTRFS_PROP_HANDLERS_HT_BITS);
>   struct prop_handler {
>   	struct hlist_node node;
>   	const char *xattr_name;
> -	int (*validate)(const char *value, size_t len);
> +	int (*validate)(const struct btrfs_inode *inode, const char *value,
> +			size_t len);
>   	int (*apply)(struct inode *inode, const char *value, size_t len);
>   	const char *(*extract)(struct inode *inode);
>   	int inheritable;
> @@ -55,7 +56,8 @@ find_prop_handler(const char *name,
>   	return NULL;
>   }
>   
> -int btrfs_validate_prop(const char *name, const char *value, size_t value_len)
> +int btrfs_validate_prop(const struct btrfs_inode *inode, const char *name,
> +			const char *value, size_t value_len)
>   {
>   	const struct prop_handler *handler;
>   
> @@ -69,7 +71,7 @@ int btrfs_validate_prop(const char *name, const char *value, size_t value_len)
>   	if (value_len == 0)
>   		return 0;
>   
> -	return handler->validate(value, value_len);
> +	return handler->validate(inode, value, value_len);
>   }
>   
>   int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
> @@ -252,8 +254,12 @@ int btrfs_load_inode_props(struct inode *inode, struct btrfs_path *path)
>   	return ret;
>   }
>   
> -static int prop_compression_validate(const char *value, size_t len)
> +static int prop_compression_validate(const struct btrfs_inode *inode,
> +				     const char *value, size_t len)
>   {
> +	if (!btrfs_inode_can_compress(inode))
> +		return -EINVAL;
> +
>   	if (!value)
>   		return 0;
>   
> @@ -364,7 +370,7 @@ static int inherit_props(struct btrfs_trans_handle *trans,
>   		 * This is not strictly necessary as the property should be
>   		 * valid, but in case it isn't, don't propagate it further.
>   		 */
> -		ret = h->validate(value, strlen(value));
> +		ret = h->validate(BTRFS_I(inode), value, strlen(value));
>   		if (ret)
>   			continue;
>   
> diff --git a/fs/btrfs/props.h b/fs/btrfs/props.h
> index 40b2c65b518c..2b2ac15ab788 100644
> --- a/fs/btrfs/props.h
> +++ b/fs/btrfs/props.h
> @@ -13,7 +13,8 @@ void __init btrfs_props_init(void);
>   int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
>   		   const char *name, const char *value, size_t value_len,
>   		   int flags);
> -int btrfs_validate_prop(const char *name, const char *value, size_t value_len);
> +int btrfs_validate_prop(const struct btrfs_inode *inode, const char *name,
> +			const char *value, size_t value_len);
>   
>   int btrfs_load_inode_props(struct inode *inode, struct btrfs_path *path);
>   
> diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
> index 99abf41b89b9..9632d0ff2038 100644
> --- a/fs/btrfs/xattr.c
> +++ b/fs/btrfs/xattr.c
> @@ -403,7 +403,7 @@ static int btrfs_xattr_handler_set_prop(const struct xattr_handler *handler,
>   	struct btrfs_root *root = BTRFS_I(inode)->root;
>   
>   	name = xattr_full_name(handler, name);
> -	ret = btrfs_validate_prop(name, value, size);
> +	ret = btrfs_validate_prop(BTRFS_I(inode), name, value, size);
>   	if (ret)
>   		return ret;
>