All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fstests: btrfs verify hardening agaist duplicate fsid
@ 2018-10-01  8:44 Anand Jain
  2018-10-06 10:14 ` Eryu Guan
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Anand Jain @ 2018-10-01  8:44 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs

We have a known bug in btrfs, that we let the device path be changed
after the device has been mounted. So using this loop hole the new
copied device would appears as if its mounted immediately after its
been copied. So this test case reproduces this issue.

For example:

Initially.. /dev/mmcblk0p4 is mounted as /

lsblk
NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
mmcblk0     179:0    0 29.2G  0 disk
|-mmcblk0p4 179:4    0    4G  0 part /
|-mmcblk0p2 179:2    0  500M  0 part /boot
|-mmcblk0p3 179:3    0  256M  0 part [SWAP]
`-mmcblk0p1 179:1    0  256M  0 part /boot/efi

btrfs fi show
Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
    Total devices 1 FS bytes used 1.40GiB
    devid    1 size 4.00GiB used 3.00GiB path /dev/mmcblk0p4

Copy mmcblk0 to sda
dd if=/dev/mmcblk0 of=/dev/sda

And immediately after the copy completes the change in the device
superblock is notified which the automount scans using
btrfs device scan and the new device sda becomes the mounted root
device.

lsblk
NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
sda           8:0    1 14.9G  0 disk
|-sda4        8:4    1    4G  0 part /
|-sda2        8:2    1  500M  0 part
|-sda3        8:3    1  256M  0 part
`-sda1        8:1    1  256M  0 part
mmcblk0     179:0    0 29.2G  0 disk
|-mmcblk0p4 179:4    0    4G  0 part
|-mmcblk0p2 179:2    0  500M  0 part /boot
|-mmcblk0p3 179:3    0  256M  0 part [SWAP]
`-mmcblk0p1 179:1    0  256M  0 part /boot/efi
btrfs fi show /
Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
    Total devices 1 FS bytes used 1.40GiB
    devid    1 size 4.00GiB used 3.00GiB path /dev/sda4

The bug is quite nasty that you can't either unmount /dev/sda4 or
/dev/mmcblk0p4. And the problem does not get solved until you take
the sda out of the system on to another system to change its fsid using
the 'btrfstune -u' command.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/btrfs/173     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/173.out |  5 ++++
 tests/btrfs/group   |  1 +
 3 files changed, 78 insertions(+)
 create mode 100755 tests/btrfs/173
 create mode 100644 tests/btrfs/173.out

diff --git a/tests/btrfs/173 b/tests/btrfs/173
new file mode 100755
index 000000000000..f59a62e206c3
--- /dev/null
+++ b/tests/btrfs/173
@@ -0,0 +1,72 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2018 Oracle. All Rights Reserved.
+#
+# FS QA Test 173
+#
+# Fuzzy test for FS image duplication.
+#  Could be fixed by
+#    [patch] btrfs: harden agaist duplicate fsid
+#
+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
+
+# Modify as appropriate.
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch_dev_pool 2
+_scratch_dev_pool_get 2
+
+dev_foo=$(echo $SCRATCH_DEV_POOL | awk '{print $1}' | rev | cut -d"/" -f1 | rev)
+dev_bar=$(echo $SCRATCH_DEV_POOL | awk '{print $2}' | rev | cut -d"/" -f1 | rev)
+
+_mkfs_dev /dev/$dev_foo
+_mount /dev/$dev_foo $SCRATCH_MNT
+
+echo mount before btrfs image clone | tee -a $seqres.full
+findmnt /dev/$dev_foo | grep -v TARGET | awk '{print $1" "$2}' | \
+	sed -e "s/$dev_foo/dev_foo/g" | _filter_scratch | tee -a $seqres.full
+findmnt /dev/$dev_bar | grep -v TARGET | awk '{print $1" "$2}' | \
+	sed -e "s/$dev_bar/dev_bar/g" | _filter_scratch | tee -a $seqres.full
+
+for sb_bytenr in 65536 67108864
+do
+	echo -n "dd status=none if=/dev/$dev_foo of=/dev/$dev_bar bs=1 "\
+		"seek=$sb_bytenr skip=$sb_bytenr count=4096" >> $seqres.full
+	dd status=none if=/dev/$dev_foo of=/dev/$dev_bar bs=1 seek=$sb_bytenr \
+				skip=$sb_bytenr count=4096 >> $seqres.full 2>&1
+	echo ..:$? >> $seqres.full
+done
+
+echo mount after btrfs image clone | tee -a $seqres.full
+findmnt /dev/$dev_foo | grep -v TARGET | awk '{print $1" "$2}' | \
+	sed -e "s/$dev_foo/dev_foo/g" | _filter_scratch | tee -a $seqres.full
+findmnt /dev/$dev_bar | grep -v TARGET | awk '{print $1" "$2}' | \
+	sed -e "s/$dev_bar/dev_bar/g" | _filter_scratch | tee -a $seqres.full
+
+_scratch_dev_pool_put
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/173.out b/tests/btrfs/173.out
new file mode 100644
index 000000000000..a2ef7a26f4b9
--- /dev/null
+++ b/tests/btrfs/173.out
@@ -0,0 +1,5 @@
+QA output created by 173
+mount before btrfs image clone
+SCRATCH_MNT /dev/dev_foo
+mount after btrfs image clone
+SCRATCH_MNT /dev/dev_foo
diff --git a/tests/btrfs/group b/tests/btrfs/group
index 45782565c3b7..b2f1393f3e97 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -175,3 +175,4 @@
 170 auto quick snapshot
 171 auto quick qgroup
 172 auto quick punch
+173 volume
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] fstests: btrfs verify hardening agaist duplicate fsid
  2018-10-01  8:44 [PATCH] fstests: btrfs verify hardening agaist duplicate fsid Anand Jain
@ 2018-10-06 10:14 ` Eryu Guan
  2018-10-08 18:28   ` Anand Jain
  2018-10-08 18:14 ` [PATCH v2] " Anand Jain
  2018-10-08 18:28 ` [PATCH v2 rev log added] " Anand Jain
  2 siblings, 1 reply; 10+ messages in thread
From: Eryu Guan @ 2018-10-06 10:14 UTC (permalink / raw)
  To: Anand Jain; +Cc: fstests, linux-btrfs

