overlay: create directory over deleted whiteout
PATCH 1/1 overlay: create directory over deleted whiteout
There's a bug in the overlayfs implementation starting from the very first merged version that may cause an Oops of various forms if a directory is created over a whiteout dentry, but the actual whiteout on the upper layer was removed to the directory creation.
Reported by: kaixuxia <[email protected]>
--- tests/overlay/062 | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/overlay/062.out | 2 ++ tests/overlay/group | 1 + 3 files changed, 63 insertions(+) create mode 100644 tests/overlay/062 create mode 100644 tests/overlay/062.out
tests/overlay/062 new file +60 -0
@@ -0,0 +1,60 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2018 Red Hat Inc. All Rights Reserved. +# +# FS QA Test 062 +# +# Create dir over cached negative dentry, but whiteout removed from upper +# +# The following kernel commit fixed the kernel crash: ??? +# +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 test file +lowerdir=${OVL_BASE_SCRATCH_MNT}/${OVL_LOWER} +upperdir=${OVL_BASE_SCRATCH_MNT}/${OVL_UPPER} +mkdir -p $lowerdir +touch ${lowerdir}/file + +_scratch_mount + +rm ${SCRATCH_MNT}/file +ls -l ${SCRATCH_MNT}/file > /dev/null 2>&1 +rm ${upperdir}/file +mkdir ${SCRATCH_MNT}/file > /dev/null 2>&1 + +# unmount overlayfs +$UMOUNT_PROG $SCRATCH_MNT + +echo "Silence is golden" +# success, all done +status=0 +exit
tests/overlay/062.out new file +2 -0
@@ -0,0 +1,2 @@ +QA output created by 062 +Silence is golden
tests/overlay/group +1 -0
@@ -64,3 +64,4 @@ 059 auto quick copyup 060 auto quick metacopy 061 auto quick copyup +062 auto quick whiteout
Signature · 1 line
2.14.3
Quoted 9 lines · Miklos Szeredi
There's a bug in the overlayfs implementation starting from the very first merged version that may cause an Oops of various forms if a directory is created over a whiteout dentry, but the actual whiteout on the upper layer was removed to the directory creation.
Reported by: kaixuxia <[email protected]>
Signed-off-by Miklos Szeredi <[email protected]>---
Looks good. A bit of commentary could be useful...
Quoted 51 lines
tests/overlay/062 | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/overlay/062.out | 2 ++ tests/overlay/group | 1 + 3 files changed, 63 insertions(+) create mode 100644 tests/overlay/062 create mode 100644 tests/overlay/062.outtests/overlay/062 new file +38
-0@@ -0,0 +1,60 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2018 Red Hat Inc. All Rights Reserved. +# +# FS QA Test 062 +# +# Create dir over cached negative dentry, but whiteout removed from upper +# +# The following kernel commit fixed the kernel crash: ??? +# +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
tests/overlay/062 line 48
Quoted 10 lines
tests/overlay/062 new file +10
-0+_scratch_mkfs + +# Create test file +lowerdir=${OVL_BASE_SCRATCH_MNT}/${OVL_LOWER} +upperdir=${OVL_BASE_SCRATCH_MNT}/${OVL_UPPER} +mkdir -p $lowerdir +touch ${lowerdir}/file + +_scratch_mount +
# Create whiteout and populate dcache with negative dentry
tests/overlay/062 line 50
Quoted 2 lines
tests/overlay/062 new file +2
-0+rm ${SCRATCH_MNT}/file +ls -l ${SCRATCH_MNT}/file > /dev/null 2>&1
# Remove whiteout and try to create dir over negative dentry
tests/overlay/062 line 55
Quoted 5 lines
tests/overlay/062 new file +5
-0+rm ${upperdir}/file +mkdir ${SCRATCH_MNT}/file > /dev/null 2>&1 + +# unmount overlayfs +$UMOUNT_PROG $SCRATCH_MNT
Umount of scratch is not needed at the end of the test.
Thanks,
Amir.
Quoted 7 lines · Amir Goldstein
Quoted 6 lines · Miklos Szeredi
There's a bug in the overlayfs implementation starting from the very first merged version that may cause an Oops of various forms if a directory is created over a whiteout dentry, but the actual whiteout on the upper layer was removed to the directory creation.
Quoted 5 lines
Quoted 3 lines
Reported by: kaixuxia <[email protected]>
Signed-off-by Miklos Szeredi <[email protected]>---
Looks good. A bit of commentary could be useful...
Okay.
Quoted 5 lines
tests/overlay/062 line 55
Quoted 3 lines
tests/overlay/062 new file +3
-0+ +# unmount overlayfs +$UMOUNT_PROG $SCRATCH_MNTUmount of scratch is not needed at the end of the test.
It's not strictly needed, but gives additional assurance that things hadn't gone bad (after the oops umount returns with EBUSY).
Thanks,
Miklos
Quoted 17 lines · Miklos Szeredi
Quoted 7 lines · Amir Goldstein
Quoted 6 lines · Miklos Szeredi
There's a bug in the overlayfs implementation starting from the very first merged version that may cause an Oops of various forms if a directory is created over a whiteout dentry, but the actual whiteout on the upper layer was removed to the directory creation.
Quoted 5 lines
Quoted 3 lines
Reported by: kaixuxia <[email protected]>
Signed-off-by Miklos Szeredi <[email protected]>---
Looks good. A bit of commentary could be useful...
Okay.
Quoted 9 lines
Quoted 5 lines
tests/overlay/062 line 55
Quoted 3 lines
tests/overlay/062 new file +3
-0+ +# unmount overlayfs +$UMOUNT_PROG $SCRATCH_MNTUmount of scratch is not needed at the end of the test.
It's not strictly needed, but gives additional assurance that things hadn't gone bad (after the oops umount returns with EBUSY).
But the test harness unmount the scratch mount anyway
and if you have fsck.overlay installed it also runs fsck.overay on the layers after each test.
The question is why do you need the umount command in the test itself? Is the output/result different without the explicit umount command in the test?
Thanks,
Amir.
Quoted 28 lines · Amir Goldstein
Quoted 17 lines · Miklos Szeredi
Quoted 7 lines · Amir Goldstein
Quoted 6 lines · Miklos Szeredi
There's a bug in the overlayfs implementation starting from the very first merged version that may cause an Oops of various forms if a directory is created over a whiteout dentry, but the actual whiteout on the upper layer was removed to the directory creation.
Quoted 5 lines
Quoted 3 lines
Reported by: kaixuxia <[email protected]>
Signed-off-by Miklos Szeredi <[email protected]>---
Looks good. A bit of commentary could be useful...
Okay.
Quoted 9 lines
Quoted 5 lines
tests/overlay/062 line 55
Quoted 3 lines
tests/overlay/062 new file +3
-0+ +# unmount overlayfs +$UMOUNT_PROG $SCRATCH_MNTUmount of scratch is not needed at the end of the test.
It's not strictly needed, but gives additional assurance that things hadn't gone bad (after the oops umount returns with EBUSY).
Quoted 6 lines
But the test harness unmount the scratch mount anyway
and if you have fsck.overlay installed it also runs fsck.overay on the layers after each test.The question is why do you need the umount command in the test itself? Is the output/result different without the explicit umount command in the test?
The output is different, because the umount command fails if the mkdir crashed.
Thanks,
Miklos
Quoted 8 lines · Amir Goldstein
Quoted 6 lines · Miklos Szeredi
There's a bug in the overlayfs implementation starting from the very first merged version that may cause an Oops of various forms if a directory is created over a whiteout dentry, but the actual whiteout on the upper layer was removed to the directory creation.
Quoted 5 lines
Quoted 3 lines
Reported by: kaixuxia <[email protected]>
Signed-off-by Miklos Szeredi <[email protected]>---
Looks good. A bit of commentary could be useful...
Eryu,
I see you have not included this test in today's update. Maybe you though that this discussion did not conclude. On my part, besides adding the 2 comment lines, there are no further concerns.
FYI, the fix for this bug is now in master:5e1275808630 ovl: check whiteout in ovl_create_over_whiteout()
Thanks,
Amir.
Quoted 80 lines
Quoted 52 lines
tests/overlay/062 | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/overlay/062.out | 2 ++ tests/overlay/group | 1 + 3 files changed, 63 insertions(+) create mode 100644 tests/overlay/062 create mode 100644 tests/overlay/062.outtests/overlay/062 new file +38
-0@@ -0,0 +1,60 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2018 Red Hat Inc. All Rights Reserved. +# +# FS QA Test 062 +# +# Create dir over cached negative dentry, but whiteout removed from upper +# +# The following kernel commit fixed the kernel crash: ??? +# +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 teststests/overlay/062 line 48
Quoted 10 lines
tests/overlay/062 new file +10
-0+_scratch_mkfs + +# Create test file +lowerdir=${OVL_BASE_SCRATCH_MNT}/${OVL_LOWER} +upperdir=${OVL_BASE_SCRATCH_MNT}/${OVL_UPPER} +mkdir -p $lowerdir +touch ${lowerdir}/file + +_scratch_mount +# Create whiteout and populate dcache with negative dentry
tests/overlay/062 line 50
Quoted 2 lines
tests/overlay/062 new file +2
-0+rm ${SCRATCH_MNT}/file +ls -l ${SCRATCH_MNT}/file > /dev/null 2>&1# Remove whiteout and try to create dir over negative dentry
tests/overlay/062 line 55
Quoted 5 lines
tests/overlay/062 new file +5
-0+rm ${upperdir}/file +mkdir ${SCRATCH_MNT}/file > /dev/null 2>&1 + +# unmount overlayfs +$UMOUNT_PROG $SCRATCH_MNTUmount of scratch is not needed at the end of the test.
Thanks,
Amir.
Quoted 13 lines · Amir Goldstein
Quoted 12 lines · Amir Goldstein
Quoted 9 lines · Miklos Szeredi
There's a bug in the overlayfs implementation starting from the very first merged version that may cause an Oops of various forms if a directory is created over a whiteout dentry, but the actual whiteout on the upper layer was removed to the directory creation.
Reported by: kaixuxia <[email protected]>
Signed-off-by Miklos Szeredi <[email protected]>---
Quoted 8 lines
Quoted 1 line
Looks good. A bit of commentary could be useful...
Eryu,
I see you have not included this test in today's update. Maybe you though that this discussion did not conclude. On my part, besides adding the 2 comment lines, there are no further concerns.
Yeah, I saw the discussion was on going so I didn't save the thread to my "to-review" queue and it fell out of my radar.. I'll queue it now.
Quoted 3 lines
FYI, the fix for this bug is now in master:
5e1275808630ovl: check whiteout inovl_create_over_whiteout()
Thanks for the reminder!
Eryu