All of lore.kernel.org
 help / color / mirror / Atom feed
From: Demi Marie Obenour <demi@invisiblethingslab.com>
To: Alasdair Kergon <agk@redhat.com>,
	Mike Snitzer <snitzer@kernel.org>,
	dm-devel@redhat.com
Cc: Demi Marie Obenour <demi@invisiblethingslab.com>,
	linux-kernel@vger.kernel.org
Subject: [dm-devel] [PATCH v2 4/4] dm ioctl: inform caller about already-existing device
Date: Sat, 24 Jun 2023 19:09:47 -0400	[thread overview]
Message-ID: <20230624230950.2272-5-demi@invisiblethingslab.com> (raw)
In-Reply-To: <20230624230950.2272-1-demi@invisiblethingslab.com>

Not only is this helpful for debugging, it also saves the caller an
ioctl in the case where a device should be used if it exists or created
otherwise.  To ensure existing userspace is not broken, this feature is
only enabled in strict mode.

Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
---
 drivers/md/dm-ioctl.c         | 31 +++++++++++++++++++++++++------
 include/uapi/linux/dm-ioctl.h |  4 ++--
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 7eab9a8c77ff3286346a1c44581d3b6370a731eb..4eb3eda2fe169f64259458dd678833f444d76ce0 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -259,11 +259,14 @@ static void free_cell(struct hash_cell *hc)
 	}
 }
 
+static void __dev_status(struct mapped_device *md, struct dm_ioctl *param);
+
 /*
  * The kdev_t and uuid of a device can never change once it is
  * initially inserted.
  */