On Mon, Oct 01, 2018 at 04:44:35PM +0800, Anand Jain wrote:
> We have a known bug in btrfs, that we let the device path be changed
> after the device has been mounted. So using this loop hole the new
> copied device would appears as if its mounted immediately after its
> been copied. So this test case reproduces this issue.
> 
> For example:
> 
> Initially.. /dev/mmcblk0p4 is mounted as /
> 
> lsblk
> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
> mmcblk0     179:0    0 29.2G  0 disk
> |-mmcblk0p4 179:4    0    4G  0 part /
> |-mmcblk0p2 179:2    0  500M  0 part /boot
> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
> 
> btrfs fi show
> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>     Total devices 1 FS bytes used 1.40GiB
>     devid    1 size 4.00GiB used 3.00GiB path /dev/mmcblk0p4
> 
> Copy mmcblk0 to sda
> dd if=/dev/mmcblk0 of=/dev/sda
> 
> And immediately after the copy completes the change in the device
> superblock is notified which the automount scans using
> btrfs device scan and the new device sda becomes the mounted root
> device.
> 
> lsblk
> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
> sda           8:0    1 14.9G  0 disk
> |-sda4        8:4    1    4G  0 part /
> |-sda2        8:2    1  500M  0 part
> |-sda3        8:3    1  256M  0 part
> `-sda1        8:1    1  256M  0 part
> mmcblk0     179:0    0 29.2G  0 disk
> |-mmcblk0p4 179:4    0    4G  0 part
> |-mmcblk0p2 179:2    0  500M  0 part /boot
> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
> btrfs fi show /
> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>     Total devices 1 FS bytes used 1.40GiB
>     devid    1 size 4.00GiB used 3.00GiB path /dev/sda4
> 
> The bug is quite nasty that you can't either unmount /dev/sda4 or
> /dev/mmcblk0p4. And the problem does not get solved until you take
> the sda out of the system on to another system to change its fsid using
> the 'btrfstune -u' command.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Looks like that the test will break the whole test env as it leaves an
unmountable $SCRATCH_MNT. I'd wait for the fix to get in first before
merging the test, in case it breaks normal regression tests. (I noticed
that the test is not in 'auto' group, so it's not that dangerous.)

Also, it'd be great if test can be reviewed by btrfs folks too!

> ---
>  tests/btrfs/173     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/173.out |  5 ++++
>  tests/btrfs/group   |  1 +
>  3 files changed, 78 insertions(+)
>  create mode 100755 tests/btrfs/173
>  create mode 100644 tests/btrfs/173.out
> 
> diff --git a/tests/btrfs/173 b/tests/btrfs/173
> new file mode 100755
> index 000000000000..f59a62e206c3
> --- /dev/null
> +++ b/tests/btrfs/173
> @@ -0,0 +1,72 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2018 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 173
> +#
> +# Fuzzy test for FS image duplication.
> +#  Could be fixed by
> +#    [patch] btrfs: harden agaist duplicate fsid
> +#
> +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
> +
> +# Modify as appropriate.
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch_dev_pool 2
> +_scratch_dev_pool_get 2
> +
> +dev_foo=$(echo $SCRATCH_DEV_POOL | awk '{print $1}' | rev | cut -d"/" -f1 | rev)
> +dev_bar=$(echo $SCRATCH_DEV_POOL | awk '{print $2}' | rev | cut -d"/" -f1 | rev)

This doesn't work if the devices in SCRATCH_DEV_POOL are symlinks, e.g.
lvm devices: /dev/mapper/testvg-testlv1, dev_foo is "testvg-testlv1" in
this case.

> +
> +_mkfs_dev /dev/$dev_foo

But /dev/testvg-testlv1 isn't existed.

_short_dev and/or _real_dev is useful in this case. e.g.

dev_foo=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
# dev_foo is like "dm-1"
dev_foo=$(_short_dev $dev_foo)
# dev_foo is like "/dev/dm-1"
dev_foo=$(_real_dev $dev_foo)

> +_mount /dev/$dev_foo $SCRATCH_MNT

It'd better to mount non-SCRATCH_DEV to other mount point, e.g.
$TEST_DIR/$seq.mnt

Thanks,
Eryu

> +
> +echo mount before btrfs image clone | tee -a $seqres.full
> +findmnt /dev/$dev_foo | grep -v TARGET | awk '{print $1" "$2}' | \
> +	sed -e "s/$dev_foo/dev_foo/g" | _filter_scratch | tee -a $seqres.full
> +findmnt /dev/$dev_bar | grep -v TARGET | awk '{print $1" "$2}' | \
> +	sed -e "s/$dev_bar/dev_bar/g" | _filter_scratch | tee -a $seqres.full
> +
> +for sb_bytenr in 65536 67108864
> +do
> +	echo -n "dd status=none if=/dev/$dev_foo of=/dev/$dev_bar bs=1 "\
> +		"seek=$sb_bytenr skip=$sb_bytenr count=4096" >> $seqres.full
> +	dd status=none if=/dev/$dev_foo of=/dev/$dev_bar bs=1 seek=$sb_bytenr \
> +				skip=$sb_bytenr count=4096 >> $seqres.full 2>&1
> +	echo ..:$? >> $seqres.full
> +done
> +
> +echo mount after btrfs image clone | tee -a $seqres.full
> +findmnt /dev/$dev_foo | grep -v TARGET | awk '{print $1" "$2}' | \
> +	sed -e "s/$dev_foo/dev_foo/g" | _filter_scratch | tee -a $seqres.full
> +findmnt /dev/$dev_bar | grep -v TARGET | awk '{print $1" "$2}' | \
> +	sed -e "s/$dev_bar/dev_bar/g" | _filter_scratch | tee -a $seqres.full
> +
> +_scratch_dev_pool_put
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/173.out b/tests/btrfs/173.out
> new file mode 100644
> index 000000000000..a2ef7a26f4b9
> --- /dev/null
> +++ b/tests/btrfs/173.out
> @@ -0,0 +1,5 @@
> +QA output created by 173
> +mount before btrfs image clone
> +SCRATCH_MNT /dev/dev_foo
> +mount after btrfs image clone
> +SCRATCH_MNT /dev/dev_foo
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index 45782565c3b7..b2f1393f3e97 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -175,3 +175,4 @@
>  170 auto quick snapshot
>  171 auto quick qgroup
>  172 auto quick punch
> +173 volume
> -- 
> 1.8.3.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] fstests: btrfs verify hardening agaist duplicate fsid
  2018-10-01  8:44 [PATCH] fstests: btrfs verify hardening agaist duplicate fsid Anand Jain
  2018-10-06 10:14 ` Eryu Guan
@ 2018-10-08 18:14 ` Anand Jain
  2018-10-08 18:28 ` [PATCH v2 rev log added] " Anand Jain
  2 siblings, 0 replies; 10+ messages in thread
From: Anand Jain @ 2018-10-08 18:14 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs

We have a known bug in btrfs, that we let the device path be changed
after the device has been mounted. So using this loop hole the new
copied device would appears as if its mounted immediately after its
been copied. So this test case reproduces this issue.

For example:

Initially.. /dev/mmcblk0p4 is mounted as /

lsblk
NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
mmcblk0     179:0    0 29.2G  0 disk
|-mmcblk0p4 179:4    0    4G  0 part /
|-mmcblk0p2 179:2    0  500M  0 part /boot
|-mmcblk0p3 179:3    0  256M  0 part [SWAP]
`-mmcblk0p1 179:1    0  256M  0 part /boot/efi

btrfs fi show
Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
    Total devices 1 FS bytes used 1.40GiB
    devid    1 size 4.00GiB used 3.00GiB path /dev/mmcblk0p4

Copy mmcblk0 to sda
dd if=/dev/mmcblk0 of=/dev/sda

And immediately after the copy completes the change in the device
superblock is notified which the automount scans using
btrfs device scan and the new device sda becomes the mounted root
device.

lsblk
NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
sda           8:0    1 14.9G  0 disk
|-sda4        8:4    1    4G  0 part /
|-sda2        8:2    1  500M  0 part
|-sda3        8:3    1  256M  0 part
`-sda1        8:1    1  256M  0 part
mmcblk0     179:0    0 29.2G  0 disk
|-mmcblk0p4 179:4    0    4G  0 part
|-mmcblk0p2 179:2    0  500M  0 part /boot
|-mmcblk0p3 179:3    0  256M  0 part [SWAP]
`-mmcblk0p1 179:1    0  256M  0 part /boot/efi
btrfs fi show /
Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
    Total devices 1 FS bytes used 1.40GiB
    devid    1 size 4.00GiB used 3.00GiB path /dev/sda4

The bug is quite nasty that you can't either unmount /dev/sda4 or
/dev/mmcblk0p4. And the problem does not get solved until you take
the sda out of the system on to another system to change its fsid using
the 'btrfstune -u' command.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/btrfs/173     | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/173.out |  6 ++++
 tests/btrfs/group   |  1 +
 3 files changed, 95 insertions(+)
 create mode 100755 tests/btrfs/173
 create mode 100644 tests/btrfs/173.out

diff --git a/tests/btrfs/173 b/tests/btrfs/173
new file mode 100755
index 000000000000..b466ae921e19
--- /dev/null
+++ b/tests/btrfs/173
@@ -0,0 +1,88 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2018 Oracle. All Rights Reserved.
+#
+# FS QA Test 173
+#
+# Fuzzy test for FS image duplication.
+#  Could be fixed by
+#    [patch] btrfs: harden agaist duplicate fsid
+#
+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
+
+mnt=$TEST_DIR/$seq.mnt
+_cleanup()
+{
+	rm -rf $mnt > /dev/null 2>&1
+	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
+
+# Modify as appropriate.
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch_dev_pool 2
+_scratch_dev_pool_get 2
+
+dev_foo=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
+dev_bar=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
+
+echo dev_foo=$dev_foo >> $seqres.full
+echo dev_bar=$dev_bar >> $seqres.full
+echo | tee -a $seqres.full
+
+rm -rf $mnt > /dev/null 2>&1
+mkdir $mnt
+_mkfs_dev $dev_foo
+_mount $dev_foo $mnt
+
+check_btrfs_mount()
+{
+	local x=$(findmnt $mnt | grep -v TARGET | awk '{print $2}')
+	[[ $x == $dev_foo ]] && echo DEV_FOO
+	[[ $x == $dev_bar ]] && echo DEV_BAR
+}
+
+echo MNT $(check_btrfs_mount)
+
+for sb_bytenr in 65536 67108864
+do
+	echo -n "dd status=none if=$dev_foo of=$dev_bar bs=1 "\
+		"seek=$sb_bytenr skip=$sb_bytenr count=4096" >> $seqres.full
+	dd status=none if=$dev_foo of=$dev_bar bs=1 seek=$sb_bytenr \
+				skip=$sb_bytenr count=4096 >> $seqres.full 2>&1
+	echo ..:$? >> $seqres.full
+done
+
+#Original device is mounted, scan of its clone should fail
+$BTRFS_UTIL_PROG device scan $dev_bar >> $seqres.full 2>&1
+echo btrfs device scan dev_bar ...:$?| tee -a $seqres.full
+
+echo MNT $(check_btrfs_mount)
+
+#Original device scan should be successful
+$BTRFS_UTIL_PROG device scan $dev_foo >> $seqres.full 2>&1
+echo btrfs device scan dev_foo ...:$?| tee -a $seqres.full
+
+umount $mnt > /dev/null 2>&1
+_scratch_dev_pool_put
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/173.out b/tests/btrfs/173.out
new file mode 100644
index 000000000000..3c7e3fb4e3f7
--- /dev/null
+++ b/tests/btrfs/173.out
@@ -0,0 +1,6 @@
+QA output created by 173
+
+MNT DEV_FOO
+btrfs device scan dev_bar ...:1
+MNT DEV_FOO
+btrfs device scan dev_foo ...:0
diff --git a/tests/btrfs/group b/tests/btrfs/group
index 45782565c3b7..b2f1393f3e97 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -175,3 +175,4 @@
 170 auto quick snapshot
 171 auto quick qgroup
 172 auto quick punch
+173 volume
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 rev log added] fstests: btrfs verify hardening agaist duplicate fsid
  2018-10-01  8:44 [PATCH] fstests: btrfs verify hardening agaist duplicate fsid Anand Jain
  2018-10-06 10:14 ` Eryu Guan
  2018-10-08 18:14 ` [PATCH v2] " Anand Jain
@ 2018-10-08 18:28 ` Anand Jain
  2018-10-21 10:25   ` Eryu Guan
  2018-10-26 15:02   ` Nikolay Borisov
  2 siblings, 2 replies; 10+ messages in thread
