Re: dm snapshot: disallow the COW and origin devices from being identical

[dingxiang <dingxiang@huawei.com>]

please ignore the previous mail, it's a mistake,sorry about it.:-[


2016/3/14 12:09, DingXiang wrote:
> Otherwise loading a "snapshot" table using the same device for the
> origin and COW devices, e.g.:
>
> echo "0 20971520 snapshot 253:3 253:3 P 8" | dmsetup create snap
>
> will trigger:
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000098
> [ 1958.979934] IP: [<ffffffffa040efba>] dm_exception_store_set_chunk_size+0x7a/0x110 [dm_snapshot]
> [ 1958.989655] PGD 0
> [ 1958.991903] Oops: 0000 [#1] SMP
> ...
> [ 1959.059647] CPU: 9 PID: 3556 Comm: dmsetup Tainted: G          IO    4.5.0-rc5.snitm+ #150
> ...
> [ 1959.083517] task: ffff8800b9660c80 ti: ffff88032a954000 task.ti: ffff88032a954000
> [ 1959.091865] RIP: 0010:[<ffffffffa040efba>]  [<ffffffffa040efba>] dm_exception_store_set_chunk_size+0x7a/0x110 [dm_snapshot]
> [ 1959.104295] RSP: 0018:ffff88032a957b30  EFLAGS: 00010246
> [ 1959.110219] RAX: 0000000000000000 RBX: 0000000000000008 RCX: 0000000000000001
> [ 1959.118180] RDX: 0000000000000000 RSI: 0000000000000008 RDI: ffff880329334a00
> [ 1959.126141] RBP: ffff88032a957b50 R08: 0000000000000000 R09: 0000000000000001
> [ 1959.134102] R10: 000000000000000a R11: f000000000000000 R12: ffff880330884d80
> [ 1959.142061] R13: 0000000000000008 R14: ffffc90001c13088 R15: ffff880330884d80
> [ 1959.150021] FS:  00007f8926ba3840(0000) GS:ffff880333440000(0000) knlGS:0000000000000000
> [ 1959.159047] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1959.165456] CR2: 0000000000000098 CR3: 000000032f48b000 CR4: 00000000000006e0
> [ 1959.173415] Stack:
> [ 1959.175656]  ffffc90001c13040 ffff880329334a00 ffff880330884ed0 ffff88032a957bdc
> [ 1959.183946]  ffff88032a957bb8 ffffffffa040f225 ffff880329334a30 ffff880300000000
> [ 1959.192233]  ffffffffa04133e0 ffff880329334b30 0000000830884d58 00000000569c58cf
> [ 1959.200521] Call Trace:
> [ 1959.203248]  [<ffffffffa040f225>] dm_exception_store_create+0x1d5/0x240 [dm_snapshot]
> [ 1959.211986]  [<ffffffffa040d310>] snapshot_ctr+0x140/0x630 [dm_snapshot]
> [ 1959.219469]  [<ffffffffa0005c44>] ? dm_split_args+0x64/0x150 [dm_mod]
> [ 1959.226656]  [<ffffffffa0005ea7>] dm_table_add_target+0x177/0x440 [dm_mod]
> [ 1959.234328]  [<ffffffffa0009203>] table_load+0x143/0x370 [dm_mod]
> [ 1959.241129]  [<ffffffffa00090c0>] ? retrieve_status+0x1b0/0x1b0 [dm_mod]
> [ 1959.248607]  [<ffffffffa0009e35>] ctl_ioctl+0x255/0x4d0 [dm_mod]
> [ 1959.255307]  [<ffffffff813304e2>] ? memzero_explicit+0x12/0x20
> [ 1959.261816]  [<ffffffffa000a0c3>] dm_ctl_ioctl+0x13/0x20 [dm_mod]
> [ 1959.268615]  [<ffffffff81215eb6>] do_vfs_ioctl+0xa6/0x5c0
> [ 1959.274637]  [<ffffffff81120d2f>] ? __audit_syscall_entry+0xaf/0x100
> [ 1959.281726]  [<ffffffff81003176>] ? do_audit_syscall_entry+0x66/0x70
> [ 1959.288814]  [<ffffffff81216449>] SyS_ioctl+0x79/0x90
> [ 1959.294450]  [<ffffffff8167e4ae>] entry_SYSCALL_64_fastpath+0x12/0x71
> ...
> [ 1959.323277] RIP  [<ffffffffa040efba>] dm_exception_store_set_chunk_size+0x7a/0x110 [dm_snapshot]
> [ 1959.333090]  RSP <ffff88032a957b30>
> [ 1959.336978] CR2: 0000000000000098
> [ 1959.344121] ---[ end trace b049991ccad1169e ]---
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1195899
> Cc: stable@vger.kernel.org
> Signed-off-by: Ding Xiang <dingxiang@huawei.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> ---
>   drivers/md/dm-snap.c          |  9 +++++++++
>   drivers/md/dm-table.c         | 36 ++++++++++++++++++++++++------------
>   include/linux/device-mapper.h |  2 ++
>   3 files changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
> index 62479ac..70bb0e8 100644
> --- a/drivers/md/dm-snap.c
> +++ b/drivers/md/dm-snap.c
> @@ -1105,6 +1105,7 @@ static int snapshot_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>   	int i;
>   	int r = -EINVAL;
>   	char *origin_path, *cow_path;
> +	dev_t origin_dev, cow_dev;
>   	unsigned args_used, num_flush_bios = 1;
>   	fmode_t origin_mode = FMODE_READ;
>   
> @@ -1135,11 +1136,19 @@ static int snapshot_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>   		ti->error = "Cannot get origin device";
>   		goto bad_origin;
>   	}
> +	origin_dev = s->origin->bdev->bd_dev;
>   
>   	cow_path = argv[0];
>   	argv++;
>   	argc--;
>   
> +	cow_dev = dm_get_dev_t(cow_path);
> +	if (cow_dev && cow_dev == origin_dev) {
> +		ti->error = "COW device cannot be the same as origin device";
> +		r = -EINVAL;
> +		goto bad_cow;
> +	}
> +
>   	r = dm_get_device(ti, cow_path, dm_table_get_mode(ti->table), &s->cow);
>   	if (r) {
>   		ti->error = "Cannot get COW device";
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 7210e53..f9e8f0b 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -365,6 +365,26 @@ static int upgrade_mode(struct dm_dev_internal *dd, fmode_t new_mode,
>   }
>   
>   /*
> + * Convert the path to a device
> + */
> +dev_t dm_get_dev_t(const char *path)
> +{
> +	dev_t uninitialized_var(dev);
> +	struct block_device *bdev;
> +
> +	bdev = lookup_bdev(path);
> +	if (IS_ERR(bdev))
> +		dev = name_to_dev_t(path);
> +	else {
> +		dev = bdev->bd_dev;
> +		bdput(bdev);
> +	}
> +
> +	return dev;
> +}
> +EXPORT_SYMBOL_GPL(dm_get_dev_t);
> +
> +/*
>    * Add a device to the list, or just increment the usage count if
>    * it's already present.
>    */
> @@ -372,23 +392,15 @@ int dm_get_device(struct dm_target *ti, const char *path, fmode_t mode,
>   		  struct dm_dev **result)
>   {
>   	int r;
> -	dev_t uninitialized_var(dev);
> +	dev_t dev;
>   	struct dm_dev_internal *dd;
>   	struct dm_table *t = ti->table;
> -	struct block_device *bdev;
>   
>   	BUG_ON(!t);
>   
> -	/* convert the path to a device */
> -	bdev = lookup_bdev(path);
> -	if (IS_ERR(bdev)) {
> -		dev = name_to_dev_t(path);
> -		if (!dev)
> -			return -ENODEV;
> -	} else {
> -		dev = bdev->bd_dev;
> -		bdput(bdev);
> -	}
> +	dev = dm_get_dev_t(path);
> +	if (!dev)
> +		return -ENODEV;
>   
>   	dd = find_device(&t->devices, dev);
>   	if (!dd) {
> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> index 82ae3b5..0830c9e 100644
> --- a/include/linux/device-mapper.h
> +++ b/include/linux/device-mapper.h
> @@ -124,6 +124,8 @@ struct dm_dev {
>   	char name[16];
>   };
>   
> +dev_t dm_get_dev_t(const char *path);
> +
>   /*
>    * Constructors should call these functions to ensure destination devices
>    * are opened/closed correctly.