linux-kernel - Re: [PATCH]btrfs: Fix fstest case btrfs/219

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0c2337c3-2b39-c54c-27e9-dd4f0a99cf71@suse.com>
Date:   Thu, 21 Jul 2022 16:37:43 +0300
From:   Nikolay Borisov <nborisov@...e.com>
To:     "Flint.Wang" <hmsjwzb@...o.com>, anand.jain@...cle.com
Cc:     Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
        David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH]btrfs: Fix fstest case btrfs/219



On 21.07.22 г. 11:36 ч., Flint.Wang wrote:
> Hi,
> fstest btrfs/291 failed.
> 
> [How to reproduce]
> mkdir -p /mnt/test/219.mnt
> xfs_io -f -c "truncate 256m" /mnt/test/219.img1
> mkfs.btrfs /mnt/test/219.img1
> cp /mnt/test/219.img1 /mnt/test/219.img2
> mount -o loop /mnt/test/219.img1 /mnt/test/219.mnt
> umount /mnt/test/219.mnt
> losetup -f --show /mnt/test/219.img1 dev
> mount /dev/loop0 /mnt/test/219.mnt
> umount /mnt/test/219.mnt
> mount -o loop /mnt/test/219.img2 /mnt/test/219.mnt
> 
> [Root cause]
> if (fs_devices->opened && found_transid < device->generation) {
> 	/*
> 	 * That is if the FS is _not_ mounted and if you
> 	 * are here, that means there is more than one
> 	 * disk with same uuid and devid.We keep the one
> 	 * with larger generation number or the last-in if
> 	 * generation are equal.
> 	 */
> 	mutex_unlock(&fs_devices->device_list_mutex);
> 	return ERR_PTR(-EEXIST);
> }
> 
> [Personal opinion]
> User might back up a block device to another. I think it is improper
> to forbid user from mounting it.
> 
> Signed-off-by: Flint.Wang <hmsjwzb@...o.com>


This lacks any explanation whatsoever so it's not possible to judge 
whether the fix is correct or not.

> ---
>   fs/btrfs/volumes.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 6aa6bc769569a..76af32032ac85 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -900,7 +900,7 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>   		 * tracking a problem where systems fail mount by subvolume id
>   		 * when we reject replacement on a mounted FS.
>   		 */
> -		if (!fs_devices->opened && found_transid < device->generation) {
> +		if (fs_devices->opened && found_transid < device->generation) {
>   			/*
>   			 * That is if the FS is _not_ mounted and if you
>   			 * are here, that means there is more than one