From: Anand Jain @ 2018-10-08 18:28 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs

We have a known bug in btrfs, that we let the device path be changed
after the device has been mounted. So using this loop hole the new
copied device would appears as if its mounted immediately after its
been copied. So this test case reproduces this issue.

For example:

Initially.. /dev/mmcblk0p4 is mounted as /

lsblk
NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
mmcblk0     179:0    0 29.2G  0 disk
|-mmcblk0p4 179:4    0    4G  0 part /
|-mmcblk0p2 179:2    0  500M  0 part /boot
|-mmcblk0p3 179:3    0  256M  0 part [SWAP]
`-mmcblk0p1 179:1    0  256M  0 part /boot/efi

btrfs fi show
Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
    Total devices 1 FS bytes used 1.40GiB
    devid    1 size 4.00GiB used 3.00GiB path /dev/mmcblk0p4

Copy mmcblk0 to sda
dd if=/dev/mmcblk0 of=/dev/sda

And immediately after the copy completes the change in the device
superblock is notified which the automount scans using
btrfs device scan and the new device sda becomes the mounted root
device.

lsblk
NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
sda           8:0    1 14.9G  0 disk
|-sda4        8:4    1    4G  0 part /
|-sda2        8:2    1  500M  0 part
|-sda3        8:3    1  256M  0 part
`-sda1        8:1    1  256M  0 part
mmcblk0     179:0    0 29.2G  0 disk
|-mmcblk0p4 179:4    0    4G  0 part
|-mmcblk0p2 179:2    0  500M  0 part /boot
|-mmcblk0p3 179:3    0  256M  0 part [SWAP]
`-mmcblk0p1 179:1    0  256M  0 part /boot/efi
btrfs fi show /
Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
    Total devices 1 FS bytes used 1.40GiB
    devid    1 size 4.00GiB used 3.00GiB path /dev/sda4

The bug is quite nasty that you can't either unmount /dev/sda4 or
/dev/mmcblk0p4. And the problem does not get solved until you take
the sda out of the system on to another system to change its fsid using
the 'btrfstune -u' command.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v1->v2: 
  dont play around with dev patch use it as it is.
  do not use SCRATCH_MNT instead create it at the TEST_DIR and its related
   changes.
  golden out changes
   
 tests/btrfs/173     | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/173.out |  6 ++++
 tests/btrfs/group   |  1 +
 3 files changed, 95 insertions(+)
 create mode 100755 tests/btrfs/173
 create mode 100644 tests/btrfs/173.out

diff --git a/tests/btrfs/173 b/tests/btrfs/173
new file mode 100755
index 000000000000..b466ae921e19
--- /dev/null
+++ b/tests/btrfs/173
@@ -0,0 +1,88 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2018 Oracle. All Rights Reserved.
+#
+# FS QA Test 173
+#
+# Fuzzy test for FS image duplication.
+#  Could be fixed by
+#    [patch] btrfs: harden agaist duplicate fsid
+#
+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
+
+mnt=$TEST_DIR/$seq.mnt
+_cleanup()
+{
+	rm -rf $mnt > /dev/null 2>&1
+	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
+
+# Modify as appropriate.
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch_dev_pool 2
+_scratch_dev_pool_get 2
+
+dev_foo=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
+dev_bar=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
+
+echo dev_foo=$dev_foo >> $seqres.full
+echo dev_bar=$dev_bar >> $seqres.full
+echo | tee -a $seqres.full
+
+rm -rf $mnt > /dev/null 2>&1
+mkdir $mnt
+_mkfs_dev $dev_foo
+_mount $dev_foo $mnt
+
+check_btrfs_mount()
+{
+	local x=$(findmnt $mnt | grep -v TARGET | awk '{print $2}')
+	[[ $x == $dev_foo ]] && echo DEV_FOO
+	[[ $x == $dev_bar ]] && echo DEV_BAR
+}
+
+echo MNT $(check_btrfs_mount)
+
+for sb_bytenr in 65536 67108864
+do
+	echo -n "dd status=none if=$dev_foo of=$dev_bar bs=1 "\
+		"seek=$sb_bytenr skip=$sb_bytenr count=4096" >> $seqres.full
+	dd status=none if=$dev_foo of=$dev_bar bs=1 seek=$sb_bytenr \
+				skip=$sb_bytenr count=4096 >> $seqres.full 2>&1
+	echo ..:$? >> $seqres.full
+done
+
+#Original device is mounted, scan of its clone should fail
+$BTRFS_UTIL_PROG device scan $dev_bar >> $seqres.full 2>&1
+echo btrfs device scan dev_bar ...:$?| tee -a $seqres.full
+
+echo MNT $(check_btrfs_mount)
+
+#Original device scan should be successful
+$BTRFS_UTIL_PROG device scan $dev_foo >> $seqres.full 2>&1
+echo btrfs device scan dev_foo ...:$?| tee -a $seqres.full
+
+umount $mnt > /dev/null 2>&1
+_scratch_dev_pool_put
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/173.out b/tests/btrfs/173.out
new file mode 100644
index 000000000000..3c7e3fb4e3f7
--- /dev/null
+++ b/tests/btrfs/173.out
@@ -0,0 +1,6 @@
+QA output created by 173
+
+MNT DEV_FOO
+btrfs device scan dev_bar ...:1
+MNT DEV_FOO
+btrfs device scan dev_foo ...:0
diff --git a/tests/btrfs/group b/tests/btrfs/group
index 45782565c3b7..b2f1393f3e97 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -175,3 +175,4 @@
 170 auto quick snapshot
 171 auto quick qgroup
 172 auto quick punch
+173 volume
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] fstests: btrfs verify hardening agaist duplicate fsid
  2018-10-06 10:14 ` Eryu Guan
