[PATCH] dm thin: reject metadata and data device aliases

[PATCH] dm thin: reject metadata and data device aliases

[Eduard Zateev]

From: angel <angel@localhost.localdomain>

The thin-pool target rejects identical argument strings, but two names for
the same block device bypass the check. In read-only metadata mode, opening
metadata first and data with write mode invokes dm_get_device()
upgrade_mode().
That swaps and frees the old dm_dev while pool_ctr retains metadata_dev,
causing a slab-use-after-free when __pool_find() reads metadata_dev->bdev.

Open the data device before metadata so an alias cannot trigger a mode
upgrade between the two references. Compare the opened block devices
and reject aliases before using either device. Adjust the error paths to
release the devices in the matching order.

Reported-by: syzbot+83827f9a59abe148bff4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=83827f9a59abe148bff4
Fixes: 70de2cbda8a5 ("dm thin: add sanity checks to thin-pool and external snapshot creation")
Signed-off-by: Eduard Zateev <hackerowskiy@gmail.com>
---
 drivers/md/dm-thin.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index 358ed77ff..0899cbaf9 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -3326,17 +3326,24 @@ static int pool_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 
 	metadata_mode = BLK_OPEN_READ |
 		((pf.mode == PM_READ_ONLY) ? 0 : BLK_OPEN_WRITE);
+	r = dm_get_device(ti, argv[1], BLK_OPEN_READ | BLK_OPEN_WRITE, &data_dev);
+	if (r) {
+		ti->error = "Error getting data device";
+		goto out_unlock;
+	}
+
 	r = dm_get_device(ti, argv[0], metadata_mode, &metadata_dev);
 	if (r) {
 		ti->error = "Error opening metadata block device";
-		goto out_unlock;
+		goto out_data;
 	}
 	warn_if_metadata_device_too_big(metadata_dev->bdev);
 
