From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 50728C6786E for ; Fri, 26 Oct 2018 15:34:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E44C02085B for ; Fri, 26 Oct 2018 15:34:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=oracle.com header.i=@oracle.com header.b="jC7eoBRO" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E44C02085B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=oracle.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-btrfs-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726198AbeJ0ALr (ORCPT ); Fri, 26 Oct 2018 20:11:47 -0400 Received: from userp2120.oracle.com ([156.151.31.85]:57254 "EHLO userp2120.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726159AbeJ0ALr (ORCPT ); Fri, 26 Oct 2018 20:11:47 -0400 Received: from pps.filterd (userp2120.oracle.com [127.0.0.1]) by userp2120.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w9QFSfHY123395; Fri, 26 Oct 2018 15:34:13 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=corp-2018-07-02; bh=AfK0Iq87tX4u9SIPCR5wF/QGlnRelUsEglFvW0nCDUY=; b=jC7eoBRO4lcC3oX4W+RV71ZDcGh+kk69GrGb0Jgx6/ykvF9q+MnbaSL2YF0po+MQDtzT 9n94ogE68s5RcZZIlryp72ug2gddZd4AB13qc62iw/izXigIi7TXWoeqYfWBns7pbNoF P3XC6Bs/dcXh0qO/+TtzUPKkuWD4pjgAPMPEWNDSsmPqSdcZoV5YiA5qVbRBNbdUdnfa 8Mn7zq5u82e7XJrl9gehm+C58V8a4OQmLNJOVoLGg09bjrA0YTx67Lja19lh5QJHoCJp OGYM0R8r11r6761Qwamss7nN2TZo2xAgOXRQHUQQq9PBwKiO9/0+/kFIIKH8f+93uEC5 hw== Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by userp2120.oracle.com with ESMTP id 2n7w0r7wq5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 26 Oct 2018 15:34:13 +0000 Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id w9QFYC4C012418 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 26 Oct 2018 15:34:12 GMT Received: from abhmp0016.oracle.com (abhmp0016.oracle.com [141.146.116.22]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id w9QFYCDQ014921; Fri, 26 Oct 2018 15:34:12 GMT Received: from [172.20.10.2] (/183.90.36.85) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 26 Oct 2018 08:34:11 -0700 Subject: Re: [PATCH v2 rev log added] fstests: btrfs verify hardening agaist duplicate fsid To: Nikolay Borisov , fstests@vger.kernel.org Cc: linux-btrfs@vger.kernel.org References: <1538383475-2532-1-git-send-email-anand.jain@oracle.com> <1539023301-4583-1-git-send-email-anand.jain@oracle.com> From: Anand Jain Message-ID: <6b58c095-d0a1-6330-0385-b06aa9c99483@oracle.com> Date: Fri, 26 Oct 2018 23:34:04 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=9057 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1810260130 Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org 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 >> --- >> 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 : > > " 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 >>