[PATCH 0/2] dm: mpath dm_get_device fix

[PATCH 0/2] dm: mpath dm_get_device fix

[Benjamin Marzinski]

dm-mpath shouldn't be calling dm_get_device() in multipath_message().
If you run:

# dmsetup message <mpath_dev> 0 "fail_path <device_not_in_multipath>"

the dm_get_device() call in multipath_message() will add that incorrect
device to the devices list, fail to find a matching pgpath, and then
remove it.  multipath shouldn't be messing with the devices table
outside of its constructor and destructor.

To fix it, this patchset factors out a helper function from
dm_get_device() to turn the device path string into a dev_t.
multipath_message() calls that helper function and uses the dev_t to
find the correct pgpath instead.

Benjamin Marzinski (2):
  dm: factor out helper function from dm_get_device
  dm mpath: don't call dm_get_device in multipath_message

 drivers/md/dm-mpath.c         | 11 ++++-------
 drivers/md/dm-table.c         | 33 ++++++++++++++++++++++++---------
 include/linux/device-mapper.h |  5 +++++
 3 files changed, 33 insertions(+), 16 deletions(-)

-- 
2.45.0

[PATCH 1/2] dm: factor out helper function from dm_get_device

[Benjamin Marzinski]

Factor out a helper function, dm_devt_from_path(), from dm_get_device()
for use in dm targets.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 drivers/md/dm-table.c         | 33 ++++++++++++++++++++++++---------
 include/linux/device-mapper.h |  5 +++++
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index d124e37dfb00..41b58e0c6876 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -329,23 +329,15 @@ static int upgrade_mode(struct dm_dev_internal *dd, blk_mode_t new_mode,
 }
 
 /*
- * Add a device to the list, or just increment the usage count if
- * it's already present.
- *
  * Note: the __ref annotation is because this function can call the __init
  * marked early_lookup_bdev when called during early boot code from dm-init.c.
  */
