On 2019/06/06 17:01, Johannes Thumshirn wrote:
> Nikolay reported the following KASAN splat when running btrfs/048:
>
(snip)
>
> This is caused by supplying a too short compression value ('lz') in the
> test-case and comparing it to 'lzo' with strncmp() and a length of 3.
> strncmp() read past the 'lz' when looking for the 'o' and thus caused an
> out-of-bounds read.
>
> Introduce a new check 'btrfs_compress_is_valid_type()' which not only
> checks the user-supplied value against known compression types, but also
> employs checks for too short values.
>
> Fixes: 272e5326c783 ("btrfs: prop: fix vanished compression property after failed set")
> Reported-by: Nikolay Borisov <nborisov@xxxxxxxx>
> Signed-off-by: Johannes Thumshirn <jthumshirn@xxxxxxx>
> ---
> fs/btrfs/compression.c | 16 ++++++++++++++++
> fs/btrfs/compression.h | 1 +
> fs/btrfs/props.c | 6 +-----
> 3 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 66e21a4e9ea2..d21ae92c172c 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -43,6 +43,22 @@ const char* btrfs_compress_type2str(enum btrfs_compression_type type)
> return NULL;
> }
>
> +bool btrfs_compress_is_valid_type(const char *str, size_t len)
> +{
> + int i;
> +
> + for (i = 1; i < ARRAY_SIZE(btrfs_compress_types); i++) {
> + size_t comp_len = strlen(btrfs_compress_types[i]);
> +
> + if (comp_len != len)
Should this be "if (comp_len > len)"?
a7164fa4e055 ("btrfs: prepare for extensions in compression options")
allowed compression property to have compression options. If we have the
options, we will have "len" larger than "comp_len".
> + continue;
> +
> + if (!strncmp(btrfs_compress_types[i], str, comp_len))
> + return true;
> + }
> + return false;
> +}
> +
> static int btrfs_decompress_bio(struct compressed_bio *cb);
>
> static inline int compressed_bio_size(struct btrfs_fs_info *fs_info,
> diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
> index 191e5f4e3523..2035b8eb1290 100644
> --- a/fs/btrfs/compression.h
> +++ b/fs/btrfs/compression.h
> @@ -173,6 +173,7 @@ extern const struct btrfs_compress_op btrfs_lzo_compress;
> extern const struct btrfs_compress_op 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);
>
> int btrfs_compress_heuristic(struct inode *inode, u64 start, u64 end);
>
> diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
> index a9e2e66152ee..af109c0ba720 100644
> --- a/fs/btrfs/props.c
> +++ b/fs/btrfs/props.c
> @@ -257,11 +257,7 @@ static int prop_compression_validate(const char *value, size_t len)
> if (!value)
> return 0;
>
> - if (!strncmp("lzo", value, 3))
> - return 0;
> - else if (!strncmp("zlib", value, 4))
> - return 0;
> - else if (!strncmp("zstd", value, 4))
> + if (btrfs_compress_is_valid_type(value, len))
> return 0;
>
> return -EINVAL;
>
