Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754425AbcDZXUi (ORCPT ); Tue, 26 Apr 2016 19:20:38 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:40719 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753316AbcDZXUe (ORCPT ); Tue, 26 Apr 2016 19:20:34 -0400 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 From: Ben Hutchings To: linux-kernel@vger.kernel.org, stable@vger.kernel.org CC: akpm@linux-foundation.org, "DingXiang" , "Mike Snitzer" Date: Wed, 27 Apr 2016 01:02:24 +0200 Message-ID: X-Mailer: LinuxStableQueue (scripts by bwh) Subject: [PATCH 3.2 029/115] dm snapshot: disallow the COW and origin devices from being identical In-Reply-To: X-SA-Exim-Connect-IP: 2a02:8426:ae4:c500:9cba:69ae:962d:6167 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6071 Lines: 170 3.2.80-rc1 review patch. If anyone has any objections, please let me know. ------------------ From: DingXiang commit 4df2bf466a9c9c92f40d27c4aa9120f4e8227bfc upstream. 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: [] 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:[] [] 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] [] dm_exception_store_create+0x1d5/0x240 [dm_snapshot] [ 1959.211986] [] snapshot_ctr+0x140/0x630 [dm_snapshot] [ 1959.219469] [] ? dm_split_args+0x64/0x150 [dm_mod] [ 1959.226656] [] dm_table_add_target+0x177/0x440 [dm_mod] [ 1959.234328] [] table_load+0x143/0x370 [dm_mod] [ 1959.241129] [] ? retrieve_status+0x1b0/0x1b0 [dm_mod] [ 1959.248607] [] ctl_ioctl+0x255/0x4d0 [dm_mod] [ 1959.255307] [] ? memzero_explicit+0x12/0x20 [ 1959.261816] [] dm_ctl_ioctl+0x13/0x20 [dm_mod] [ 1959.268615] [] do_vfs_ioctl+0xa6/0x5c0 [ 1959.274637] [] ? __audit_syscall_entry+0xaf/0x100 [ 1959.281726] [] ? do_audit_syscall_entry+0x66/0x70 [ 1959.288814] [] SyS_ioctl+0x79/0x90 [ 1959.294450] [] entry_SYSCALL_64_fastpath+0x12/0x71 ... [ 1959.323277] RIP [] dm_exception_store_set_chunk_size+0x7a/0x110 [dm_snapshot] [ 1959.333090] RSP [ 1959.336978] CR2: 0000000000000098 [ 1959.344121] ---[ end trace b049991ccad1169e ]--- Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1195899 Signed-off-by: Ding Xiang Signed-off-by: Mike Snitzer [bwh: Backported to 3.2: the device path parsing code is rather different, but move it into dm_get_dev_t() anyway] Signed-off-by: Ben Hutchings --- --- a/drivers/md/dm-snap.c +++ b/drivers/md/dm-snap.c @@ -1055,6 +1055,7 @@ static int snapshot_ctr(struct dm_target int i; int r = -EINVAL; char *origin_path, *cow_path; + dev_t origin_dev, cow_dev; unsigned args_used, num_flush_requests = 1; fmode_t origin_mode = FMODE_READ; @@ -1085,11 +1086,19 @@ static int snapshot_ctr(struct dm_target 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"; --- a/drivers/md/dm-table.c +++ b/drivers/md/dm-table.c @@ -458,35 +458,50 @@ static int upgrade_mode(struct dm_dev_in } /* - * Add a device to the list, or just increment the usage count if - * it's already present. + * Convert the path to a device */ -int dm_get_device(struct dm_target *ti, const char *path, fmode_t mode, - struct dm_dev **result) +dev_t dm_get_dev_t(const char *path) { - int r; dev_t uninitialized_var(dev); - struct dm_dev_internal *dd; unsigned int major, minor; - struct dm_table *t = ti->table; - - BUG_ON(!t); if (sscanf(path, "%u:%u", &major, &minor) == 2) { /* Extract the major/minor numbers */ dev = MKDEV(major, minor); if (MAJOR(dev) != major || MINOR(dev) != minor) - return -EOVERFLOW; + return 0; } else { /* convert the path to a device */ struct block_device *bdev = lookup_bdev(path); if (IS_ERR(bdev)) - return PTR_ERR(bdev); + return 0; 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. + */ +int dm_get_device(struct dm_target *ti, const char *path, fmode_t mode, + struct dm_dev **result) +{ + int r; + dev_t dev; + struct dm_dev_internal *dd; + struct dm_table *t = ti->table; + + BUG_ON(!t); + + dev = dm_get_dev_t(path); + if (!dev) + return -ENODEV; + dd = find_device(&t->devices, dev); if (!dd) { dd = kmalloc(sizeof(*dd), GFP_KERNEL); --- a/include/linux/device-mapper.h +++ b/include/linux/device-mapper.h @@ -116,6 +116,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.