-int __ref dm_get_device(struct dm_target *ti, const char *path, blk_mode_t mode,
-		  struct dm_dev **result)
+int __ref dm_devt_from_path(const char *path, dev_t *dev_p)
 {
 	int r;
 	dev_t dev;
 	unsigned int major, minor;
 	char dummy;
-	struct dm_dev_internal *dd;
-	struct dm_table *t = ti->table;
-
-	BUG_ON(!t);
 
 	if (sscanf(path, "%u:%u%c", &major, &minor, &dummy) == 2) {
 		/* Extract the major/minor numbers */
@@ -361,6 +353,29 @@ int __ref dm_get_device(struct dm_target *ti, const char *path, blk_mode_t mode,
 		if (r)
 			return r;
 	}
+	*dev_p = dev;
+	return 0;
+}
+EXPORT_SYMBOL(dm_devt_from_path);
+
+/*
+ * 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, blk_mode_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);
+
+	r = dm_devt_from_path(path, &dev);
+	if (r)
+		return r;
+
 	if (dev == disk_devt(t->md->disk))
 		return -EINVAL;
 
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 82b2195efaca..b4c7c4fa2de6 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -179,6 +179,11 @@ int dm_get_device(struct dm_target *ti, const char *path, blk_mode_t mode,
 		  struct dm_dev **result);
 void dm_put_device(struct dm_target *ti, struct dm_dev *d);
 
+/*
+ * Helper function for getting devices
+ */
+int dm_devt_from_path(const char *path, dev_t *dev_p);
+
 /*
  * Information about a target type
  */
-- 
2.45.0

[PATCH 2/2] dm mpath: don't call dm_get_device in multipath_message

[Benjamin Marzinski]

When mutipath_message is called with an action and a device, it needs to
find the pgpath that matches that device. dm_get_device() is not the
right function for this. dm_get_device() will look for a table_device
matching the requested path in use by either the live or inactive table.
If it doesn't find the device, dm_get_device() will open it and add it
to the table. Means that multipath_message will accept any block device,
add it to the table if not present, and then look through the pgpaths
to see if it finds a match. Afterwards it will remove the device if it
was not previously in the table devices list.

This is the only function that can modify the device list of a table
besides the constructors and destructors, and it can only do this when
it was passed an invalid message.  Instead, multipath_message() should
call dm_devt_from_path() to get the device dev_t, and match that against
its pgpaths.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 drivers/md/dm-mpath.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 05d1328d1811..a1d08a702709 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1420,8 +1420,7 @@ static int reinstate_path(struct pgpath *pgpath)
 /*
  * Fail or reinstate all paths that match the provided struct dm_dev.
  */
-static int action_dev(struct multipath *m, struct dm_dev *dev,
-		      action_fn action)
+static int action_dev(struct multipath *m, dev_t dev, action_fn action)
 {
 	int r = -EINVAL;
 	struct pgpath *pgpath;
@@ -1429,7 +1428,7 @@ static int action_dev(struct multipath *m, struct dm_dev *dev,
 
 	list_for_each_entry(pg, &m->priority_groups, list) {
 		list_for_each_entry(pgpath, &pg->pgpaths, list) {
-			if (pgpath->path.dev == dev)
+			if (pgpath->path.dev->bdev->bd_dev == dev)
 				r = action(pgpath);
 		}
 	}
@@ -1960,7 +1959,7 @@ static int multipath_message(struct dm_target *ti, unsigned int argc, char **arg
 			     char *result, unsigned int maxlen)
 {
 	int r = -EINVAL;
-	struct dm_dev *dev;
+	dev_t dev;
 	struct multipath *m = ti->private;
 	action_fn action;
 	unsigned long flags;
@@ -2009,7 +2008,7 @@ static int multipath_message(struct dm_target *ti, unsigned int argc, char **arg
 		goto out;
 	}
 
-	r = dm_get_device(ti, argv[1], dm_table_get_mode(ti->table), &dev);
+	r = dm_devt_from_path(argv[1], &dev);
 	if (r) {
 		DMWARN("message: error getting device %s",
 		       argv[1]);
@@ -2018,8 +2017,6 @@ static int multipath_message(struct dm_target *ti, unsigned int argc, char **arg
 
 	r = action_dev(m, dev, action);
 
-	dm_put_device(ti, dev);
-
 out:
 	mutex_unlock(&m->work_mutex);
 	return r;
-- 
2.45.0

Re: [PATCH 0/2] dm: mpath dm_get_device fix

[Benjamin Marzinski]

On Thu, May 16, 2024 at 08:41:27PM -0400, Benjamin Marzinski wrote:

ping.

> dm-mpath shouldn't be calling dm_get_device() in multipath_message().
> If you run:
> 
> # dmsetup message <mpath_dev> 0 "fail_path <device_not_in_multipath>"
> 
> the dm_get_device() call in multipath_message() will add that incorrect
> device to the devices list, fail to find a matching pgpath, and then
> remove it.  multipath shouldn't be messing with the devices table
> outside of its constructor and destructor.
> 
> To fix it, this patchset factors out a helper function from
> dm_get_device() to turn the device path string into a dev_t.
> multipath_message() calls that helper function and uses the dev_t to
> find the correct pgpath instead.
> 
> Benjamin Marzinski (2):
>   dm: factor out helper function from dm_get_device
>   dm mpath: don't call dm_get_device in multipath_message
> 
>  drivers/md/dm-mpath.c         | 11 ++++-------
>  drivers/md/dm-table.c         | 33 ++++++++++++++++++++++++---------
>  include/linux/device-mapper.h |  5 +++++
>  3 files changed, 33 insertions(+), 16 deletions(-)
> 
> -- 
> 2.45.0
> 

Re: [PATCH 0/2] dm: mpath dm_get_device fix

[Mikulas Patocka]

On Mon, 10 Jun 2024, Benjamin Marzinski wrote:

> On Thu, May 16, 2024 at 08:41:27PM -0400, Benjamin Marzinski wrote:
> 
> ping.

I staged these patches for 6.11.

Mikulas

> > dm-mpath shouldn't be calling dm_get_device() in multipath_message().
> > If you run:
> > 
> > # dmsetup message <mpath_dev> 0 "fail_path <device_not_in_multipath>"
> > 
> > the dm_get_device() call in multipath_message() will add that incorrect
> > device to the devices list, fail to find a matching pgpath, and then
> > remove it.  multipath shouldn't be messing with the devices table
> > outside of its constructor and destructor.
> > 
> > To fix it, this patchset factors out a helper function from
> > dm_get_device() to turn the device path string into a dev_t.
> > multipath_message() calls that helper function and uses the dev_t to
> > find the correct pgpath instead.
> > 
> > Benjamin Marzinski (2):
> >   dm: factor out helper function from dm_get_device
> >   dm mpath: don't call dm_get_device in multipath_message
> > 
> >  drivers/md/dm-mpath.c         | 11 ++++-------
> >  drivers/md/dm-table.c         | 33 ++++++++++++++++++++++++---------
> >  include/linux/device-mapper.h |  5 +++++
> >  3 files changed, 33 insertions(+), 16 deletions(-)
> > 
> > -- 
> > 2.45.0
> > 
>