@ 2018-10-08 18:28   ` Anand Jain
  0 siblings, 0 replies; 10+ messages in thread
From: Anand Jain @ 2018-10-08 18:28 UTC (permalink / raw)
  To: Eryu Guan; +Cc: fstests, linux-btrfs



On 10/06/2018 06:14 PM, Eryu Guan wrote:
> On Mon, Oct 01, 2018 at 04:44:35PM +0800, Anand Jain wrote:
>> We have a known bug in btrfs, that we let the device path be changed
>> after the device has been mounted. So using this loop hole the new
>> copied device would appears as if its mounted immediately after its
>> been copied. So this test case reproduces this issue.
>>
>> For example:
>>
>> Initially.. /dev/mmcblk0p4 is mounted as /
>>
>> lsblk
>> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
>> mmcblk0     179:0    0 29.2G  0 disk
>> |-mmcblk0p4 179:4    0    4G  0 part /
>> |-mmcblk0p2 179:2    0  500M  0 part /boot
>> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
>> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
>>
>> btrfs fi show
>> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>>      Total devices 1 FS bytes used 1.40GiB
>>      devid    1 size 4.00GiB used 3.00GiB path /dev/mmcblk0p4
>>
>> Copy mmcblk0 to sda
>> dd if=/dev/mmcblk0 of=/dev/sda
>>
>> And immediately after the copy completes the change in the device
>> superblock is notified which the automount scans using
>> btrfs device scan and the new device sda becomes the mounted root
>> device.
>>
>> lsblk
>> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
>> sda           8:0    1 14.9G  0 disk
>> |-sda4        8:4    1    4G  0 part /
>> |-sda2        8:2    1  500M  0 part
>> |-sda3        8:3    1  256M  0 part
>> `-sda1        8:1    1  256M  0 part
>> mmcblk0     179:0    0 29.2G  0 disk
>> |-mmcblk0p4 179:4    0    4G  0 part
>> |-mmcblk0p2 179:2    0  500M  0 part /boot
>> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
>> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
>> btrfs fi show /
>> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>>      Total devices 1 FS bytes used 1.40GiB
>>      devid    1 size 4.00GiB used 3.00GiB path /dev/sda4
>>
>> The bug is quite nasty that you can't either unmount /dev/sda4 or
>> /dev/mmcblk0p4. And the problem does not get solved until you take
>> the sda out of the system on to another system to change its fsid using
>> the 'btrfstune -u' command.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> 
> Looks like that the test will break the whole test env as it leaves an
> unmountable $SCRATCH_MNT. I'd wait for the fix to get in first before
> merging the test, in case it breaks normal regression tests. (I noticed
> that the test is not in 'auto' group, so it's not that dangerous.)

Its possible that its unmountable without the kernel patch. But I am 
unable to reproduce it consistently with or without the kernel patch.

Any idea ways to make it auto for kernels without the patch?

> Also, it'd be great if test can be reviewed by btrfs folks too!
> 
>> ---
>>   tests/btrfs/173     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/btrfs/173.out |  5 ++++
>>   tests/btrfs/group   |  1 +
>>   3 files changed, 78 insertions(+)
>>   create mode 100755 tests/btrfs/173
>>   create mode 100644 tests/btrfs/173.out
>>
>> diff --git a/tests/btrfs/173 b/tests/btrfs/173
>> new file mode 100755
>> index 000000000000..f59a62e206c3
>> --- /dev/null
>> +++ b/tests/btrfs/173
>> @@ -0,0 +1,72 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2018 Oracle. All Rights Reserved.
>> +#
>> +# FS QA Test 173
>> +#
>> +# Fuzzy test for FS image duplication.
>> +#  Could be fixed by
>> +#    [patch] btrfs: harden agaist duplicate fsid
>> +#
>> +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
>> +
>> +# Modify as appropriate.
>> +_supported_fs btrfs
>> +_supported_os Linux
>> +_require_scratch_dev_pool 2
>> +_scratch_dev_pool_get 2
>> +
>> +dev_foo=$(echo $SCRATCH_DEV_POOL | awk '{print $1}' | rev | cut -d"/" -f1 | rev)
>> +dev_bar=$(echo $SCRATCH_DEV_POOL | awk '{print $2}' | rev | cut -d"/" -f1 | rev)
> 
> This doesn't work if the devices in SCRATCH_DEV_POOL are symlinks, e.g.
> lvm devices: /dev/mapper/testvg-testlv1, dev_foo is "testvg-testlv1" in
> this case.

  Ah, right will fix.

>> +
>> +_mkfs_dev /dev/$dev_foo
> 
> But /dev/testvg-testlv1 isn't existed.
> 
> _short_dev and/or _real_dev is useful in this case. e.g.
> 
> dev_foo=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
> # dev_foo is like "dm-1"
> dev_foo=$(_short_dev $dev_foo)
> # dev_foo is like "/dev/dm-1"
> dev_foo=$(_real_dev $dev_foo)

  I changed the code a bit which avoids the split. Pls review if that 
will be ok.

>> +_mount /dev/$dev_foo $SCRATCH_MNT
> 
> It'd better to mount non-SCRATCH_DEV to other mount point, e.g.
> $TEST_DIR/$seq.mnt

  Will do, any idea why? Isn't the framework automatically try to 
unmount SCRATCH_MNT.

Thanks, Anand

> Thanks,
> Eryu
> 
>> +
>> +echo mount before btrfs image clone | tee -a $seqres.full
>> +findmnt /dev/$dev_foo | grep -v TARGET | awk '{print $1" "$2}' | \
>> +	sed -e "s/$dev_foo/dev_foo/g" | _filter_scratch | tee -a $seqres.full
>> +findmnt /dev/$dev_bar | grep -v TARGET | awk '{print $1" "$2}' | \
>> +	sed -e "s/$dev_bar/dev_bar/g" | _filter_scratch | tee -a $seqres.full
>> +
>> +for sb_bytenr in 65536 67108864
>> +do
>> +	echo -n "dd status=none if=/dev/$dev_foo of=/dev/$dev_bar bs=1 "\
>> +		"seek=$sb_bytenr skip=$sb_bytenr count=4096" >> $seqres.full
>> +	dd status=none if=/dev/$dev_foo of=/dev/$dev_bar bs=1 seek=$sb_bytenr \
>> +				skip=$sb_bytenr count=4096 >> $seqres.full 2>&1
>> +	echo ..:$? >> $seqres.full
>> +done
>> +
>> +echo mount after btrfs image clone | tee -a $seqres.full
>> +findmnt /dev/$dev_foo | grep -v TARGET | awk '{print $1" "$2}' | \
>> +	sed -e "s/$dev_foo/dev_foo/g" | _filter_scratch | tee -a $seqres.full
>> +findmnt /dev/$dev_bar | grep -v TARGET | awk '{print $1" "$2}' | \
>> +	sed -e "s/$dev_bar/dev_bar/g" | _filter_scratch | tee -a $seqres.full
>> +
>> +_scratch_dev_pool_put
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/btrfs/173.out b/tests/btrfs/173.out
>> new file mode 100644
>> index 000000000000..a2ef7a26f4b9
>> --- /dev/null
>> +++ b/tests/btrfs/173.out
>> @@ -0,0 +1,5 @@
>> +QA output created by 173
>> +mount before btrfs image clone
>> +SCRATCH_MNT /dev/dev_foo
>> +mount after btrfs image clone
>> +SCRATCH_MNT /dev/dev_foo
>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>> index 45782565c3b7..b2f1393f3e97 100644
>> --- a/tests/btrfs/group
>> +++ b/tests/btrfs/group
>> @@ -175,3 +175,4 @@
>>   170 auto quick snapshot
>>   171 auto quick qgroup
>>   172 auto quick punch
>> +173 volume
>> -- 
>> 1.8.3.1
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 rev log added] fstests: btrfs verify hardening agaist duplicate fsid
  2018-10-08 18:28 ` [PATCH v2 rev log added] " Anand Jain
@ 2018-10-21 10:25   ` Eryu Guan
  2018-10-26 15:02   ` Nikolay Borisov
  1 sibling, 0 replies; 10+ messages in thread
From: Eryu Guan @ 2018-10-21 10:25 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs; +Cc: fstests

On Tue, Oct 09, 2018 at 02:28:21AM +0800, Anand Jain wrote:
> We have a known bug in btrfs, that we let the device path be changed
> after the device has been mounted. So using this loop hole the new
> copied device would appears as if its mounted immediately after its
> been copied. So this test case reproduces this issue.
> 
> For example:
> 
> Initially.. /dev/mmcblk0p4 is mounted as /
> 
> lsblk
> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
> mmcblk0     179:0    0 29.2G  0 disk
> |-mmcblk0p4 179:4    0    4G  0 part /
> |-mmcblk0p2 179:2    0  500M  0 part /boot
> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
> 
> btrfs fi show
> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>     Total devices 1 FS bytes used 1.40GiB
>     devid    1 size 4.00GiB used 3.00GiB path /dev/mmcblk0p4
> 
> Copy mmcblk0 to sda
> dd if=/dev/mmcblk0 of=/dev/sda
> 
> And immediately after the copy completes the change in the device
> superblock is notified which the automount scans using
> btrfs device scan and the new device sda becomes the mounted root
> device.
> 
> lsblk
> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
> sda           8:0    1 14.9G  0 disk
> |-sda4        8:4    1    4G  0 part /
> |-sda2        8:2    1  500M  0 part
> |-sda3        8:3    1  256M  0 part
> `-sda1        8:1    1  256M  0 part
> mmcblk0     179:0    0 29.2G  0 disk
> |-mmcblk0p4 179:4    0    4G  0 part
> |-mmcblk0p2 179:2    0  500M  0 part /boot
> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
> btrfs fi show /
> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>     Total devices 1 FS bytes used 1.40GiB
>     devid    1 size 4.00GiB used 3.00GiB path /dev/sda4
> 
> The bug is quite nasty that you can't either unmount /dev/sda4 or
> /dev/mmcblk0p4. And the problem does not get solved until you take
> the sda out of the system on to another system to change its fsid using
> the 'btrfstune -u' command.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Hi btrfs folks,

Please help review if this patch adds a valid test for btrfs. It looks
fine to me from fstests' perspective of view, though it needs some
really minor tweaks that I can fix on commit (e.g. format of for loop,
adding space after '#' in comments line). Thanks a lot!

Eryu

