[PATCH] fstests: test fsync on overlayfs merged directory

[PATCH] fstests: test fsync on overlayfs merged directory

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Test that calling fsync against a file using the merged directory does
not result in a crash nor fails unexpectedly.

This is motivated by a change in overlayfs that resulted in a crash
(invalid memory access) when the lower or upper directory belonged to
a btrfs file system. The overlayfs change came in commit 4bacc9c9234
(overlayfs: Make f_path always point to the overlay and f_inode to the
underlay). At the moment there are two patches in the linux-fsdevel
and linux-btrfs mailing lists to fix this problem:

  * vfs: add file_dentry()
  * Btrfs: fix crash/invalid memory access on fsync when using overlayfs

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 tests/overlay/002     | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/002.out |  3 ++
 tests/overlay/group   |  1 +
 3 files changed, 80 insertions(+)
 create mode 100755 tests/overlay/002
 create mode 100644 tests/overlay/002.out

diff --git a/tests/overlay/002 b/tests/overlay/002
new file mode 100755
index 0000000..4f82f60
--- /dev/null
+++ b/tests/overlay/002
@@ -0,0 +1,76 @@
+#! /bin/bash
+# FS QA Test 002
+#
+# Test that calling fsync against a file using the merged directory does not
+# result in a crash nor fails unexpectedly.
+#
+# This is motivated by a change in overlayfs that resulted in a crash (invalid
+# memory access) when the lower or upper directory belonged to a btrfs file
+# system.
+#
+#-----------------------------------------------------------------------
+#
+# Copyright (C) 2016 SUSE Linux Products GmbH. All Rights Reserved.
+# Author: Filipe Manana <fdmanana@suse.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+
+# Remove all files from previous tests
+_scratch_mkfs
+
+# Create our test file.
+#lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
+lowerdir=$SCRATCH_DEV/$OVERLAY_UPPER_DIR
+mkdir -p $lowerdir
+touch $lowerdir/foobar
+sync
+
+_scratch_mount
+
+# Write some data to our file and fsync it, using the merged directory path.
+# This should work and not result in a crash.
+$XFS_IO_PROG -c "pwrite 0 64k" -c "fsync" $SCRATCH_MNT/foobar | _filter_xfs_io
+
+# success, all done
+status=0
+exit
diff --git a/tests/overlay/002.out b/tests/overlay/002.out
new file mode 100644
index 0000000..666e61e
--- /dev/null
+++ b/tests/overlay/002.out
@@ -0,0 +1,3 @@
+QA output created by 002
+wrote 65536/65536 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/overlay/group b/tests/overlay/group
index 5056c3b..84d164e 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -4,3 +4,4 @@
 # - comment line before each group is "new" description
 #
 001 auto quick copyup
+002 auto quick metadata
-- 
2.7.0.rc3

[PATCH v2] fstests: test fsync on overlayfs merged directory

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Test that calling fsync against a file using the merged directory does
not result in a crash nor fails unexpectedly.

This is motivated by a change in overlayfs that resulted in a crash
(invalid memory access) when the lower or upper directory belonged to
a btrfs file system. The overlayfs change came in commit 4bacc9c9234
(overlayfs: Make f_path always point to the overlay and f_inode to the
underlay). At the moment there are two patches in the linux-fsdevel
and linux-btrfs mailing lists to fix this problem:

  * vfs: add file_dentry()
  * Btrfs: fix crash/invalid memory access on fsync when using overlayfs

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---

V2: Removed leftover comment from debugging.

 tests/overlay/002     | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/002.out |  3 +++
 tests/overlay/group   |  1 +
 3 files changed, 78 insertions(+)
 create mode 100755 tests/overlay/002
 create mode 100644 tests/overlay/002.out

diff --git a/tests/overlay/002 b/tests/overlay/002
new file mode 100755
index 0000000..e5aa610
--- /dev/null
+++ b/tests/overlay/002
@@ -0,0 +1,74 @@
+#! /bin/bash
+# FS QA Test 002
+#
+# Test that calling fsync against a file using the merged directory does not
+# result in a crash nor fails unexpectedly.
+#
+# This is motivated by a change in overlayfs that resulted in a crash (invalid
+# memory access) when the lower or upper directory belonged to a btrfs file
+# system.
+#
+#-----------------------------------------------------------------------
+#
+# Copyright (C) 2016 SUSE Linux Products GmbH. All Rights Reserved.
+# Author: Filipe Manana <fdmanana@suse.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+
+# Remove all files from previous tests
+_scratch_mkfs
+
+# Create our test file.
+lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
+mkdir -p $lowerdir
+touch $lowerdir/foobar
+
+_scratch_mount
+
+# Write some data to our file and fsync it, using the merged directory path.
+# This should work and not result in a crash.
+$XFS_IO_PROG -c "pwrite 0 64k" -c "fsync" $SCRATCH_MNT/foobar | _filter_xfs_io
+
+# success, all done
+status=0
+exit
diff --git a/tests/overlay/002.out b/tests/overlay/002.out
new file mode 100644
index 0000000..666e61e
--- /dev/null
+++ b/tests/overlay/002.out
@@ -0,0 +1,3 @@
+QA output created by 002
+wrote 65536/65536 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/overlay/group b/tests/overlay/group
index 5056c3b..84d164e 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -4,3 +4,4 @@
 # - comment line before each group is "new" description
 #
 001 auto quick copyup
