On 2021/11/24 19:22, Filipe Manana wrote:
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
The reason I didn't want to use dd is it doesn't have good enough
wrapper in fstests.
(Thus I guess that's also the reason why you use above command to
workaround it)
If you're really concerned about the block size, it can be easily
changed using "-b" option of pwrite, to archive the same behavior of the
dd command.
Furthermore, since we're reading from urandom, isn't it already ensured
we won't get blocked nor get short read until we're reading very large
blocks?
Thus a very basic filter on the pwrite should be enough to make sure we
really got page sized data written.
Thanks,
Qu
_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