> ---
> v1->v2: 
>   dont play around with dev patch use it as it is.
>   do not use SCRATCH_MNT instead create it at the TEST_DIR and its related
>    changes.
>   golden out changes
>    
>  tests/btrfs/173     | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/173.out |  6 ++++
>  tests/btrfs/group   |  1 +
>  3 files changed, 95 insertions(+)
>  create mode 100755 tests/btrfs/173
>  create mode 100644 tests/btrfs/173.out
> 
> diff --git a/tests/btrfs/173 b/tests/btrfs/173
> new file mode 100755
> index 000000000000..b466ae921e19
> --- /dev/null
> +++ b/tests/btrfs/173
> @@ -0,0 +1,88 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2018 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 173
> +#
> +# Fuzzy test for FS image duplication.
> +#  Could be fixed by
> +#    [patch] btrfs: harden agaist duplicate fsid
> +#
> +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
> +
> +mnt=$TEST_DIR/$seq.mnt
> +_cleanup()
> +{
> +	rm -rf $mnt > /dev/null 2>&1
> +	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
> +
> +# Modify as appropriate.
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch_dev_pool 2
> +_scratch_dev_pool_get 2
> +
> +dev_foo=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
> +dev_bar=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
> +
> +echo dev_foo=$dev_foo >> $seqres.full
> +echo dev_bar=$dev_bar >> $seqres.full
> +echo | tee -a $seqres.full
> +
> +rm -rf $mnt > /dev/null 2>&1
> +mkdir $mnt
> +_mkfs_dev $dev_foo
> +_mount $dev_foo $mnt
> +
> +check_btrfs_mount()
> +{
> +	local x=$(findmnt $mnt | grep -v TARGET | awk '{print $2}')
> +	[[ $x == $dev_foo ]] && echo DEV_FOO
> +	[[ $x == $dev_bar ]] && echo DEV_BAR
> +}
> +
> +echo MNT $(check_btrfs_mount)
> +
> +for sb_bytenr in 65536 67108864
> +do
> +	echo -n "dd status=none if=$dev_foo of=$dev_bar bs=1 "\
> +		"seek=$sb_bytenr skip=$sb_bytenr count=4096" >> $seqres.full
> +	dd status=none if=$dev_foo of=$dev_bar bs=1 seek=$sb_bytenr \
> +				skip=$sb_bytenr count=4096 >> $seqres.full 2>&1
> +	echo ..:$? >> $seqres.full
> +done
> +
> +#Original device is mounted, scan of its clone should fail
> +$BTRFS_UTIL_PROG device scan $dev_bar >> $seqres.full 2>&1
> +echo btrfs device scan dev_bar ...:$?| tee -a $seqres.full
> +
> +echo MNT $(check_btrfs_mount)
> +
> +#Original device scan should be successful
> +$BTRFS_UTIL_PROG device scan $dev_foo >> $seqres.full 2>&1
> +echo btrfs device scan dev_foo ...:$?| tee -a $seqres.full
> +
> +umount $mnt > /dev/null 2>&1
> +_scratch_dev_pool_put
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/173.out b/tests/btrfs/173.out
> new file mode 100644
> index 000000000000..3c7e3fb4e3f7
> --- /dev/null
> +++ b/tests/btrfs/173.out
> @@ -0,0 +1,6 @@
> +QA output created by 173
> +
> +MNT DEV_FOO
> +btrfs device scan dev_bar ...:1
> +MNT DEV_FOO
> +btrfs device scan dev_foo ...:0
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index 45782565c3b7..b2f1393f3e97 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -175,3 +175,4 @@
>  170 auto quick snapshot
>  171 auto quick qgroup
>  172 auto quick punch
> +173 volume
> -- 
> 1.8.3.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 rev log added] fstests: btrfs verify hardening agaist duplicate fsid
  2018-10-08 18:28 ` [PATCH v2 rev log added] " Anand Jain
  2018-10-21 10:25   ` Eryu Guan
@ 2018-10-26 15:02   ` Nikolay Borisov
  2018-10-26 15:34     ` Anand Jain
  1 sibling, 1 reply; 10+ messages in thread
From: Nikolay Borisov @ 2018-10-26 15:02 UTC (permalink / raw)
  To: Anand Jain, fstests; +Cc: linux-btrfs



On 8.10.18 г. 21:28 ч., Anand Jain wrote:
> We have a known bug in btrfs, that we let the device path be changed
> after the device has been mounted. So using this loop hole the new
> copied device would appears as if its mounted immediately after its
> been copied. So this test case reproduces this issue.
> 
> For example:
> 
> Initially.. /dev/mmcblk0p4 is mounted as /
> 
> lsblk
> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
> mmcblk0     179:0    0 29.2G  0 disk
> |-mmcblk0p4 179:4    0    4G  0 part /
> |-mmcblk0p2 179:2    0  500M  0 part /boot
> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
> 
> btrfs fi show
> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>     Total devices 1 FS bytes used 1.40GiB
>     devid    1 size 4.00GiB used 3.00GiB path /dev/mmcblk0p4
> 
> Copy mmcblk0 to sda
> dd if=/dev/mmcblk0 of=/dev/sda
> 
> And immediately after the copy completes the change in the device
> superblock is notified which the automount scans using
> btrfs device scan and the new device sda becomes the mounted root
> device.
> 
> lsblk
> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
> sda           8:0    1 14.9G  0 disk
> |-sda4        8:4    1    4G  0 part /
> |-sda2        8:2    1  500M  0 part
> |-sda3        8:3    1  256M  0 part
> `-sda1        8:1    1  256M  0 part
> mmcblk0     179:0    0 29.2G  0 disk
> |-mmcblk0p4 179:4    0    4G  0 part
> |-mmcblk0p2 179:2    0  500M  0 part /boot
> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
> btrfs fi show /
> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>     Total devices 1 FS bytes used 1.40GiB
>     devid    1 size 4.00GiB used 3.00GiB path /dev/sda4
> 
> The bug is quite nasty that you can't either unmount /dev/sda4 or
> /dev/mmcblk0p4. And the problem does not get solved until you take
> the sda out of the system on to another system to change its fsid using
> the 'btrfstune -u' command.

Is there a pending fix for this?

> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v1->v2: 
>   dont play around with dev patch use it as it is.
>   do not use SCRATCH_MNT instead create it at the TEST_DIR and its related
>    changes.
>   golden out changes
>    
>  tests/btrfs/173     | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/173.out |  6 ++++
>  tests/btrfs/group   |  1 +
>  3 files changed, 95 insertions(+)
>  create mode 100755 tests/btrfs/173
>  create mode 100644 tests/btrfs/173.out
> 
> diff --git a/tests/btrfs/173 b/tests/btrfs/173
> new file mode 100755
> index 000000000000..b466ae921e19
> --- /dev/null
> +++ b/tests/btrfs/173
> @@ -0,0 +1,88 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2018 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 173
> +#
> +# Fuzzy test for FS image duplication.
> +#  Could be fixed by
> +#    [patch] btrfs: harden agaist duplicate fsid
> +#
> +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
> +
> +mnt=$TEST_DIR/$seq.mnt
> +_cleanup()
> +{
> +	rm -rf $mnt > /dev/null 2>&1
> +	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
> +
> +# Modify as appropriate.
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch_dev_pool 2
> +_scratch_dev_pool_get 2
> +
> +dev_foo=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
> +dev_bar=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')

Naming devices _foo and _bar just shows you could not care less about
the test. Either use sane names - primary_dev/secondary dev or device_1
and device_2.

> +
> +echo dev_foo=$dev_foo >> $seqres.full
> +echo dev_bar=$dev_bar >> $seqres.full
> +echo | tee -a $seqres.full
> +
> +rm -rf $mnt > /dev/null 2>&1

So what is $mnt? I can see it's used by very few tests and it's not
obvious? Generally you should either define it as a local variable or
use one of the SCRATCH_MNT/TEST_MNT global variables. Also I checked
with Eric re. the use of $mnt in tests and his conclusion is :

"<sandeen> looks like a bug"

> +mkdir $mnt
> +_mkfs_dev $dev_foo
> +_mount $dev_foo $mnt
> +
> +check_btrfs_mount()
> +{
> +	local x=$(findmnt $mnt | grep -v TARGET | awk '{print $2}')
> +	[[ $x == $dev_foo ]] && echo DEV_FOO
> +	[[ $x == $dev_bar ]] && echo DEV_BAR
> +}

Same thing here re. DEV_(FOO|BAR).

> +
> +echo MNT $(check_btrfs_mount)
> +
> +for sb_bytenr in 65536 67108864
> +do
> +	echo -n "dd status=none if=$dev_foo of=$dev_bar bs=1 "\
> +		"seek=$sb_bytenr skip=$sb_bytenr count=4096" >> $seqres.full
> +	dd status=none if=$dev_foo of=$dev_bar bs=1 seek=$sb_bytenr \
> +				skip=$sb_bytenr count=4096 >> $seqres.full 2>&1
> +	echo ..:$? >> $seqres.full

This is an overkill, just use dd ... >/dev/null 2>&1, no one cares about
the output of that. Also use DIO so that sb's are directly written to
the devices.

> +done
> +
> +#Original device is mounted, scan of its clone should fail
> +$BTRFS_UTIL_PROG device scan $dev_bar >> $seqres.full 2>&1
> +echo btrfs device scan dev_bar ...:$?| tee -a $seqres.full
> +
> +echo MNT $(check_btrfs_mount)
> +
> +#Original device scan should be successful
> +$BTRFS_UTIL_PROG device scan $dev_foo >> $seqres.full 2>&1
> +echo btrfs device scan dev_foo ...:$?| tee -a $seqres.full
> +
> +umount $mnt > /dev/null 2>&1
> +_scratch_dev_pool_put
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/173.out b/tests/btrfs/173.out
> new file mode 100644
> index 000000000000..3c7e3fb4e3f7
> --- /dev/null
> +++ b/tests/btrfs/173.out
> @@ -0,0 +1,6 @@
> +QA output created by 173
> +
> +MNT DEV_FOO
> +btrfs device scan dev_bar ...:1
> +MNT DEV_FOO
> +btrfs device scan dev_foo ...:0

It seems the output is really pointless it should just be "Silence is
golden". Then in the test you would do all your run-time checks of
what's the source of the mount etc etc and if your expectations don't
match you fail the test and don't output "Silence is golden" which would
signal success. Check for example

> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index 45782565c3b7..b2f1393f3e97 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -175,3 +175,4 @@
>  170 auto quick snapshot
>  171 auto quick qgroup
>  172 auto quick punch
> +173 volume
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 rev log added] fstests: btrfs verify hardening agaist duplicate fsid
  2018-10-26 15:02   ` Nikolay Borisov
