Re: [PATCH v2 2/4] btrfs: also validate compression levels in btrfs_compress_is_valid_type()
Qu Wenruo <[email protected]>
| Newsgroups | org.kernel.vger.linux-btrfs |
|---|---|
| Message-ID | <[email protected]> |
在 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 <[email protected]> > --- > 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); >