On Wed, Nov 24, 2021 at 9:24 AM Qu Wenruo <wqu@xxxxxxxx> wrote:
>
> Since kernel commit d4088803f511 ("btrfs: subpage: make lzo_compress_pages()
> compatible"), lzo compression no longer respects the max compressed page
> limit, and can cause kernel crash.
>
> The upstream fix is 6f019c0e0193 ("btrfs: fix a out-of-bound access in
> copy_compressed_data_to_page()").
>
> This patch will add such regression test for all possible compress-force
> mount options, including lzo, zstd and zlib.
>
> And since we're here, also make sure the content of the file matches
> after a mount cycle.
>
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> ---
> Changelog:
> v2:
> - Also test zlib and zstd
> - Add file content verification check
> ---
> tests/btrfs/049 | 56 +++++++++++++++++++++++++++++++++++++++++++++
> tests/btrfs/049.out | 6 +++++
> 2 files changed, 62 insertions(+)
> create mode 100755 tests/btrfs/049
>
> diff --git a/tests/btrfs/049 b/tests/btrfs/049
> new file mode 100755
> index 00000000..264e576f
> --- /dev/null
> +++ b/tests/btrfs/049
> @@ -0,0 +1,56 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2021 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FS QA Test 049
> +#
> +# Test if btrfs will crash when using compress-force mount option against
> +# incompressible data
> +#
> +. ./common/preamble
> +_begin_fstest auto quick compress dangerous
> +
> +# Override the default cleanup function.
> +_cleanup()
> +{
> + cd /
> + rm -r -f $tmp.*
> +}
> +
> +# Import common functions.
> +. ./common/filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs btrfs
> +_require_scratch
> +
> +pagesize=$(get_page_size)
> +workload()
> +{
> + local compression
> + compression=$1
Could be shorter by doing it in one step:
local compression=$1
> +
> + echo "=== Testing compress-force=$compression ==="
> + _scratch_mkfs -s "$pagesize">> $seqres.full
> + _scratch_mount -o compress-force="$compression"
> + $XFS_IO_PROG -f -c "pwrite -i /dev/urandom 0 $pagesize" \
> + "$SCRATCH_MNT/file" > /dev/null
> + md5sum "$SCRATCH_MNT/file" > "$tmp.$compression"
This doesn't really check if everything we asked to write was actually written.
pwrite(2), write(2), etc, return the number of bytes written, which
can be less than what we asked for.
And using the checksum verification in that way, we are only checking
that what we had before unmounting is the same after mounting again.
I.e. we are not checking that what was actually written is what we
have asked for.
We could do something like:
data=$(dd count=4096 bs=1 if=/dev/urandom)
echo -n "$data" > file
_scratch_cycle_mount
check that the the md5sum of file is the same as: echo -n "$data" | md5sum
As it is, the test is enough to trigger the original bug, but having
such additional checks is more valuable IMO for the long run, and can
help prevent other types of regressions too.
Thanks Qu.
> +
> + # When unpatched, compress-force=lzo would crash at data writeback
> + _scratch_cycle_mount
> +
> + # Make sure the content matches
> + md5sum -c "$tmp.$compression" | _filter_scratch
> + _scratch_unmount
> +}
> +
> +workload lzo
> +workload zstd
> +workload zlib
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/049.out b/tests/btrfs/049.out
> index cb0061b3..258f3c09 100644
> --- a/tests/btrfs/049.out
> +++ b/tests/btrfs/049.out
> @@ -1 +1,7 @@
> QA output created by 049
> +=== Testing compress-force=lzo ===
> +SCRATCH_MNT/file: OK
> +=== Testing compress-force=zstd ===
> +SCRATCH_MNT/file: OK
> +=== Testing compress-force=zlib ===
> +SCRATCH_MNT/file: OK
> --
> 2.34.0
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