@ 2018-10-26 15:34     ` Anand Jain
  2018-10-26 15:52       ` Nikolay Borisov
  0 siblings, 1 reply; 10+ messages in thread
From: Anand Jain @ 2018-10-26 15:34 UTC (permalink / raw)
  To: Nikolay Borisov, fstests; +Cc: linux-btrfs



On 10/26/2018 11:02 PM, Nikolay Borisov wrote:
> 
> 
> On 8.10.18 г. 21:28 ч., Anand Jain wrote:
>> We have a known bug in btrfs, that we let the device path be changed
>> after the device has been mounted. So using this loop hole the new
>> copied device would appears as if its mounted immediately after its
>> been copied. So this test case reproduces this issue.
>>
>> For example:
>>
>> Initially.. /dev/mmcblk0p4 is mounted as /
>>
>> lsblk
>> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
>> mmcblk0     179:0    0 29.2G  0 disk
>> |-mmcblk0p4 179:4    0    4G  0 part /
>> |-mmcblk0p2 179:2    0  500M  0 part /boot
>> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
>> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
>>
>> btrfs fi show
>> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>>      Total devices 1 FS bytes used 1.40GiB
>>      devid    1 size 4.00GiB used 3.00GiB path /dev/mmcblk0p4
>>
>> Copy mmcblk0 to sda
>> dd if=/dev/mmcblk0 of=/dev/sda
>>
>> And immediately after the copy completes the change in the device
>> superblock is notified which the automount scans using
>> btrfs device scan and the new device sda becomes the mounted root
>> device.
>>
>> lsblk
>> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
>> sda           8:0    1 14.9G  0 disk
>> |-sda4        8:4    1    4G  0 part /
>> |-sda2        8:2    1  500M  0 part
>> |-sda3        8:3    1  256M  0 part
>> `-sda1        8:1    1  256M  0 part
>> mmcblk0     179:0    0 29.2G  0 disk
>> |-mmcblk0p4 179:4    0    4G  0 part
>> |-mmcblk0p2 179:2    0  500M  0 part /boot
>> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
>> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
>> btrfs fi show /
>> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>>      Total devices 1 FS bytes used 1.40GiB
>>      devid    1 size 4.00GiB used 3.00GiB path /dev/sda4
>>
>> The bug is quite nasty that you can't either unmount /dev/sda4 or
>> /dev/mmcblk0p4. And the problem does not get solved until you take
>> the sda out of the system on to another system to change its fsid using
>> the 'btrfstune -u' command.
> 
> Is there a pending fix for this?

Yes.
https://patchwork.kernel.org/patch/10641041/

Test case header mentioned it.

> 
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>> v1->v2:
>>    dont play around with dev patch use it as it is.
>>    do not use SCRATCH_MNT instead create it at the TEST_DIR and its related
>>     changes.
>>    golden out changes
>>     
>>   tests/btrfs/173     | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/btrfs/173.out |  6 ++++
>>   tests/btrfs/group   |  1 +
>>   3 files changed, 95 insertions(+)
>>   create mode 100755 tests/btrfs/173
>>   create mode 100644 tests/btrfs/173.out
>>
>> diff --git a/tests/btrfs/173 b/tests/btrfs/173
>> new file mode 100755
>> index 000000000000..b466ae921e19
>> --- /dev/null
>> +++ b/tests/btrfs/173
>> @@ -0,0 +1,88 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2018 Oracle. All Rights Reserved.
>> +#
>> +# FS QA Test 173
>> +#
>> +# Fuzzy test for FS image duplication.
>> +#  Could be fixed by
>> +#    [patch] btrfs: harden agaist duplicate fsid
>> +#
>> +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
>> +
>> +mnt=$TEST_DIR/$seq.mnt
>> +_cleanup()
>> +{
>> +	rm -rf $mnt > /dev/null 2>&1
>> +	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
>> +
>> +# Modify as appropriate.
>> +_supported_fs btrfs
>> +_supported_os Linux
>> +_require_scratch_dev_pool 2
>> +_scratch_dev_pool_get 2
>> +
>> +dev_foo=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
>> +dev_bar=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
> 
> Naming devices _foo and _bar just shows you could not care less about
> the test.

   Hmm. Will make it device_1 and device_2.

> Either use sane names - primary_dev/secondary dev or device_1
> and device_2.

>> +
>> +echo dev_foo=$dev_foo >> $seqres.full
>> +echo dev_bar=$dev_bar >> $seqres.full
>> +echo | tee -a $seqres.full
>> +
>> +rm -rf $mnt > /dev/null 2>&1
> 
> So what is $mnt? I can see it's used by very few tests and it's not
> obvious? Generally you should either define it as a local variable or
> use one of the SCRATCH_MNT/TEST_MNT global variables. Also I checked
> with Eric re. the use of $mnt in tests and his conclusion is :
> 
> "<sandeen> looks like a bug"

  No its not. As few lines above, I have assigned it as..
     mnt=$TEST_DIR/$seq.mnt


>> +mkdir $mnt
>> +_mkfs_dev $dev_foo
>> +_mount $dev_foo $mnt
>> +
>> +check_btrfs_mount()
>> +{
>> +	local x=$(findmnt $mnt | grep -v TARGET | awk '{print $2}')
>> +	[[ $x == $dev_foo ]] && echo DEV_FOO
>> +	[[ $x == $dev_bar ]] && echo DEV_BAR
>> +}
> 
> Same thing here re. DEV_(FOO|BAR).
> 
>> +
>> +echo MNT $(check_btrfs_mount)
>> +
>> +for sb_bytenr in 65536 67108864
>> +do
>> +	echo -n "dd status=none if=$dev_foo of=$dev_bar bs=1 "\
>> +		"seek=$sb_bytenr skip=$sb_bytenr count=4096" >> $seqres.full
>> +	dd status=none if=$dev_foo of=$dev_bar bs=1 seek=$sb_bytenr \
>> +				skip=$sb_bytenr count=4096 >> $seqres.full 2>&1
>> +	echo ..:$? >> $seqres.full
> 
> This is an overkill, just use dd ... >/dev/null 2>&1, no one cares about
> the output of that. Also use DIO so that sb's are directly written to
> the devices.

  Yeah right will fix.

>> +done
>> +
>> +#Original device is mounted, scan of its clone should fail
>> +$BTRFS_UTIL_PROG device scan $dev_bar >> $seqres.full 2>&1
>> +echo btrfs device scan dev_bar ...:$?| tee -a $seqres.full
>> +
>> +echo MNT $(check_btrfs_mount)
>> +
>> +#Original device scan should be successful
>> +$BTRFS_UTIL_PROG device scan $dev_foo >> $seqres.full 2>&1
>> +echo btrfs device scan dev_foo ...:$?| tee -a $seqres.full
>> +
>> +umount $mnt > /dev/null 2>&1
>> +_scratch_dev_pool_put
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/btrfs/173.out b/tests/btrfs/173.out
>> new file mode 100644
>> index 000000000000..3c7e3fb4e3f7
>> --- /dev/null
>> +++ b/tests/btrfs/173.out
>> @@ -0,0 +1,6 @@
>> +QA output created by 173
>> +
>> +MNT DEV_FOO
>> +btrfs device scan dev_bar ...:1
>> +MNT DEV_FOO
>> +btrfs device scan dev_foo ...:0
> 
> It seems the output is really pointless it should just be "Silence is
> golden". Then in the test you would do all your run-time checks of
> what's the source of the mount etc etc and if your expectations don't
> match you fail the test and don't output "Silence is golden" which would
> signal success. Check for example

  I could do that. Will fix.

Thanks, Anand

>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>> index 45782565c3b7..b2f1393f3e97 100644
>> --- a/tests/btrfs/group
>> +++ b/tests/btrfs/group
>> @@ -175,3 +175,4 @@
>>   170 auto quick snapshot
>>   171 auto quick qgroup
>>   172 auto quick punch
>> +173 volume
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 rev log added] fstests: btrfs verify hardening agaist duplicate fsid
  2018-10-26 15:34     ` Anand Jain
@ 2018-10-26 15:52       ` Nikolay Borisov
  2018-10-26 15:57         ` Anand Jain
  0 siblings, 1 reply; 10+ messages in thread
From: Nikolay Borisov @ 2018-10-26 15:52 UTC (permalink / raw)
  To: Anand Jain, fstests; +Cc: linux-btrfs



