[PATCH] fs: fix possible inconsistent mount device

[PATCH] fs: fix possible inconsistent mount device

[Yu Kuai]

From: Yu Kuai <yukuai3@huawei.com>

If device support rename, for example dm, following concurrent scenario
is possible:

t1: mount		t2: rename

mount_bdev
 blkdev_get_by_path
  lookup_bdev(path, &dev);
			dm_rename
			// change device name
   blkdev_get_by_dev
...
vfs_create_mount
 alloc_vfsmnt
  // mnt->mnt_devname is set to old name

Test cmd:
dmsetup create test1 --table "0 100000 linear /dev/sda 0"
mkfs.ext2 -F /dev/mapper/test1
mount /dev/mapper/test1 /mnt/test &
dmsetup rename test1 test2

Test result(If the above scenario triggers):
mount will show test1 is mounted:
[root@localhost ~]# mount | grep test
/dev/mapper/test1 on /mnt/test type ext2 (rw,relatime,errors=continue,user_xattr,acl)

while lsblk show test2 is mounted:
[root@localhost ~]# lsblk /dev/sda
NAME    MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
sda       8:0    0  100G  0 disk
└─test2 252:0    0 48.8M  0 dm   /mnt/test

Furthermore, if a new device with name test1 is created, lsblk will show
that it's mounted:
[root@localhost ~]# dmsetup create test1 --table "0 100000 linear /dev/sdb 0" && lsblk /dev/sdb
NAME    MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
sdb       8:16   0   10G  0 disk
└─test1 252:1    0 48.8M  0 dm   /mnt/test

Fix the problem by checking dev_name again after bdev if found by
blkdev_get_by_path() in mount_bdev().

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
changes in v2: rebase to latest version
 fs/super.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/fs/super.c b/fs/super.c
index 734ed584a946..c738da299fc1 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1341,6 +1341,20 @@ static int test_bdev_super(struct super_block *s, void *data)
 	return !(s->s_iflags & SB_I_RETIRED) && (void *)s->s_bdev == data;
 }
 
+static int check_dev_name(struct block_device *bdev, const char *dev_name)
+{
+	dev_t dev;
+	int error = lookup_bdev(dev_name, &dev);
+
+	if (error)
+		return error;
+
+	if (dev != bdev->bd_dev)
+		return -EBUSY;
+
+	return 0;
+}
+
 struct dentry *mount_bdev(struct file_system_type *fs_type,
 	int flags, const char *dev_name, void *data,
 	int (*fill_super)(struct super_block *, void *, int))
@@ -1357,6 +1371,15 @@ struct dentry *mount_bdev(struct file_system_type *fs_type,
 	if (IS_ERR(bdev))
 		return ERR_CAST(bdev);
 
+	/*
+	 * Some drivers support device rename, for example dm, make sure the
+	 * right bdev is found, otherwise inconsistent device and mountpoint
+	 * will be shown in /proc/mounts.
+	 */
+	error = check_dev_name(bdev, dev_name);
+	if (error)
+		goto error_bdev;
+
 	/*
 	 * once the super is inserted into the list by sget, s_umount
 	 * will protect the lockfs code from trying to start a snapshot
-- 
2.31.1

Re: [PATCH] fs: fix possible inconsistent mount device

[Christoph Hellwig]

On Sat, Aug 13, 2022 at 02:08:48PM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> If device support rename, for example dm, following concurrent scenario
> is possible:

The fix is easy:  don't rename mounted block device, and even
bettersimply don't rename them at all, which is just causing huge
amounts of pain.

Re: [PATCH] fs: fix possible inconsistent mount device

[Yu Kuai]

Hi, Christoph!

在 2022/08/13 14:48, Christoph Hellwig 写道:
> On Sat, Aug 13, 2022 at 02:08:48PM +0800, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> If device support rename, for example dm, following concurrent scenario
>> is possible:
> 
> The fix is easy:  don't rename mounted block device, and even
> bettersimply don't rename them at all, which is just causing huge
> amounts of pain.

Thanks for your reply. Do you think it's better to remove the rename
support from dm? Or it's better to add such limit?

Best regards,
Kuai

Re: [PATCH] fs: fix possible inconsistent mount device

[Christoph Hellwig]

On Sat, Aug 13, 2022 at 03:09:58PM +0800, Yu Kuai wrote:
> Thanks for your reply. Do you think it's better to remove the rename
> support from dm? Or it's better to add such limit?

It will probably be hard to entirely remove it.  But documentation
and a rate limited warning discouraging it seems like a good idea.

Re: [PATCH] fs: fix possible inconsistent mount device

[Yu Kuai]

在 2022/08/13 15:15, Christoph Hellwig 写道:
> On Sat, Aug 13, 2022 at 03:09:58PM +0800, Yu Kuai wrote:
>> Thanks for your reply. Do you think it's better to remove the rename
>> support from dm? Or it's better to add such limit?
> 
> It will probably be hard to entirely remove it.  But documentation
> and a rate limited warning discouraging it seems like a good idea.

Yes, that's a good idea.

Thanks,
Kuai

Re: [PATCH] fs: fix possible inconsistent mount device

[Yu Kuai]

Hi, Christoph!

在 2022/08/13 15:15, Christoph Hellwig 写道:
> On Sat, Aug 13, 2022 at 03:09:58PM +0800, Yu Kuai wrote:
>> Thanks for your reply. Do you think it's better to remove the rename
>> support from dm? Or it's better to add such limit?
> 
> It will probably be hard to entirely remove it.  But documentation
> and a rate limited warning discouraging it seems like a good idea.
> .
> 

I just found that not just rename, mount concurrent with device
remove/create can trigger this problem as well:

t1:                       t2
// create dm-0 with name test1
// mount /dev/mapper/test1
mount_bdev
  blkdev_get_by_path
   lookup_bdev
                           // remove dm-0
                           // create dm-0 with different name test2
   blkdev_get_by_dev
   // succeed

Do you think it's ok to add such checking to prevent this problem?

Thanks,
Kuai