| Message ID | 20220813060848.1457301-1-yukuai1@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fs: fix possible inconsistent mount device | expand |
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.
在 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
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
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