On 26.10.18 г. 18:34 ч., Anand Jain wrote:
> 
> 
> On 10/26/2018 11:02 PM, Nikolay Borisov wrote:
>>
>>
>> On 8.10.18 г. 21:28 ч., Anand Jain wrote:
>>> We have a known bug in btrfs, that we let the device path be changed
>>> after the device has been mounted. So using this loop hole the new
>>> copied device would appears as if its mounted immediately after its
>>> been copied. So this test case reproduces this issue.
>>>
>>> For example:
>>>
>>> Initially.. /dev/mmcblk0p4 is mounted as /
>>>
>>> lsblk
>>> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
>>> mmcblk0     179:0    0 29.2G  0 disk
>>> |-mmcblk0p4 179:4    0    4G  0 part /
>>> |-mmcblk0p2 179:2    0  500M  0 part /boot
>>> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
>>> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
>>>
>>> btrfs fi show
>>> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>>>      Total devices 1 FS bytes used 1.40GiB
>>>      devid    1 size 4.00GiB used 3.00GiB path /dev/mmcblk0p4
>>>
>>> Copy mmcblk0 to sda
>>> dd if=/dev/mmcblk0 of=/dev/sda
>>>
>>> And immediately after the copy completes the change in the device
>>> superblock is notified which the automount scans using
>>> btrfs device scan and the new device sda becomes the mounted root
>>> device.
>>>
>>> lsblk
>>> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
>>> sda           8:0    1 14.9G  0 disk
>>> |-sda4        8:4    1    4G  0 part /
>>> |-sda2        8:2    1  500M  0 part
>>> |-sda3        8:3    1  256M  0 part
>>> `-sda1        8:1    1  256M  0 part
>>> mmcblk0     179:0    0 29.2G  0 disk
>>> |-mmcblk0p4 179:4    0    4G  0 part
>>> |-mmcblk0p2 179:2    0  500M  0 part /boot
>>> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
>>> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
>>> btrfs fi show /
>>> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>>>      Total devices 1 FS bytes used 1.40GiB
>>>      devid    1 size 4.00GiB used 3.00GiB path /dev/sda4
>>>
>>> The bug is quite nasty that you can't either unmount /dev/sda4 or
>>> /dev/mmcblk0p4. And the problem does not get solved until you take
>>> the sda out of the system on to another system to change its fsid using
>>> the 'btrfstune -u' command.
>>
>> Is there a pending fix for this?
> 
> Yes.
> https://patchwork.kernel.org/patch/10641041/
> 
> Test case header mentioned it.
> 
>>
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>> v1->v2:
>>>    dont play around with dev patch use it as it is.
>>>    do not use SCRATCH_MNT instead create it at the TEST_DIR and its
>>> related
>>>     changes.
>>>    golden out changes
>>>       tests/btrfs/173     | 88
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   tests/btrfs/173.out |  6 ++++
>>>   tests/btrfs/group   |  1 +
>>>   3 files changed, 95 insertions(+)
>>>   create mode 100755 tests/btrfs/173
>>>   create mode 100644 tests/btrfs/173.out
>>>
>>> diff --git a/tests/btrfs/173 b/tests/btrfs/173
>>> new file mode 100755
>>> index 000000000000..b466ae921e19
>>> --- /dev/null
>>> +++ b/tests/btrfs/173
>>> @@ -0,0 +1,88 @@
>>> +#! /bin/bash
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +# Copyright (c) 2018 Oracle. All Rights Reserved.
>>> +#
>>> +# FS QA Test 173
>>> +#
>>> +# Fuzzy test for FS image duplication.
>>> +#  Could be fixed by
>>> +#    [patch] btrfs: harden agaist duplicate fsid
>>> +#
>>> +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
>>> +
>>> +mnt=$TEST_DIR/$seq.mnt
>>> +_cleanup()
>>> +{
>>> +    rm -rf $mnt > /dev/null 2>&1
>>> +    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
>>> +
>>> +# Modify as appropriate.
>>> +_supported_fs btrfs
>>> +_supported_os Linux
>>> +_require_scratch_dev_pool 2
>>> +_scratch_dev_pool_get 2
>>> +
>>> +dev_foo=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
>>> +dev_bar=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
>>
>> Naming devices _foo and _bar just shows you could not care less about
>> the test.
> 
>   Hmm. Will make it device_1 and device_2.
> 
>> Either use sane names - primary_dev/secondary dev or device_1
>> and device_2.
> 
>>> +
>>> +echo dev_foo=$dev_foo >> $seqres.full
>>> +echo dev_bar=$dev_bar >> $seqres.full
>>> +echo | tee -a $seqres.full
>>> +
>>> +rm -rf $mnt > /dev/null 2>&1
>>
>> So what is $mnt? I can see it's used by very few tests and it's not
>> obvious? Generally you should either define it as a local variable or
>> use one of the SCRATCH_MNT/TEST_MNT global variables. Also I checked
>> with Eric re. the use of $mnt in tests and his conclusion is :
>>
>> "<sandeen> looks like a bug"
> 
>  No its not. As few lines above, I have assigned it as..
>     mnt=$TEST_DIR/$seq.mnt

I missed that, I will recommend moving the assignment near where you set
the devices

> 
> 
>>> +mkdir $mnt
>>> +_mkfs_dev $dev_foo
>>> +_mount $dev_foo $mnt
>>> +
>>> +check_btrfs_mount()
>>> +{
>>> +    local x=$(findmnt $mnt | grep -v TARGET | awk '{print $2}')
>>> +    [[ $x == $dev_foo ]] && echo DEV_FOO
>>> +    [[ $x == $dev_bar ]] && echo DEV_BAR
>>> +}
>>
>> Same thing here re. DEV_(FOO|BAR).
>>
>>> +
>>> +echo MNT $(check_btrfs_mount)
>>> +
>>> +for sb_bytenr in 65536 67108864
>>> +do
>>> +    echo -n "dd status=none if=$dev_foo of=$dev_bar bs=1 "\
>>> +        "seek=$sb_bytenr skip=$sb_bytenr count=4096" >> $seqres.full
>>> +    dd status=none if=$dev_foo of=$dev_bar bs=1 seek=$sb_bytenr \
>>> +                skip=$sb_bytenr count=4096 >> $seqres.full 2>&1
>>> +    echo ..:$? >> $seqres.full
>>
>> This is an overkill, just use dd ... >/dev/null 2>&1, no one cares about
>> the output of that. Also use DIO so that sb's are directly written to
>> the devices.
> 
>  Yeah right will fix.
> 
>>> +done
>>> +
>>> +#Original device is mounted, scan of its clone should fail
>>> +$BTRFS_UTIL_PROG device scan $dev_bar >> $seqres.full 2>&1
>>> +echo btrfs device scan dev_bar ...:$?| tee -a $seqres.full
>>> +
>>> +echo MNT $(check_btrfs_mount)
>>> +
>>> +#Original device scan should be successful
>>> +$BTRFS_UTIL_PROG device scan $dev_foo >> $seqres.full 2>&1
>>> +echo btrfs device scan dev_foo ...:$?| tee -a $seqres.full
>>> +
>>> +umount $mnt > /dev/null 2>&1
>>> +_scratch_dev_pool_put
>>> +
>>> +# success, all done
>>> +status=0
>>> +exit
>>> diff --git a/tests/btrfs/173.out b/tests/btrfs/173.out
>>> new file mode 100644
>>> index 000000000000..3c7e3fb4e3f7
>>> --- /dev/null
>>> +++ b/tests/btrfs/173.out
>>> @@ -0,0 +1,6 @@
>>> +QA output created by 173
>>> +
>>> +MNT DEV_FOO
>>> +btrfs device scan dev_bar ...:1
>>> +MNT DEV_FOO
>>> +btrfs device scan dev_foo ...:0
>>
>> It seems the output is really pointless it should just be "Silence is
>> golden". Then in the test you would do all your run-time checks of
>> what's the source of the mount etc etc and if your expectations don't
>> match you fail the test and don't output "Silence is golden" which would
>> signal success. Check for example
> 
>  I could do that. Will fix.
> 
> Thanks, Anand
> 
>>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>>> index 45782565c3b7..b2f1393f3e97 100644
>>> --- a/tests/btrfs/group
>>> +++ b/tests/btrfs/group
>>> @@ -175,3 +175,4 @@
>>>   170 auto quick snapshot
>>>   171 auto quick qgroup
>>>   172 auto quick punch
>>> +173 volume
>>>
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 rev log added] fstests: btrfs verify hardening agaist duplicate fsid
  2018-10-26 15:52       ` Nikolay Borisov
