diff mbox series

[3/4] generic: test data corruption when blocksize < pagesize for mmaped data

Message ID 20220420083653.1031631-4-zlang@redhat.com (mailing list archive)
State New, archived
Headers show
Series several long time unmerged patches from zlang | expand

Commit Message

Zorro Lang April 20, 2022, 8:36 a.m. UTC
page_mkwrite() is used by filesystems to allocate blocks under a page
which is becoming writeably mmapped in some process' address space.
This allows a filesystem to return a page fault if there is not enough
space available, user exceeds quota or similar problem happens, rather
than silently discarding data later when writepage is called. However
VFS fails to call ->page_mkwrite() in all the cases where filesystems
need it when blocksize < pagesize.

At the moment page_mkwrite() is called, filesystem can allocate only
one block for the page if i_size < page size. Otherwise it would
create blocks beyond i_size which is generally undesirable. But later
at writepage() time, we also need to store data at offset 4095 but we
don't have block allocated for it.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---
 tests/generic/999     | 75 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  5 +++
 2 files changed, 80 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out

Comments

Darrick J. Wong April 20, 2022, 5:25 p.m. UTC | #1
On Wed, Apr 20, 2022 at 04:36:52PM +0800, Zorro Lang wrote:
> page_mkwrite() is used by filesystems to allocate blocks under a page
> which is becoming writeably mmapped in some process' address space.
> This allows a filesystem to return a page fault if there is not enough
> space available, user exceeds quota or similar problem happens, rather
> than silently discarding data later when writepage is called. However
> VFS fails to call ->page_mkwrite() in all the cases where filesystems
> need it when blocksize < pagesize.
> 
> At the moment page_mkwrite() is called, filesystem can allocate only
> one block for the page if i_size < page size. Otherwise it would
> create blocks beyond i_size which is generally undesirable. But later
> at writepage() time, we also need to store data at offset 4095 but we
> don't have block allocated for it.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
>  tests/generic/999     | 75 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/999.out |  5 +++
>  2 files changed, 80 insertions(+)
>  create mode 100755 tests/generic/999
>  create mode 100644 tests/generic/999.out
> 
> diff --git a/tests/generic/999 b/tests/generic/999
> new file mode 100755
> index 00000000..f1b65982
> --- /dev/null
> +++ b/tests/generic/999
> @@ -0,0 +1,75 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Red Hat Inc.  All Rights Reserved.
> +#
> +# FS QA Test 999
> +#
> +# Regression test for linux commit 90a80202 ("data corruption when
> +# blocksize < pagesize for mmaped data")
> +#
> +. ./common/preamble
> +_begin_fstest auto quick
> +
> +# Import common functions.
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs generic
> +_require_scratch
> +_require_block_device $SCRATCH_DEV
> +_require_xfs_io_command mmap "-s <size>"
> +_require_xfs_io_command mremap
> +_require_xfs_io_command truncate
> +_require_xfs_io_command mwrite
> +
> +sector_size=`_min_dio_alignment $SCRATCH_DEV`
> +page_size=$(get_page_size)
> +block_size=$((page_size/2))
> +if [ $sector_size -gt $block_size ];then
> +	_notrun "need sector size < page size"
> +fi
> +
> +unset MKFS_OPTIONS
> +# For save time, 512MiB is enough to reproduce bug
> +_scratch_mkfs_sized 536870912 $block_size >$seqres.full 2>&1 || _fail "mkfs failed"
> +_scratch_mount
> +
> +# take one block size space
> +$XFS_IO_PROG -f -t -c "pwrite 0 $block_size" $SCRATCH_MNT/testfile >>$seqres.full 2>&1
> +
> +# Fill all free space, dd can keep writing until no space. Note: if the fs
> +# isn't be full, this test will fail.
> +for ((i=0; i<2; i++));do
> +	dd if=/dev/zero of=$SCRATCH_MNT/full bs=$block_size >>$seqres.full 2>&1
> +	_scratch_cycle_mount
> +done

_fill_fs ?

Also, this test ought to check that we actually consumed all the
freespace and note that in the golden output so that a test runner can
tell the difference between "test preconditions not satisfied" vs.
"kernel needs patching".

--D