+002 auto quick metadata
-- 
2.7.0.rc3

Re: [PATCH v2] fstests: test fsync on overlayfs merged directory

[Eryu Guan]

On Mon, Mar 21, 2016 at 05:50:25PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Test that calling fsync against a file using the merged directory does
> not result in a crash nor fails unexpectedly.
> 
> This is motivated by a change in overlayfs that resulted in a crash
> (invalid memory access) when the lower or upper directory belonged to
> a btrfs file system. The overlayfs change came in commit 4bacc9c9234
> (overlayfs: Make f_path always point to the overlay and f_inode to the
> underlay). At the moment there are two patches in the linux-fsdevel
> and linux-btrfs mailing lists to fix this problem:
> 
>   * vfs: add file_dentry()
>   * Btrfs: fix crash/invalid memory access on fsync when using overlayfs
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
> 
> V2: Removed leftover comment from debugging.
> 
>  tests/overlay/002     | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/002.out |  3 +++
>  tests/overlay/group   |  1 +
>  3 files changed, 78 insertions(+)
>  create mode 100755 tests/overlay/002
>  create mode 100644 tests/overlay/002.out
> 
> diff --git a/tests/overlay/002 b/tests/overlay/002
> new file mode 100755
> index 0000000..e5aa610
> --- /dev/null
> +++ b/tests/overlay/002
> @@ -0,0 +1,74 @@
> +#! /bin/bash
> +# FS QA Test 002
> +#
> +# Test that calling fsync against a file using the merged directory does not
> +# result in a crash nor fails unexpectedly.
> +#
> +# This is motivated by a change in overlayfs that resulted in a crash (invalid
> +# memory access) when the lower or upper directory belonged to a btrfs file
> +# system.
> +#
> +#-----------------------------------------------------------------------
> +#
> +# Copyright (C) 2016 SUSE Linux Products GmbH. All Rights Reserved.
> +# Author: Filipe Manana <fdmanana@suse.com>
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs generic

The supported fs should be "overlay"? overlay/001 has the same issue
though.

Looks good to me otherwise, test passed with XFS as underlying fs and
test crashed v4.5 kernel with btrfs as underlying fs, as expected.

Reviewed-by: Eryu Guan <eguan@redhat.com>

Re: [PATCH v2] fstests: test fsync on overlayfs merged directory

[Filipe Manana]