-	r = dm_get_device(ti, argv[1], BLK_OPEN_READ | BLK_OPEN_WRITE, &data_dev);
-	if (r) {
-		ti->error = "Error getting data device";
-		goto out_metadata;
+	/* make sure metadata and data are different devices */
+	if (metadata_dev->bdev == data_dev->bdev) {
+		ti->error = "Error setting metadata or data device";
+		r = -EINVAL;
+		goto out;
 	}
 
 	if (kstrtoul(argv[2], 10, &block_size) || !block_size ||
@@ -3426,9 +3433,9 @@ static int pool_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 out_free_pt:
 	kfree(pt);
 out:
-	dm_put_device(ti, data_dev);
-out_metadata:
 	dm_put_device(ti, metadata_dev);
+out_data:
+	dm_put_device(ti, data_dev);
 out_unlock:
 	mutex_unlock(&dm_thin_pool_table.mutex);
 

base-commit: 21a7f44fd0765fcc5194130b28767786eba1d1e7
-- 
2.56.0

Re: [PATCH] dm thin: reject metadata and data device aliases

[Mikulas Patocka]

Hi

If dm_get_device spuriously closes a device that was already open, it is a 
bug in dm_get_device and it should be fixed there.

Note that we got 3 patches for this syzbot warning from 3 different 
people. Please, cooperate next time and send just one patch.

Mikulas


On Mon, 26 Oct 2026, Eduard Zateev wrote:

> From: angel <angel@localhost.localdomain>
> 
> The thin-pool target rejects identical argument strings, but two names for
> the same block device bypass the check. In read-only metadata mode, opening
> metadata first and data with write mode invokes dm_get_device()
> upgrade_mode().
> That swaps and frees the old dm_dev while pool_ctr retains metadata_dev,
> causing a slab-use-after-free when __pool_find() reads metadata_dev->bdev.
> 
> Open the data device before metadata so an alias cannot trigger a mode
> upgrade between the two references. Compare the opened block devices
> and reject aliases before using either device. Adjust the error paths to
> release the devices in the matching order.
> 
> Reported-by: syzbot+83827f9a59abe148bff4@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=83827f9a59abe148bff4
> Fixes: 70de2cbda8a5 ("dm thin: add sanity checks to thin-pool and external snapshot creation")
> Signed-off-by: Eduard Zateev <hackerowskiy@gmail.com>
> ---
>  drivers/md/dm-thin.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
> index 358ed77ff..0899cbaf9 100644
> --- a/drivers/md/dm-thin.c
> +++ b/drivers/md/dm-thin.c
> @@ -3326,17 +3326,24 @@ static int pool_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>  
>  	metadata_mode = BLK_OPEN_READ |
>  		((pf.mode == PM_READ_ONLY) ? 0 : BLK_OPEN_WRITE);
> +	r = dm_get_device(ti, argv[1], BLK_OPEN_READ | BLK_OPEN_WRITE, &data_dev);
> +	if (r) {
> +		ti->error = "Error getting data device";
> +		goto out_unlock;
> +	}
> +
>  	r = dm_get_device(ti, argv[0], metadata_mode, &metadata_dev);
>  	if (r) {
>  		ti->error = "Error opening metadata block device";
> -		goto out_unlock;
> +		goto out_data;
>  	}
>  	warn_if_metadata_device_too_big(metadata_dev->bdev);
>  
> -	r = dm_get_device(ti, argv[1], BLK_OPEN_READ | BLK_OPEN_WRITE, &data_dev);
> -	if (r) {
> -		ti->error = "Error getting data device";
> -		goto out_metadata;
> +	/* make sure metadata and data are different devices */
> +	if (metadata_dev->bdev == data_dev->bdev) {
> +		ti->error = "Error setting metadata or data device";
> +		r = -EINVAL;
> +		goto out;
>  	}
>  
>  	if (kstrtoul(argv[2], 10, &block_size) || !block_size ||
> @@ -3426,9 +3433,9 @@ static int pool_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>  out_free_pt:
>  	kfree(pt);
>  out:
> -	dm_put_device(ti, data_dev);
> -out_metadata:
>  	dm_put_device(ti, metadata_dev);
> +out_data:
> +	dm_put_device(ti, data_dev);
>  out_unlock:
>  	mutex_unlock(&dm_thin_pool_table.mutex);
>  
> 
> base-commit: 21a7f44fd0765fcc5194130b28767786eba1d1e7
> -- 
> 2.56.0
> 

[PATCH] dm: preserve outstanding device references across mode upgrades

[Qing Zhang]

dm_get_device() keeps one dm_dev_internal entry per dev_t. When a later
caller requests the same device with additional modes, upgrade_mode()
opens a new table device (table devices are keyed by (dev_t, mode)),
points dd->dm_dev at it and releases the old one.

The pointer returned by the first dm_get_device() call is still held by
the target, e.g. pool_ctr()'s metadata_dev or thin_ctr()'s
tc->origin_dev, so releasing the old table device frees memory that is
still in use and reading e.g. metadata_dev->bdev is a use-after-free.
dm_put_device() can then no longer match the stale pointer either, so
its reference leaks. The old device cannot be widened in place: its
bdev may be accessed concurrently and the dm_dev may be shared with
another table, so it has to stay valid until its holder releases it.

Instead of releasing the replaced dm_dev in upgrade_mode(), keep it on
a list on the dm_dev_internal entry. dm_put_device() matches both the
current and retained pointers, and all associated table devices are
released once the entry's reference count reaches zero, including when
free_devices() tears down a table whose devices were not all put. If
acquiring the upgraded device fails, leave the existing entry unchanged.

This affects any target that opens the same device twice with different
modes, such as the thin-pool metadata/data and thin pool/external-origin
arguments reported by syzbot.

Fixes: 86f1152b117a ("dm: allow active and inactive tables to share dm_devs")
Reported-by: syzbot+83827f9a59abe148bff4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=83827f9a59abe148bff4
Signed-off-by: Qing Zhang <diasyzhang@tencent.com>
---
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

Fixing this in dm_get_device() itself, as you suggest, also covers the
same-device mode-upgrade instances in the other targets, so a single
change closes them all.  Verified with the syzbot reproducer: the
unpatched modules hit the pool_ctr use-after-free within a few
iterations, the patched modules run the same reproducer clean, with no
KASAN reports and no leaked table-device references.

 drivers/md/dm-table.c | 53 +++++++++++++++++++++++++++++++++++++------
 drivers/md/dm.h       |  1 +
 2 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index a483f49dd4ce..f64db28aa15c 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -163,6 +163,24 @@ int dm_table_create(struct dm_table **result, blk_mode_t mode,
 	return 0;
 }
 
+struct dm_dev_old {
+	struct list_head list;
+	struct dm_dev *dm_dev;
+};
+
+static void put_table_devices(struct dm_dev_internal *dd,
+			      struct mapped_device *md)
+{
+	struct dm_dev_old *old, *tmp;
+
+	list_for_each_entry_safe(old, tmp, &dd->old_devices, list) {
+		dm_put_table_device(md, old->dm_dev);
+		list_del(&old->list);
+		kfree(old);
+	}
+	dm_put_table_device(md, dd->dm_dev);
+}
+
 static void free_devices(struct list_head *devices, struct mapped_device *md)
 {
 	struct list_head *tmp, *next;
@@ -172,7 +190,7 @@ static void free_devices(struct list_head *devices, struct mapped_device *md)
 		    list_entry(tmp, struct dm_dev_internal, list);
 		DMWARN("%s: dm_table_destroy: dm_put_device call missing for %s",
 		       dm_device_name(md), dd->dm_dev->name);
-		dm_put_table_device(md, dd->dm_dev);
+		put_table_devices(dd, md);
 		kfree(dd);
 	}
 }
@@ -313,17 +331,23 @@ static int upgrade_mode(struct dm_dev_internal *dd, blk_mode_t new_mode,
 			struct mapped_device *md)
 {
 	int r;
-	struct dm_dev *old_dev, *new_dev;
+	struct dm_dev *new_dev;
+	struct dm_dev_old *old;
 
-	old_dev = dd->dm_dev;
+	old = kmalloc_obj(*old);
+	if (!old)
+		return -ENOMEM;
 
 	r = dm_get_table_device(md, dd->dm_dev->bdev->bd_dev,
 				dd->dm_dev->mode | new_mode, &new_dev);
-	if (r)
+	if (r) {
+		kfree(old);
 		return r;
+	}
 
+	old->dm_dev = dd->dm_dev;
+	list_add_tail(&old->list, &dd->old_devices);
 	dd->dm_dev = new_dev;
-	dm_put_table_device(md, old_dev);
 
 	return 0;
 }
@@ -392,6 +416,7 @@ int dm_get_device(struct dm_target *ti, const char *path, blk_mode_t mode,
 		}
 
 		refcount_set(&dd->count, 1);
+		INIT_LIST_HEAD(&dd->old_devices);
 		list_add(&dd->list, &t->devices);
 		goto out;
 
@@ -451,6 +476,20 @@ static int dm_set_device_limits(struct dm_target *ti, struct dm_dev *dev,
 /*
  * Decrement a device's use count and remove it if necessary.
  */
+static bool device_matches(struct dm_dev_internal *dd, struct dm_dev *d)
+{
+	struct dm_dev_old *old;
+
+	if (dd->dm_dev == d)
+		return true;
+
+	list_for_each_entry(old, &dd->old_devices, list)
+		if (old->dm_dev == d)
+			return true;
+
+	return false;
+}
+
 void dm_put_device(struct dm_target *ti, struct dm_dev *d)
 {
 	int found = 0;
@@ -458,7 +497,7 @@ void dm_put_device(struct dm_target *ti, struct dm_dev *d)
 	struct dm_dev_internal *dd;
 
 	list_for_each_entry(dd, devices, list) {
-		if (dd->dm_dev == d) {
+		if (device_matches(dd, d)) {
 			found = 1;
 			break;
 		}
@@ -469,7 +508,7 @@ void dm_put_device(struct dm_target *ti, struct dm_dev *d)
 		return;
 	}
 	if (refcount_dec_and_test(&dd->count)) {
-		dm_put_table_device(ti->table->md, d);
+		put_table_devices(dd, ti->table->md);
 		list_del(&dd->list);
 		kfree(dd);
 	}
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index 7a795979ec72..037d6cdf2f1c 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -42,6 +42,7 @@ struct dm_dev_internal {
 	struct list_head list;
 	refcount_t count;
 	struct dm_dev *dm_dev;
+	struct list_head old_devices;
 };
 
 struct dm_table;
-- 
2.43.7

Re: [syzbot] [dm?] KASAN: slab-use-after-free Read in pool_ctr

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+83827f9a59abe148bff4@syzkaller.appspotmail.com
Tested-by: syzbot+83827f9a59abe148bff4@syzkaller.appspotmail.com

Tested on:

commit:         d135ab45 Merge tag 'backlight-next-7.4' of git://git.k..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1595102d580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=c899018ccfbe890a
dashboard link: https://syzkaller.appspot.com/bug?extid=83827f9a59abe148bff4
compiler:       Debian clang version 22.1.8 (1~deb13u4), Debian LLD 22.1.8
patch:          https://syzkaller.appspot.com/x/patch.diff?x=1311fb0d580000

Note: testing is done by a robot and is best-effort only.