[PATCH] dm thin: reject metadata and data devices that resolve to the same device

[PATCH] dm thin: reject metadata and data devices that resolve to the same device

[Mohammad Pishdar]

pool_ctr() checks that the metadata and data devices differ by comparing
the two argument strings. Two different strings can name the same block
device, for example "/dev/ram1" and "1:1". The check is then bypassed.

With the "read_only" feature the metadata device is opened with
BLK_OPEN_READ only, while the data device is requested with
BLK_OPEN_READ | BLK_OPEN_WRITE. dm_get_device() finds the already
present entry by dev_t, sees that the mode differs and calls
upgrade_mode(). That opens a new table device, points dd->dm_dev at it
and releases the old one. pool_ctr() however still holds the old
struct dm_dev in metadata_dev. When pool_ctr() later fails,
dm_put_device(ti, metadata_dev) does not find the stale pointer in the
table's device list and reads d->name from the freed table_device to
print an error:

  BUG: KASAN: slab-use-after-free in string_nocheck lib/vsprintf.c:650
  Read of size 1
  dm_put_device+... drivers/md/dm-table.c:467
  pool_ctr+... drivers/md/dm-thin.c:3431
  Freed by:  close_table_device <- dm_put_table_device <- upgrade_mode
             <- dm_get_device <- pool_ctr
  Allocated by: open_table_device <- dm_get_table_device

Resolve both paths with dm_devt_from_path() and reject the table if they
refer to the same device. If either path cannot be resolved, keep the
existing behaviour and let dm_get_device() report the error.

This only fixes pool_ctr(). Any target that calls dm_get_device() twice
for the same device with different modes while keeping the first result
can hit the same stale pointer through upgrade_mode(); that would need a
change in the dm core and is not attempted here.

Testing: on v7.4-rc1 built with KASAN, in QEMU (software emulation), the
syzbot C reproducer triggers the slab-use-after-free before this patch.
After the patch the table load is rejected with -EINVAL ("Error setting
metadata or data device") and there is no KASAN report. A regular
thin-pool with two different devices still loads, and both a
"/dev/ramN" + "major:minor" spelling of the same device and two identical
strings are rejected with -EINVAL. The kernel config was generated from
x86_64_defconfig plus KASAN and dm-thin, not taken from syzbot.

Fixes: 86f1152b117a ("dm: allow active and inactive tables to share dm_devs")
Reported-by: syzbot+9519233e6d144970bbc2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9519233e6d144970bbc2

Tooling: the root cause analysis, this patch and the test scripts were
drafted with an AI assistant (Claude), based on the syzbot report and the
reproducer. The tests described above were run by the submitter.

Signed-off-by: Mohammad Pishdar <mohamadpishdar@gmail.com>
---
 drivers/md/dm-thin.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index 358ed77ff..6285bbc8c 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -3292,6 +3292,7 @@ static int pool_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	dm_block_t low_water_blocks;
 	struct dm_dev *metadata_dev;
 	blk_mode_t metadata_mode;
+	dev_t metadata_devt, data_devt;
 
 	/*
 	 * FIXME Remove validation from scope of lock.
@@ -3307,8 +3308,17 @@ static int pool_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	as.argc = argc;
 	as.argv = argv;
 
-	/* make sure metadata and data are different devices */
-	if (!strcmp(argv[0], argv[1])) {
+	/*
+	 * Make sure metadata and data are different devices. Comparing the
+	 * strings is not enough: "/dev/foo" and "major:minor" can name the
+	 * same device, which would make dm_get_device() upgrade the open
+	 * mode of the already acquired metadata device and free the
+	 * struct dm_dev we are still holding.
+	 */
+	if (!strcmp(argv[0], argv[1]) ||
+	    (!dm_devt_from_path(argv[0], &metadata_devt) &&
+	     !dm_devt_from_path(argv[1], &data_devt) &&
+	     metadata_devt == data_devt)) {
 		ti->error = "Error setting metadata or data device";
 		r = -EINVAL;
 		goto out_unlock;
-- 
2.34.1