-static int dm_hash_insert(const char *name, const char *uuid, struct mapped_device *md)
+static int dm_hash_insert(const char *name, const char *uuid, struct mapped_device *md,
+			  struct dm_ioctl *param)
 {
 	struct hash_cell *cell, *hc;
 
@@ -280,6 +283,8 @@ static int dm_hash_insert(const char *name, const char *uuid, struct mapped_devi
 	down_write(&_hash_lock);
 	hc = __get_name_cell(name);
 	if (hc) {
+		if (param)
+			__dev_status(hc->md, param);
 		dm_put(hc->md);
 		goto bad;
 	}
@@ -290,6 +295,8 @@ static int dm_hash_insert(const char *name, const char *uuid, struct mapped_devi
 		hc = __get_uuid_cell(uuid);
 		if (hc) {
 			__unlink_name(cell);
+			if (param)
+				__dev_status(hc->md, param);
 			dm_put(hc->md);
 			goto bad;
 		}
@@ -901,10 +908,12 @@ static int dev_create(struct file *filp, struct dm_ioctl *param, size_t param_si
 		m = MINOR(huge_decode_dev(param->dev));
 
 	r = dm_create(m, &md, param->flags & DM_DISABLE_UEVENTS_FLAG);
-	if (r)
+	if (r) {
+		DMERR("Could not create device-mapper device");
 		return r;
+	}
 
-	r = dm_hash_insert(param->name, *param->uuid ? param->uuid : NULL, md);
+	r = dm_hash_insert(param->name, *param->uuid ? param->uuid : NULL, md, param);
 	if (r) {
 		dm_put(md);
 		dm_destroy(md);
@@ -2234,6 +2243,7 @@ static int ctl_ioctl(struct file *file, uint command, struct dm_ioctl __user *us
 	int ioctl_flags;
 	int param_flags;
 	unsigned int cmd;
+	bool do_copy;
 	struct dm_ioctl *param;
 	ioctl_fn fn = NULL;
 	size_t input_param_size;
@@ -2307,9 +2317,18 @@ static int ctl_ioctl(struct file *file, uint command, struct dm_ioctl __user *us
 	param->flags |= old_flags;
 
 	/*
-	 * Copy the results back to userland.
+	 * Copy the results back to userland if either:
+	 *
+	 * - The ioctl succeeded.
+	 * - The ioctl is DM_DEV_CREATE, the return value is -EBUSY,
+	 *   and strict parameter checking is enabled.
 	 */
-	if (!r && copy_to_user(user, param, param->data_size))
+	do_copy = r == 0;
+	if (r == -EBUSY && !sloppy_checks(param) && cmd == DM_DEV_CREATE_CMD) {
+		param->flags |= DM_EXISTS_FLAG;
+		do_copy = true;
+	}
+	if (do_copy && copy_to_user(user, param, param->data_size))
 		r = -EFAULT;
 
 out:
@@ -2488,7 +2507,7 @@ int __init dm_early_create(struct dm_ioctl *dmi,
 		return r;
 
 	/* hash insert */
-	r = dm_hash_insert(dmi->name, *dmi->uuid ? dmi->uuid : NULL, md);
+	r = dm_hash_insert(dmi->name, *dmi->uuid ? dmi->uuid : NULL, md, NULL);
 	if (r)
 		goto err_destroy_dm;
 
diff --git a/include/uapi/linux/dm-ioctl.h b/include/uapi/linux/dm-ioctl.h
index b338a4f9a299acda9430995d63fbb490e70c8cd8..195d60be4aba4f20b7220b900c87d6d3a86014a2 100644
--- a/include/uapi/linux/dm-ioctl.h
+++ b/include/uapi/linux/dm-ioctl.h
@@ -344,8 +344,8 @@ enum {
 /* Status bits */
 #define DM_READONLY_FLAG	(1 << 0) /* In/Out */
 #define DM_SUSPEND_FLAG		(1 << 1) /* In/Out */
-#define DM_EXISTS_FLAG		(1 << 2) /* Not used by kernel, reserved for
-					  * libdevmapper in userland
+#define DM_EXISTS_FLAG		(1 << 2) /* Set when trying to create a
+					  * device that already exists
 					  */
 #define DM_PERSISTENT_DEV_FLAG	(1 << 3) /* In */
 
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


WARNING: multiple messages have this Message-ID (diff)
From: Demi Marie Obenour <demi@invisiblethingslab.com>
To: Alasdair Kergon <agk@redhat.com>,
	Mike Snitzer <snitzer@kernel.org>,
	dm-devel@redhat.com
Cc: Demi Marie Obenour <demi@invisiblethingslab.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2 4/4] dm ioctl: inform caller about already-existing device
Date: Sat, 24 Jun 2023 19:09:47 -0400	[thread overview]
Message-ID: <20230624230950.2272-5-demi@invisiblethingslab.com> (raw)
In-Reply-To: <20230624230950.2272-1-demi@invisiblethingslab.com>

Not only is this helpful for debugging, it also saves the caller an
ioctl in the case where a device should be used if it exists or created
otherwise.  To ensure existing userspace is not broken, this feature is
only enabled in strict mode.

Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
---
 drivers/md/dm-ioctl.c         | 31 +++++++++++++++++++++++++------
 include/uapi/linux/dm-ioctl.h |  4 ++--
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 7eab9a8c77ff3286346a1c44581d3b6370a731eb..4eb3eda2fe169f64259458dd678833f444d76ce0 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -259,11 +259,14 @@ static void free_cell(struct hash_cell *hc)
 	}
 }
 
+static void __dev_status(struct mapped_device *md, struct dm_ioctl *param);
+
 /*
  * The kdev_t and uuid of a device can never change once it is
  * initially inserted.
  */
-static int dm_hash_insert(const char *name, const char *uuid, struct mapped_device *md)
+static int dm_hash_insert(const char *name, const char *uuid, struct mapped_device *md,
+			  struct dm_ioctl *param)
 {
 	struct hash_cell *cell, *hc;
 
@@ -280,6 +283,8 @@ static int dm_hash_insert(const char *name, const char *uuid, struct mapped_devi
 	down_write(&_hash_lock);
 	hc = __get_name_cell(name);
 	if (hc) {
+		if (param)
+			__dev_status(hc->md, param);
 		dm_put(hc->md);
 		goto bad;
 	}
@@ -290,6 +295,8 @@ static int dm_hash_insert(const char *name, const char *uuid, struct mapped_devi
 		hc = __get_uuid_cell(uuid);
 		if (hc) {
 			__unlink_name(cell);
+			if (param)
+				__dev_status(hc->md, param);
 			dm_put(hc->md);
 			goto bad;
 		}
@@ -901,10 +908,12 @@ static int dev_create(struct file *filp, struct dm_ioctl *param, size_t param_si
 		m = MINOR(huge_decode_dev(param->dev));
 
 	r = dm_create(m, &md, param->flags & DM_DISABLE_UEVENTS_FLAG);
-	if (r)
+	if (r) {
+		DMERR("Could not create device-mapper device");
 		return r;
+	}
 
-	r = dm_hash_insert(param->name, *param->uuid ? param->uuid : NULL, md);
+	r = dm_hash_insert(param->name, *param->uuid ? param->uuid : NULL, md, param);
 	if (r) {
 		dm_put(md);
 		dm_destroy(md);
@@ -2234,6 +2243,7 @@ static int ctl_ioctl(struct file *file, uint command, struct dm_ioctl __user *us
 	int ioctl_flags;
 	int param_flags;
 	unsigned int cmd;
+	bool do_copy;
 	struct dm_ioctl *param;
 	ioctl_fn fn = NULL;
 	size_t input_param_size;
@@ -2307,9 +2317,18 @@ static int ctl_ioctl(struct file *file, uint command, struct dm_ioctl __user *us
 	param->flags |= old_flags;
 
 	/*
-	 * Copy the results back to userland.
+	 * Copy the results back to userland if either:
+	 *
+	 * - The ioctl succeeded.
+	 * - The ioctl is DM_DEV_CREATE, the return value is -EBUSY,
+	 *   and strict parameter checking is enabled.
 	 */
-	if (!r && copy_to_user(user, param, param->data_size))
+	do_copy = r == 0;
+	if (r == -EBUSY && !sloppy_checks(param) && cmd == DM_DEV_CREATE_CMD) {
+		param->flags |= DM_EXISTS_FLAG;
+		do_copy = true;
+	}
+	if (do_copy && copy_to_user(user, param, param->data_size))
 		r = -EFAULT;
 
 out:
@@ -2488,7 +2507,7 @@ int __init dm_early_create(struct dm_ioctl *dmi,
 		return r;
 
 	/* hash insert */
-	r = dm_hash_insert(dmi->name, *dmi->uuid ? dmi->uuid : NULL, md);
+	r = dm_hash_insert(dmi->name, *dmi->uuid ? dmi->uuid : NULL, md, NULL);
 	if (r)
 		goto err_destroy_dm;
 
diff --git a/include/uapi/linux/dm-ioctl.h b/include/uapi/linux/dm-ioctl.h
index b338a4f9a299acda9430995d63fbb490e70c8cd8..195d60be4aba4f20b7220b900c87d6d3a86014a2 100644
--- a/include/uapi/linux/dm-ioctl.h
+++ b/include/uapi/linux/dm-ioctl.h
@@ -344,8 +344,8 @@ enum {
 /* Status bits */
 #define DM_READONLY_FLAG	(1 << 0) /* In/Out */
 #define DM_SUSPEND_FLAG		(1 << 1) /* In/Out */
-#define DM_EXISTS_FLAG		(1 << 2) /* Not used by kernel, reserved for
-					  * libdevmapper in userland
+#define DM_EXISTS_FLAG		(1 << 2) /* Set when trying to create a
+					  * device that already exists
 					  */
 #define DM_PERSISTENT_DEV_FLAG	(1 << 3) /* In */
 
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab


  parent reply	other threads:[~2023-06-24 23:10 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-24 23:09 [dm-devel] [PATCH v2 0/4] Diskseq support in device-mapper Demi Marie Obenour
2023-06-24 23:09 ` Demi Marie Obenour
2023-06-24 23:09 ` [dm-devel] [PATCH v2 1/4] dm ioctl: Allow userspace to opt-in to strict parameter checks Demi Marie Obenour
2023-06-24 23:09   ` Demi Marie Obenour
2023-06-24 23:09 ` [dm-devel] [PATCH v2 2/4] dm ioctl: Allow userspace to provide expected diskseq Demi Marie Obenour
2023-06-24 23:09   ` Demi Marie Obenour
2023-06-25 11:23   ` [dm-devel] " Markus Elfring
2023-06-25 17:39     ` Demi Marie Obenour
2023-06-25 17:39       ` Demi Marie Obenour
2023-06-26 12:59       ` [dm-devel] " Dan Carpenter
2023-06-26 12:59         ` Dan Carpenter
2023-06-26 13:30         ` [dm-devel] [v2 " Markus Elfring
2023-06-26 14:51           ` Dan Carpenter
2023-06-26 14:51             ` Dan Carpenter
2023-06-26 15:19             ` [dm-devel] " Markus Elfring
2023-06-26 16:20         ` [dm-devel] [PATCH v2 " Markus Elfring
2023-06-27  6:14           ` Dan Carpenter
2023-06-27  6:14             ` Dan Carpenter
2023-06-27  7:19             ` [dm-devel] [v2 " Markus Elfring
2023-06-24 23:09 ` [dm-devel] [PATCH v2 3/4] dm ioctl: Allow userspace to suppress uevent generation Demi Marie Obenour
2023-06-24 23:09   ` Demi Marie Obenour
2023-06-25 13:25   ` [dm-devel] " Milan Broz
2023-06-25 13:25     ` Milan Broz
2023-06-25 16:02     ` Demi Marie Obenour
2023-06-25 16:02       ` Demi Marie Obenour
2023-06-25 16:33       ` Milan Broz
2023-06-25 16:33         ` Milan Broz
2023-06-25 16:43         ` Demi Marie Obenour
2023-06-25 16:43           ` Demi Marie Obenour
2023-06-25 17:13           ` Milan Broz
2023-06-25 17:13             ` Milan Broz
2023-06-24 23:09 ` Demi Marie Obenour [this message]
2023-06-24 23:09   ` [PATCH v2 4/4] dm ioctl: inform caller about already-existing device Demi Marie Obenour
2024-01-15 17:56 ` [dm-devel] [PATCH v2 0/4] Diskseq support in device-mapper Martin Wilck
2024-01-15 21:44   ` Demi Marie Obenour
2024-01-16  8:00     ` Martin Wilck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230624230950.2272-5-demi@invisiblethingslab.com \
    --to=demi@invisiblethingslab.com \
    --cc=agk@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=snitzer@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.