> +
> +# truncate 0
> +# pwrite -S 0x61 0 $block_size
> +# mmap -rw -s $((page_size * 2)) 0 $block_size
> +# mwrite -S 0x61 0 1  --> page_mkwrite() for index 0 is called
> +# truncate $((page_size * 2))
> +# mremap $((page_size * 2))
> +# mwrite -S 0x61 $((page_size - 1)) 1  --> [bug] no page_mkwrite() called
> +#
> +# If there's a bug, the last step will be killed by SIGBUS, and it won't
> +# write a "0x61" on the disk.
> +#
> +# Note: mremap maybe failed when memory load is heavy.
> +$XFS_IO_PROG -f \
> +             -c "truncate 0" \
> +             -c "pwrite -S 0x61 0 $block_size" \
> +             -c "mmap -rw -s $((page_size * 2)) 0 $block_size" \
> +             -c "mwrite -S 0x61 0 1" \
> +             -c "truncate $((page_size * 2))" \
> +             -c "mremap $((page_size * 2))" \
> +             -c "mwrite -S 0x61 $((page_size - 1)) 1" \
> +             $SCRATCH_MNT/testfile | _filter_xfs_io
> +
> +# we will see 0x61 written by "mwrite -S 0x61 0 1", but we shouldn't see one
> +# more 0x61 by the last mwrite (except this fs isn't be full, or a bug)
> +od -An -c $SCRATCH_MNT/testfile
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/999.out b/tests/generic/999.out
> new file mode 100644
> index 00000000..f1c59e83
> --- /dev/null
> +++ b/tests/generic/999.out
> @@ -0,0 +1,5 @@
> +QA output created by 999
> +   a   a   a   a   a   a   a   a   a   a   a   a   a   a   a   a
> +*
> +  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0
> +*
> -- 
> 2.31.1
>
diff mbox series

Patch

diff --git a/tests/generic/999 b/tests/generic/999
new file mode 100755
index 00000000..f1b65982
--- /dev/null
+++ b/tests/generic/999
@@ -0,0 +1,75 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 Red Hat Inc.  All Rights Reserved.
+#
+# FS QA Test 999
+#
+# Regression test for linux commit 90a80202 ("data corruption when
+# blocksize < pagesize for mmaped data")
+#
+. ./common/preamble
+_begin_fstest auto quick
+
+# Import common functions.
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_require_scratch
+_require_block_device $SCRATCH_DEV
+_require_xfs_io_command mmap "-s <size>"
+_require_xfs_io_command mremap
+_require_xfs_io_command truncate
+_require_xfs_io_command mwrite
+
+sector_size=`_min_dio_alignment $SCRATCH_DEV`
+page_size=$(get_page_size)
+block_size=$((page_size/2))
+if [ $sector_size -gt $block_size ];then
+	_notrun "need sector size < page size"
+fi
+
+unset MKFS_OPTIONS
+# For save time, 512MiB is enough to reproduce bug
+_scratch_mkfs_sized 536870912 $block_size >$seqres.full 2>&1 || _fail "mkfs failed"
+_scratch_mount
+
+# take one block size space
+$XFS_IO_PROG -f -t -c "pwrite 0 $block_size" $SCRATCH_MNT/testfile >>$seqres.full 2>&1
+
+# Fill all free space, dd can keep writing until no space. Note: if the fs
+# isn't be full, this test will fail.
+for ((i=0; i<2; i++));do
+	dd if=/dev/zero of=$SCRATCH_MNT/full bs=$block_size >>$seqres.full 2>&1
+	_scratch_cycle_mount
+done
+
+# truncate 0
+# pwrite -S 0x61 0 $block_size
+# mmap -rw -s $((page_size * 2)) 0 $block_size
+# mwrite -S 0x61 0 1  --> page_mkwrite() for index 0 is called
+# truncate $((page_size * 2))
+# mremap $((page_size * 2))
+# mwrite -S 0x61 $((page_size - 1)) 1  --> [bug] no page_mkwrite() called
+#
+# If there's a bug, the last step will be killed by SIGBUS, and it won't
+# write a "0x61" on the disk.
+#
+# Note: mremap maybe failed when memory load is heavy.
+$XFS_IO_PROG -f \
+             -c "truncate 0" \
+             -c "pwrite -S 0x61 0 $block_size" \
+             -c "mmap -rw -s $((page_size * 2)) 0 $block_size" \
+             -c "mwrite -S 0x61 0 1" \
+             -c "truncate $((page_size * 2))" \
+             -c "mremap $((page_size * 2))" \
+             -c "mwrite -S 0x61 $((page_size - 1)) 1" \
+             $SCRATCH_MNT/testfile | _filter_xfs_io
+
+# we will see 0x61 written by "mwrite -S 0x61 0 1", but we shouldn't see one
+# more 0x61 by the last mwrite (except this fs isn't be full, or a bug)
+od -An -c $SCRATCH_MNT/testfile
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/999.out b/tests/generic/999.out
new file mode 100644
index 00000000..f1c59e83
--- /dev/null
+++ b/tests/generic/999.out
@@ -0,0 +1,5 @@ 
+QA output created by 999
+   a   a   a   a   a   a   a   a   a   a   a   a   a   a   a   a
+*
+  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0  \0
+*