@ 2018-10-26 15:57         ` Anand Jain
  0 siblings, 0 replies; 10+ messages in thread
From: Anand Jain @ 2018-10-26 15:57 UTC (permalink / raw)
  To: Nikolay Borisov, fstests; +Cc: linux-btrfs



On 10/26/2018 11:52 PM, Nikolay Borisov wrote:
> 
> 
> On 26.10.18 г. 18:34 ч., Anand Jain wrote:
>>
>>
>> On 10/26/2018 11:02 PM, Nikolay Borisov wrote:
>>>
>>>
>>> On 8.10.18 г. 21:28 ч., Anand Jain wrote:
>>>> We have a known bug in btrfs, that we let the device path be changed
>>>> after the device has been mounted. So using this loop hole the new
>>>> copied device would appears as if its mounted immediately after its
>>>> been copied. So this test case reproduces this issue.
>>>>
>>>> For example:
>>>>
>>>> Initially.. /dev/mmcblk0p4 is mounted as /
>>>>
>>>> lsblk
>>>> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
>>>> mmcblk0     179:0    0 29.2G  0 disk
>>>> |-mmcblk0p4 179:4    0    4G  0 part /
>>>> |-mmcblk0p2 179:2    0  500M  0 part /boot
>>>> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
>>>> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
>>>>
>>>> btrfs fi show
>>>> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>>>>       Total devices 1 FS bytes used 1.40GiB
>>>>       devid    1 size 4.00GiB used 3.00GiB path /dev/mmcblk0p4
>>>>
>>>> Copy mmcblk0 to sda
>>>> dd if=/dev/mmcblk0 of=/dev/sda
>>>>
>>>> And immediately after the copy completes the change in the device
>>>> superblock is notified which the automount scans using
>>>> btrfs device scan and the new device sda becomes the mounted root
>>>> device.
>>>>
>>>> lsblk
>>>> NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
>>>> sda           8:0    1 14.9G  0 disk
>>>> |-sda4        8:4    1    4G  0 part /
>>>> |-sda2        8:2    1  500M  0 part
>>>> |-sda3        8:3    1  256M  0 part
>>>> `-sda1        8:1    1  256M  0 part
>>>> mmcblk0     179:0    0 29.2G  0 disk
>>>> |-mmcblk0p4 179:4    0    4G  0 part
>>>> |-mmcblk0p2 179:2    0  500M  0 part /boot
>>>> |-mmcblk0p3 179:3    0  256M  0 part [SWAP]
>>>> `-mmcblk0p1 179:1    0  256M  0 part /boot/efi
>>>> btrfs fi show /
>>>> Label: none  uuid: 07892354-ddaa-4443-90ea-f76a06accaba
>>>>       Total devices 1 FS bytes used 1.40GiB
>>>>       devid    1 size 4.00GiB used 3.00GiB path /dev/sda4
>>>>
>>>> The bug is quite nasty that you can't either unmount /dev/sda4 or
>>>> /dev/mmcblk0p4. And the problem does not get solved until you take
>>>> the sda out of the system on to another system to change its fsid using
>>>> the 'btrfstune -u' command.
>>>
>>> Is there a pending fix for this?
>>
>> Yes.
>> https://patchwork.kernel.org/patch/10641041/
>>
>> Test case header mentioned it.
>>
>>>
>>>>
>>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>>> ---
>>>> v1->v2:
>>>>     dont play around with dev patch use it as it is.
>>>>     do not use SCRATCH_MNT instead create it at the TEST_DIR and its
>>>> related
>>>>      changes.
>>>>     golden out changes
>>>>        tests/btrfs/173     | 88
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    tests/btrfs/173.out |  6 ++++
>>>>    tests/btrfs/group   |  1 +
>>>>    3 files changed, 95 insertions(+)
>>>>    create mode 100755 tests/btrfs/173
>>>>    create mode 100644 tests/btrfs/173.out
>>>>
>>>> diff --git a/tests/btrfs/173 b/tests/btrfs/173
>>>> new file mode 100755
>>>> index 000000000000..b466ae921e19
>>>> --- /dev/null
>>>> +++ b/tests/btrfs/173
>>>> @@ -0,0 +1,88 @@
>>>> +#! /bin/bash
>>>> +# SPDX-License-Identifier: GPL-2.0
>>>> +# Copyright (c) 2018 Oracle. All Rights Reserved.
>>>> +#
>>>> +# FS QA Test 173
>>>> +#
>>>> +# Fuzzy test for FS image duplication.
>>>> +#  Could be fixed by
>>>> +#    [patch] btrfs: harden agaist duplicate fsid
>>>> +#
>>>> +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
>>>> +
>>>> +mnt=$TEST_DIR/$seq.mnt
>>>> +_cleanup()
>>>> +{
>>>> +    rm -rf $mnt > /dev/null 2>&1
>>>> +    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
>>>> +
>>>> +# Modify as appropriate.
>>>> +_supported_fs btrfs
>>>> +_supported_os Linux
>>>> +_require_scratch_dev_pool 2
>>>> +_scratch_dev_pool_get 2
>>>> +
>>>> +dev_foo=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
>>>> +dev_bar=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
>>>
>>> Naming devices _foo and _bar just shows you could not care less about
>>> the test.
>>
>>    Hmm. Will make it device_1 and device_2.
>>
>>> Either use sane names - primary_dev/secondary dev or device_1
>>> and device_2.
>>
>>>> +
>>>> +echo dev_foo=$dev_foo >> $seqres.full
>>>> +echo dev_bar=$dev_bar >> $seqres.full
>>>> +echo | tee -a $seqres.full
>>>> +
>>>> +rm -rf $mnt > /dev/null 2>&1
>>>
>>> So what is $mnt? I can see it's used by very few tests and it's not
>>> obvious? Generally you should either define it as a local variable or
>>> use one of the SCRATCH_MNT/TEST_MNT global variables. Also I checked
>>> with Eric re. the use of $mnt in tests and his conclusion is :
>>>
>>> "<sandeen> looks like a bug"
>>
>>   No its not. As few lines above, I have assigned it as..
>>      mnt=$TEST_DIR/$seq.mnt
> 
> I missed that, I will recommend moving the assignment near where you set
> the devices

  cleanup() is using $mnt. cleanup() may get called for any trap before 
the actual test.

Thanks, Anand


>>
>>
>>>> +mkdir $mnt
>>>> +_mkfs_dev $dev_foo
>>>> +_mount $dev_foo $mnt
>>>> +
>>>> +check_btrfs_mount()
>>>> +{
>>>> +    local x=$(findmnt $mnt | grep -v TARGET | awk '{print $2}')
>>>> +    [[ $x == $dev_foo ]] && echo DEV_FOO
>>>> +    [[ $x == $dev_bar ]] && echo DEV_BAR
>>>> +}
>>>
>>> Same thing here re. DEV_(FOO|BAR).
>>>
>>>> +
>>>> +echo MNT $(check_btrfs_mount)
>>>> +
>>>> +for sb_bytenr in 65536 67108864
>>>> +do
>>>> +    echo -n "dd status=none if=$dev_foo of=$dev_bar bs=1 "\
>>>> +        "seek=$sb_bytenr skip=$sb_bytenr count=4096" >> $seqres.full
>>>> +    dd status=none if=$dev_foo of=$dev_bar bs=1 seek=$sb_bytenr \
>>>> +                skip=$sb_bytenr count=4096 >> $seqres.full 2>&1
>>>> +    echo ..:$? >> $seqres.full
>>>
>>> This is an overkill, just use dd ... >/dev/null 2>&1, no one cares about
>>> the output of that. Also use DIO so that sb's are directly written to
>>> the devices.
>>
>>   Yeah right will fix.
>>
>>>> +done
>>>> +
>>>> +#Original device is mounted, scan of its clone should fail
>>>> +$BTRFS_UTIL_PROG device scan $dev_bar >> $seqres.full 2>&1
>>>> +echo btrfs device scan dev_bar ...:$?| tee -a $seqres.full
>>>> +
>>>> +echo MNT $(check_btrfs_mount)
>>>> +
>>>> +#Original device scan should be successful
>>>> +$BTRFS_UTIL_PROG device scan $dev_foo >> $seqres.full 2>&1
>>>> +echo btrfs device scan dev_foo ...:$?| tee -a $seqres.full
>>>> +
>>>> +umount $mnt > /dev/null 2>&1
>>>> +_scratch_dev_pool_put
>>>> +
>>>> +# success, all done
>>>> +status=0
>>>> +exit
>>>> diff --git a/tests/btrfs/173.out b/tests/btrfs/173.out
>>>> new file mode 100644
>>>> index 000000000000..3c7e3fb4e3f7
>>>> --- /dev/null
>>>> +++ b/tests/btrfs/173.out
>>>> @@ -0,0 +1,6 @@
>>>> +QA output created by 173
>>>> +
>>>> +MNT DEV_FOO
>>>> +btrfs device scan dev_bar ...:1
>>>> +MNT DEV_FOO
>>>> +btrfs device scan dev_foo ...:0
>>>
>>> It seems the output is really pointless it should just be "Silence is
>>> golden". Then in the test you would do all your run-time checks of
>>> what's the source of the mount etc etc and if your expectations don't
>>> match you fail the test and don't output "Silence is golden" which would
>>> signal success. Check for example
>>
>>   I could do that. Will fix.
>>
>> Thanks, Anand
>>
>>>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>>>> index 45782565c3b7..b2f1393f3e97 100644
>>>> --- a/tests/btrfs/group
>>>> +++ b/tests/btrfs/group
>>>> @@ -175,3 +175,4 @@
>>>>    170 auto quick snapshot
>>>>    171 auto quick qgroup
>>>>    172 auto quick punch
>>>> +173 volume
>>>>
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-10-26 15:57 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-01  8:44 [PATCH] fstests: btrfs verify hardening agaist duplicate fsid Anand Jain
2018-10-06 10:14 ` Eryu Guan
2018-10-08 18:28   ` Anand Jain
2018-10-08 18:14 ` [PATCH v2] " Anand Jain
2018-10-08 18:28 ` [PATCH v2 rev log added] " Anand Jain
2018-10-21 10:25   ` Eryu Guan
2018-10-26 15:02   ` Nikolay Borisov
2018-10-26 15:34     ` Anand Jain
2018-10-26 15:52       ` Nikolay Borisov
2018-10-26 15:57         ` Anand Jain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.