On 2018年05月15日 16:05, Nikolay Borisov wrote:
>
>
> On 15.05.2018 10:36, Qu Wenruo wrote:
>> Unlike zlib decompression, lzo decompression doesn't need any
>> initialization, thus we can't detect early corruption from
>> initialization.
>>
>> However for lzo compressed extent, its first 4bytes records the real
>> unaligned compressed data size.
>> We could use this as a clue, since any compressed extent should not
>> exceed 128K, thus if we find such compressed data length, we are sure
>> it's corrupted, then no need to continue decompression.
>>
>> Normally, such problem won't really bother anyone, as compression relies
>> on dataCoW and data csum, which means normally such corruption should be
>> detect by data csum before going into compression.
>> However due to a bug in compression condition, it's possible to create
>> compressed extent without csum.
>>
>> So we still need to do extra check for lzo just in case the compressed
>> data is corrupted.
>>
>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>> ---
>> lease note that, even with the binary dump of corrupted extent provided
>> by the original reporter, James Harvey, I can only reproduce the "decompress
>> failed" error message, but not the serious memory corruption followed.
>> So there must be something missing, maybe we need to double check both
>> btrfs lzo caller and kernel lzo lib.
>>
>> But anyway, making btrfs lzo compression a little more robust is never a
>> bad thing.
>> ---
>> fs/btrfs/compression.h | 1 +
>> fs/btrfs/lzo.c | 4 ++++
>> 2 files changed, 5 insertions(+)
>>
>> diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
>> index cc605f7b23fb..317703d9b073 100644
>> --- a/fs/btrfs/compression.h
>> +++ b/fs/btrfs/compression.h
>> @@ -6,6 +6,7 @@
>> #ifndef BTRFS_COMPRESSION_H
>> #define BTRFS_COMPRESSION_H
>>
>> +#include <linux/sizes.h>
>
> Stray include otherwise:
Surprisingly that's really needed.
As in compression.h we uses SZ_*, while we didn't include that header.
It's other *.c files get that header included first so no compiler error.
However in this case, lzo.c is only including compression.h, no other
headers including sizes.h, so it will cause compiler error.
That's to fix it.
Thanks,
Qu
>
> Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx>
>
>> /*
>> * We want to make sure that amount of RAM required to uncompress an extent is
>> * reasonable, so we limit the total size in ram of a compressed extent to
>> diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
>> index 0667ea07f766..7ae2c0925770 100644
>> --- a/fs/btrfs/lzo.c
>> +++ b/fs/btrfs/lzo.c
>> @@ -271,6 +271,10 @@ static int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
>>
>> data_in = kmap(pages_in[0]);
>> tot_len = read_compress_length(data_in);
>> + if (tot_len > BTRFS_MAX_COMPRESSED) {
>> + ret = -EIO;
>> + goto done;
>> + }
>>
>> tot_in = LZO_LEN;
>> in_offset = LZO_LEN;
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
