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

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.

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

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.

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

在 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