[PATCH] dm snap: fix use-after-free when origin and COW devices match

[PATCH] dm snap: fix use-after-free when origin and COW devices match

[Igor Putko]

When snapshot_ctr() parses target arguments where both origin and COW
paths resolve to the same underlying block device, a slab-use-after-free
occurs.

The origin device is acquired first with origin_mode (e.g. BLK_OPEN_READ
or BLK_OPEN_WRITE). When opening the COW device with dm_table_get_mode()
(typically BLK_OPEN_READ | BLK_OPEN_WRITE), dm_get_device() sees that
the device is already in the table list and calls upgrade_mode() because
the requested mode is broader than the existing one.

upgrade_mode() opens a new table_device, points dd->dm_dev to it, and
calls dm_put_table_device() on the previous one. Since the refcount drops
to zero, the old table_device is freed. However, s->origin still holds a
pointer to that freed table_device. The subsequent check:

    if (s->cow->bdev && s->cow->bdev == s->origin->bdev)

dereferences s->origin->bdev, tripping a KASAN slab-use-after-free read.
Furthermore, under the bad_store error path, dm_put_device(ti, s->origin)
fails to find s->origin in ti->table->devices because dd->dm_dev was
replaced, leaking the device reference count.

Fix this by resolving cow_path to a dev_t via dm_devt_from_path() and
verifying that it does not match s->origin->bdev->bd_dev before calling
dm_get_device() for the COW device.

Fixes: 26110d5afe81 ("dm-snap: simplify the origin_dev == cow_dev check in snapshot_ctr")
Reported-by: syzbot+8e6fc64255334ccf3ab3@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8e6fc64255334ccf3ab3
Signed-off-by: Igor Putko <igorpetindev@gmail.com>
---
 drivers/md/dm-snap.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 7c3725e4ac54..4a7c04c780d1 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -1232,6 +1232,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 cow_dev;
 	unsigned int args_used, num_flush_bios = 1;
 	blk_mode_t origin_mode = BLK_OPEN_READ;
 
@@ -1274,15 +1275,21 @@ static int snapshot_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	argv++;
 	argc--;
 
-	r = dm_get_device(ti, cow_path, dm_table_get_mode(ti->table), &s->cow);
+	r = dm_devt_from_path(cow_path, &cow_dev);
 	if (r) {
 		ti->error = "Cannot get COW device";
 		goto bad_cow;
 	}
-	if (s->cow->bdev && s->cow->bdev == s->origin->bdev) {
+	if (cow_dev == s->origin->bdev->bd_dev) {
 		ti->error = "COW device cannot be the same as origin device";
 		r = -EINVAL;
-		goto bad_store;
+		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";
+		goto bad_cow;
 	}
 
 	r = dm_exception_store_create(ti, argc, argv, s, &args_used, &s->store);
-- 
2.47.3

Re: [PATCH] dm snap: fix use-after-free when origin and COW devices match

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] dm snap: TOCTOU race in snapshot_ctr() allows upgrade_mode() UAF on s->origin
--

commit b2c589f4852e6503996057072cf65216f108da3f
Author: Igor Putko <igorpetindev@gmail.com>

dm snap: fix use-after-free when origin and COW devices match

This commit resolves cow_path via dm_devt_from_path() in snapshot_ctr() and
compares the resulting dev_t against s->origin->bdev->bd_dev before calling
dm_get_device(), avoiding a use-after-free when upgrade_mode() replaces the
origin table_device.

> diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
> index 797c432594d86..ecda207e800d7 100644
> --- a/drivers/md/dm-snap.c
> +++ b/drivers/md/dm-snap.c
> @@ -1234,6 +1234,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 cow_dev;
>  	unsigned int args_used, num_flush_bios = 1;
>  	blk_mode_t origin_mode = BLK_OPEN_READ;
>  
> @@ -1276,15 +1277,21 @@ static int snapshot_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>  	argv++;
>  	argc--;
>  
> -	r = dm_get_device(ti, cow_path, dm_table_get_mode(ti->table), &s->cow);
> +	r = dm_devt_from_path(cow_path, &cow_dev);
>  	if (r) {
>  		ti->error = "Cannot get COW device";
>  		goto bad_cow;
>  	}
> -	if (s->cow->bdev && s->cow->bdev == s->origin->bdev) {
> +	if (cow_dev == s->origin->bdev->bd_dev) {
>  		ti->error = "COW device cannot be the same as origin device";
>  		r = -EINVAL;
> -		goto bad_store;
> +		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";
> +		goto bad_cow;
>  	}

[Severity: Critical]
Could a race between the two path lookups still trigger the use-after-free on
s->origin and leak the device reference?

When cow_path is a filesystem path rather than a major:minor string,
snapshot_ctr() first resolves it via dm_devt_from_path() -> lookup_bdev() ->
kern_path(), reads inode->i_rdev, and drops the path reference via
path_put().

Immediately afterwards, dm_get_device() resolves cow_path a second time:

drivers/md/dm-table.c:dm_get_device() {
	r = dm_devt_from_path(path, &dev);
	if (r)
		return r;
	...
	dd = find_device(&t->devices, dev);
	if (!dd) {
		...
	} else if (dd->dm_dev->mode != (mode | dd->dm_dev->mode)) {
		r = upgrade_mode(dd, mode, t->md);
		if (r)
			return r;
	}
}

If cow_path (such as a symlink or block device node) is modified between the
first lookup in snapshot_ctr() and the second lookup in dm_get_device() to
point to the same block device as s->origin, find_device() matches the
existing dm_dev_internal entry for s->origin and calls upgrade_mode():

drivers/md/dm-table.c:upgrade_mode() {
	old_dev = dd->dm_dev;

	r = dm_get_table_device(md, dd->dm_dev->bdev->bd_dev,
				dd->dm_dev->mode | new_mode, &new_dev);
	if (r)
		return r;

	dd->dm_dev = new_dev;
	dm_put_table_device(md, old_dev);
}

Because the post-open check comparing s->cow->bdev and s->origin->bdev was
removed, wouldn't snapshot_ctr() continue execution with s->origin pointing
to the freed old_dev, dereferencing freed slab memory in register_snapshot()
(via snap->origin->bdev) or dm_put_device(), while also leaking the reference
on dd in ti->table->devices when dm_put_device(ti, s->origin) fails to find
the replaced dm_dev?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024161459.5539-1-igorpetindev@gmail.com?part=1