On Tue, Mar 22, 2016 at 3:00 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Mon, Mar 21, 2016 at 05:50:25PM +0000, fdmanana@kernel.org wrote:
>> From: Filipe Manana <fdmanana@suse.com>
>>
>> Test that calling fsync against a file using the merged directory does
>> not result in a crash nor fails unexpectedly.
>>
>> This is motivated by a change in overlayfs that resulted in a crash
>> (invalid memory access) when the lower or upper directory belonged to
>> a btrfs file system. The overlayfs change came in commit 4bacc9c9234
>> (overlayfs: Make f_path always point to the overlay and f_inode to the
>> underlay). At the moment there are two patches in the linux-fsdevel
>> and linux-btrfs mailing lists to fix this problem:
>>
>>   * vfs: add file_dentry()
>>   * Btrfs: fix crash/invalid memory access on fsync when using overlayfs
>>
>> Signed-off-by: Filipe Manana <fdmanana@suse.com>
>> ---
>>
>> V2: Removed leftover comment from debugging.
>>
>>  tests/overlay/002     | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/overlay/002.out |  3 +++
>>  tests/overlay/group   |  1 +
>>  3 files changed, 78 insertions(+)
>>  create mode 100755 tests/overlay/002
>>  create mode 100644 tests/overlay/002.out
>>
>> diff --git a/tests/overlay/002 b/tests/overlay/002
>> new file mode 100755
>> index 0000000..e5aa610
>> --- /dev/null
>> +++ b/tests/overlay/002
>> @@ -0,0 +1,74 @@
>> +#! /bin/bash
>> +# FS QA Test 002
>> +#
>> +# Test that calling fsync against a file using the merged directory does not
>> +# result in a crash nor fails unexpectedly.
>> +#
>> +# This is motivated by a change in overlayfs that resulted in a crash (invalid
>> +# memory access) when the lower or upper directory belonged to a btrfs file
>> +# system.
>> +#
>> +#-----------------------------------------------------------------------
>> +#
>> +# Copyright (C) 2016 SUSE Linux Products GmbH. All Rights Reserved.
>> +# Author: Filipe Manana <fdmanana@suse.com>
>> +#
>> +# This program is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU General Public License as
>> +# published by the Free Software Foundation.
>> +#
>> +# This program is distributed in the hope that it would be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, write the Free Software Foundation,
>> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>> +#-----------------------------------------------------------------------
>> +#
>> +
>> +seq=`basename $0`
>> +seqres=$RESULT_DIR/$seq
>> +echo "QA output created by $seq"
>> +
>> +here=`pwd`
>> +tmp=/tmp/$$
>> +status=1     # failure is the default!
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +_cleanup()
>> +{
>> +     cd /
>> +     rm -f $tmp.*
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/filter
>> +
>> +# remove previous $seqres.full before test
>> +rm -f $seqres.full
>> +
>> +# real QA test starts here
>> +_supported_fs generic
>
> The supported fs should be "overlay"? overlay/001 has the same issue
> though.

Yeah, I copied it from 001. And that's a question I made myself but
forgot later to investigate. Since you authored test 001, can you
confirm if it's a mistake or is it really supposed to be 'overlay'?

Thanks

>
> Looks good to me otherwise, test passed with XFS as underlying fs and
> test crashed v4.5 kernel with btrfs as underlying fs, as expected.
>
> Reviewed-by: Eryu Guan <eguan@redhat.com>

Re: [PATCH v2] fstests: test fsync on overlayfs merged directory

[Eryu Guan]

On Tue, Mar 22, 2016 at 11:07:06AM +0000, Filipe Manana wrote:
> On Tue, Mar 22, 2016 at 3:00 AM, Eryu Guan <eguan@redhat.com> wrote:
...
> >> +
> >> +# real QA test starts here
> >> +_supported_fs generic
> >
> > The supported fs should be "overlay"? overlay/001 has the same issue
> > though.
> 
> Yeah, I copied it from 001. And that's a question I made myself but
> forgot later to investigate. Since you authored test 001, can you
> confirm if it's a mistake or is it really supposed to be 'overlay'?

It's my mistake in overlay/001, it should be overlay.

Thanks,
Eryu

[PATCH v3] fstests: test fsync on overlayfs merged directory

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Test that calling fsync against a file using the merged directory does
not result in a crash nor fails unexpectedly.

This is motivated by a change in overlayfs that resulted in a crash
(invalid memory access) when the lower or upper directory belonged to
a btrfs file system. The overlayfs change came in commit 4bacc9c9234
(overlayfs: Make f_path always point to the overlay and f_inode to the
underlay). At the moment there are two patches in the linux-fsdevel
and linux-btrfs mailing lists to fix this problem:

  * vfs: add file_dentry()
  * Btrfs: fix crash/invalid memory access on fsync when using overlayfs

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Eryu Guan <eguan@redhat.com>
---

V2: Removed leftover comment from debugging.
V3: Change supperted fs type from generic to overlay.

 tests/overlay/002     | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/002.out |  3 +++
 tests/overlay/group   |  1 +
 3 files changed, 78 insertions(+)
 create mode 100755 tests/overlay/002
 create mode 100644 tests/overlay/002.out

diff --git a/tests/overlay/002 b/tests/overlay/002
new file mode 100755
index 0000000..ec7874e
--- /dev/null
+++ b/tests/overlay/002
@@ -0,0 +1,74 @@
+#! /bin/bash
+# FS QA Test 002
+#
+# Test that calling fsync against a file using the merged directory does not
+# result in a crash nor fails unexpectedly.
+#
+# This is motivated by a change in overlayfs that resulted in a crash (invalid
+# memory access) when the lower or upper directory belonged to a btrfs file
+# system.
+#
+#-----------------------------------------------------------------------
+#
+# Copyright (C) 2016 SUSE Linux Products GmbH. All Rights Reserved.
+# Author: Filipe Manana <fdmanana@suse.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs overlay
+_supported_os Linux
+_require_scratch
+
+# Remove all files from previous tests
+_scratch_mkfs
+
+# Create our test file.
+lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
+mkdir -p $lowerdir
+touch $lowerdir/foobar
+
+_scratch_mount
+
+# Write some data to our file and fsync it, using the merged directory path.
+# This should work and not result in a crash.
+$XFS_IO_PROG -c "pwrite 0 64k" -c "fsync" $SCRATCH_MNT/foobar | _filter_xfs_io
+
+# success, all done
+status=0
+exit
diff --git a/tests/overlay/002.out b/tests/overlay/002.out
new file mode 100644
index 0000000..666e61e
--- /dev/null
+++ b/tests/overlay/002.out
@@ -0,0 +1,3 @@
+QA output created by 002
+wrote 65536/65536 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/overlay/group b/tests/overlay/group
index 5056c3b..84d164e 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -4,3 +4,4 @@
 # - comment line before each group is "new" description
 #
 001 auto quick copyup
+002 auto quick metadata
-- 
2.7.0.rc3