Re: [PATCH v2 2/4] btrfs: also validate compression levels in btrfs_compress_is_valid_type()

[Qu Wenruo <wqu@suse.com>]

在 2026/8/9 11:20, koraynilay 写道:
> Change btrfs_compress_is_valid_type() to use btrfs_match_compress_type()
> and btrfs_compress_str2level() instead of a simple strncmp, which would
> validate even incorrect strings (e.g. "zstd:invalid" or "zstdinvalid").
> 
> This also makes the function validate levels in the same way the
> compress= option gets parsed, allowing bigger or smaller values, but
> still clamping them to the min or max supported.
> 
> Furthermore, remove the len parameter, since now it requires a
> NUL-terminated string because of btrfs_compress_str2level(); this change
> is fine because btrfs_compress_is_valid_type() is used only once in
> props.c
> 
> Assisted-by: Gemini:3.1-pro antigravity-cli-1.1.5
> Signed-off-by: koraynilay <koray.fra@gmail.com>
> ---
>   fs/btrfs/compression.c | 11 ++++++++---
>   fs/btrfs/compression.h |  2 +-
>   2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 58138f300b58..833c5f45afec 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -70,18 +70,23 @@ static struct compressed_bio *alloc_compressed_bio(struct btrfs_inode *inode,
>   	return to_compressed_bio(bbio);
>   }
>   
> -bool btrfs_compress_is_valid_type(const char *str, size_t len)
> +bool btrfs_compress_is_valid_type(const char *str)

Please make sure every commit compiles, this will easily break bisection.

You're changing a function prototype and implementation without 
modifying any callers, this fails compiling.

Furthermore, I do not think it's a good idea to just rely on the strlen().

E.g. if a crafted image removing the last terminating NUL, relying 
strlen() can easily go beyond the expected string.

I think the change to remove @len is going to reduce the robustness of 
the original code.

Thanks,
Qu

>   {
> +	size_t len = strlen(str);
>   	int i;
>   
>   	for (i = 1; i < ARRAY_SIZE(btrfs_compress_types); i++) {
>   		size_t comp_len = strlen(btrfs_compress_types[i]);
> +		const char *comp_type = btrfs_compress_types[i];
> +		int tmp_level;
>   
>   		if (len < comp_len)
>   			continue;
>   
> -		if (!strncmp(btrfs_compress_types[i], str, comp_len))
> -			return true;
> +		if (btrfs_match_compress_type(str, comp_type, true)) {
> +			if (btrfs_compress_str2level(i, str + comp_len, &tmp_level) == 0)
> +				return true;
> +		}
>   	}
>   	return false;
>   }
> diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
> index e67ba47b4cdc..c63bed9f4152 100644
> --- a/fs/btrfs/compression.h
> +++ b/fs/btrfs/compression.h
> @@ -132,7 +132,7 @@ extern const struct btrfs_compress_levels btrfs_lzo_compress;
>   extern const struct btrfs_compress_levels btrfs_zstd_compress;
>   
>   const char* btrfs_compress_type2str(enum btrfs_compression_type type);
> -bool btrfs_compress_is_valid_type(const char *str, size_t len);
> +bool btrfs_compress_is_valid_type(const char *str);
>   
>   int btrfs_compress_heuristic(struct btrfs_inode *inode, u64 start, u64 end);
>