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: [PATCH v2 2/4] dm ioctl: Allow userspace to provide expected diskseq
Date: Sat, 24 Jun 2023 19:09:45 -0400	[thread overview]
Message-ID: <20230624230950.2272-3-demi@invisiblethingslab.com> (raw)
In-Reply-To: <20230624230950.2272-1-demi@invisiblethingslab.com>

This can be used to avoid race conditions in which a device is destroyed
and recreated with the same major/minor, name, or UUID.  diskseqs are
only honored if strict parameter checking is on, to avoid any risk of
breaking old userspace.

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

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index e7693479c0cd974ddde69b3b1c4c67abc2ae3ad6..7abaeec33f1884d4588e8563fb02e9ea1a6782c8 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -878,6 +878,9 @@ static void __dev_status(struct mapped_device *md, struct dm_ioctl *param)
 		}
 		dm_put_live_table(md, srcu_idx);
 	}
+
+	if (param->version[0] >= DM_VERSION_MAJOR_STRICT)
+		dm_set_diskseq(param, disk->diskseq);
 }
 
 static int dev_create(struct file *filp, struct dm_ioctl *param, size_t param_size)
@@ -918,6 +921,9 @@ static int dev_create(struct file *filp, struct dm_ioctl *param, size_t param_si
 static struct hash_cell *__find_device_hash_cell(struct dm_ioctl *param)
 {
 	struct hash_cell *hc = NULL;
+	static_assert(offsetof(struct dm_ioctl, diskseq_high) ==
+		      offsetof(struct dm_ioctl, data) + 3,
+		      "diskseq_high field misplaced");
 
 	if (*param->uuid) {
 		if (*param->name || param->dev) {
@@ -946,6 +952,27 @@ static struct hash_cell *__find_device_hash_cell(struct dm_ioctl *param)
 	} else
 		return NULL;
 
+	if (param->version[0] >= DM_VERSION_MAJOR_STRICT) {
+		u64 expected_diskseq = dm_get_diskseq(param);
+		u64 diskseq;
+		struct mapped_device *md = hc->md;
+
+		if (WARN_ON_ONCE(md->disk == NULL))
+			return NULL;
+		diskseq = md->disk->diskseq;
+		if (WARN_ON_ONCE(diskseq == 0))
+			return NULL;
+		if (expected_diskseq != 0) {
+			if (expected_diskseq != diskseq) {
+				DMERR("Diskseq mismatch: expected %llu actual %llu",
+				      expected_diskseq, diskseq);
+				return NULL;
+			}
+		} else {
+			dm_set_diskseq(param, diskseq);
+		}
+	}
+
 	/*
 	 * Sneakily write in both the name and the uuid
 	 * while we have the cell.
@@ -2139,6 +2166,12 @@ static int validate_params(uint cmd, struct dm_ioctl *param,
 		return 0;
 	}
 
+	if (param->data_size < sizeof(struct dm_ioctl)) {
+		DMERR("Entire struct dm_ioctl (size %zu) must be valid, but only %u was valid",
+		      sizeof(struct dm_ioctl), param->data_size);
+		return -EINVAL;
+	}
+
 	/* Check that strings are terminated */
 	if (!no_non_nul_after_nul(param->name, DM_NAME_LEN, cmd, "Name") ||
 	    !no_non_nul_after_nul(param->uuid, DM_UUID_LEN, cmd, "UUID"))
@@ -2148,7 +2181,7 @@ static int validate_params(uint cmd, struct dm_ioctl *param,
 	 * This also checks the last byte of the UUID field, but that was
 	 * checked to be zero above.
 	 */
-	if (*(const u64 *)((const char *)param + (sizeof(*param) - 8)) != 0) {
+	if (*(const u32 *)((const char *)param + (sizeof(*param) - 8)) != 0) {
 		DMERR("second padding field not zeroed in strict mode (cmd %u)", cmd);
 		return -EINVAL;
 	}
@@ -2159,12 +2192,6 @@ static int validate_params(uint cmd, struct dm_ioctl *param,
 		return -EINVAL;
 	}
 
-	if (param->padding != 0) {
-		DMERR("padding not zeroed in strict mode (got %u, cmd %u)",
-		      param->padding, cmd);
-		return -EINVAL;
-	}
-
 	if (param->open_count != 0) {
 		DMERR("open_count not zeroed in strict mode (got %d, cmd %u)",
 		      param->open_count, cmd);
diff --git a/include/uapi/linux/dm-ioctl.h b/include/uapi/linux/dm-ioctl.h
index 62bfdc95ebccb2f1c20c24496a449fe3e2a76113..1d33109aece2ff9854e752066baa96fdf7d85857 100644
--- a/include/uapi/linux/dm-ioctl.h
+++ b/include/uapi/linux/dm-ioctl.h
@@ -146,16 +146,43 @@ struct dm_ioctl {
 	 * For output, the ioctls return the event number, not the cookie.
 	 */
 	__u32 event_nr;      	/* in/out */
-	__u32 padding;
+
+	union {
+		/* valid if DM_VERSION_MAJOR is used */
+		__u32 padding;		/* padding */
+		/* valid if DM_VERSION_MAJOR_STRICT is used */
+		__u32 diskseq_low;	/* in/out: low 4 bytes of the diskseq */
+	};
 
 	__u64 dev;		/* in/out */
 
 	char name[DM_NAME_LEN];	/* device name */
 	char uuid[DM_UUID_LEN];	/* unique identifier for
 				 * the block device */
-	char data[7];		/* padding or data, must be zero in strict mode */
+	union {
+		/* valid if DM_VERSION_MAJOR is used */
+		char data[7];	/* padding or data */
+		/* valid if DM_VERSION_MAJOR_STRICT is used */
+		struct {
+			char _padding[3];	/* padding, must be zeroed */
+			__u32 diskseq_high;	/* in/out: high 4 bytes of the diskseq */
+		} __attribute__((packed));
+	};
 };
 
+__attribute__((always_inline)) static inline __u64
+dm_get_diskseq(const struct dm_ioctl *_i)
+{
+	return (__u64)_i->diskseq_high << 32 | (__u64)_i->diskseq_low;
+}
+
+__attribute__((always_inline)) static inline void
+dm_set_diskseq(struct dm_ioctl *_i, __u64 _diskseq)
+{
+	_i->diskseq_low = (__u32)(_diskseq & 0xFFFFFFFFU);
+	_i->diskseq_high = (__u32)(_diskseq >> 32);
+}
+
 /*
  * Used to specify tables.  These structures appear after the
  * dm_ioctl.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab


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: [dm-devel] [PATCH v2 2/4] dm ioctl: Allow userspace to provide expected diskseq
Date: Sat, 24 Jun 2023 19:09:45 -0400	[thread overview]
Message-ID: <20230624230950.2272-3-demi@invisiblethingslab.com> (raw)
In-Reply-To: <20230624230950.2272-1-demi@invisiblethingslab.com>

This can be used to avoid race conditions in which a device is destroyed
and recreated with the same major/minor, name, or UUID.  diskseqs are
only honored if strict parameter checking is on, to avoid any risk of
breaking old userspace.

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

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index e7693479c0cd974ddde69b3b1c4c67abc2ae3ad6..7abaeec33f1884d4588e8563fb02e9ea1a6782c8 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -878,6 +878,9 @@ static void __dev_status(struct mapped_device *md, struct dm_ioctl *param)
 		}
 		dm_put_live_table(md, srcu_idx);
 	}
+
+	if (param->version[0] >= DM_VERSION_MAJOR_STRICT)
+		dm_set_diskseq(param, disk->diskseq);
 }
 
 static int dev_create(struct file *filp, struct dm_ioctl *param, size_t param_size)
@@ -918,6 +921,9 @@ static int dev_create(struct file *filp, struct dm_ioctl *param, size_t param_si
 static struct hash_cell *__find_device_hash_cell(struct dm_ioctl *param)
 {
 	struct hash_cell *hc = NULL;
+	static_assert(offsetof(struct dm_ioctl, diskseq_high) ==
+		      offsetof(struct dm_ioctl, data) + 3,
+		      "diskseq_high field misplaced");
 
 	if (*param->uuid) {
 		if (*param->name || param->dev) {
@@ -946,6 +952,27 @@ static struct hash_cell *__find_device_hash_cell(struct dm_ioctl *param)
 	} else
 		return NULL;
 
+	if (param->version[0] >= DM_VERSION_MAJOR_STRICT) {
+		u64 expected_diskseq = dm_get_diskseq(param);
+		u64 diskseq;
+		struct mapped_device *md = hc->md;
+
+		if (WARN_ON_ONCE(md->disk == NULL))
+			return NULL;
+		diskseq = md->disk->diskseq;
+		if (WARN_ON_ONCE(diskseq == 0))
+			return NULL;
+		if (expected_diskseq != 0) {
+			if (expected_diskseq != diskseq) {
+				DMERR("Diskseq mismatch: expected %llu actual %llu",
+				      expected_diskseq, diskseq);
+				return NULL;
+			}
+		} else {
+			dm_set_diskseq(param, diskseq);
+		}
+	}
+
 	/*
 	 * Sneakily write in both the name and the uuid
 	 * while we have the cell.
@@ -2139,6 +2166,12 @@ static int validate_params(uint cmd, struct dm_ioctl *param,
 		return 0;
 	}
 
+	if (param->data_size < sizeof(struct dm_ioctl)) {
+		DMERR("Entire struct dm_ioctl (size %zu) must be valid, but only %u was valid",
+		      sizeof(struct dm_ioctl), param->data_size);
+		return -EINVAL;
+	}
+
 	/* Check that strings are terminated */
 	if (!no_non_nul_after_nul(param->name, DM_NAME_LEN, cmd, "Name") ||
 	    !no_non_nul_after_nul(param->uuid, DM_UUID_LEN, cmd, "UUID"))
@@ -2148,7 +2181,7 @@ static int validate_params(uint cmd, struct dm_ioctl *param,
 	 * This also checks the last byte of the UUID field, but that was
 	 * checked to be zero above.
 	 */
-	if (*(const u64 *)((const char *)param + (sizeof(*param) - 8)) != 0) {
+	if (*(const u32 *)((const char *)param + (sizeof(*param) - 8)) != 0) {
 		DMERR("second padding field not zeroed in strict mode (cmd %u)", cmd);
 		return -EINVAL;
 	}
@@ -2159,12 +2192,6 @@ static int validate_params(uint cmd, struct dm_ioctl *param,
 		return -EINVAL;
 	}
 
-	if (param->padding != 0) {
-		DMERR("padding not zeroed in strict mode (got %u, cmd %u)",
-		      param->padding, cmd);
-		return -EINVAL;
-	}
-
 	if (param->open_count != 0) {
 		DMERR("open_count not zeroed in strict mode (got %d, cmd %u)",
 		      param->open_count, cmd);
diff --git a/include/uapi/linux/dm-ioctl.h b/include/uapi/linux/dm-ioctl.h
index 62bfdc95ebccb2f1c20c24496a449fe3e2a76113..1d33109aece2ff9854e752066baa96fdf7d85857 100644
--- a/include/uapi/linux/dm-ioctl.h
+++ b/include/uapi/linux/dm-ioctl.h
@@ -146,16 +146,43 @@ struct dm_ioctl {
 	 * For output, the ioctls return the event number, not the cookie.
 	 */
 	__u32 event_nr;      	/* in/out */
-	__u32 padding;
+
+	union {
+		/* valid if DM_VERSION_MAJOR is used */
+		__u32 padding;		/* padding */
+		/* valid if DM_VERSION_MAJOR_STRICT is used */
+		__u32 diskseq_low;	/* in/out: low 4 bytes of the diskseq */
+	};
 
 	__u64 dev;		/* in/out */
 
 	char name[DM_NAME_LEN];	/* device name */
 	char uuid[DM_UUID_LEN];	/* unique identifier for
 				 * the block device */
-	char data[7];		/* padding or data, must be zero in strict mode */
+	union {
+		/* valid if DM_VERSION_MAJOR is used */
+		char data[7];	/* padding or data */
+		/* valid if DM_VERSION_MAJOR_STRICT is used */
+		struct {
+			char _padding[3];	/* padding, must be zeroed */
+			__u32 diskseq_high;	/* in/out: high 4 bytes of the diskseq */
+		} __attribute__((packed));
+	};
 };
 
+__attribute__((always_inline)) static inline __u64
+dm_get_diskseq(const struct dm_ioctl *_i)
+{
+	return (__u64)_i->diskseq_high << 32 | (__u64)_i->diskseq_low;
+}
+
+__attribute__((always_inline)) static inline void
+dm_set_diskseq(struct dm_ioctl *_i, __u64 _diskseq)
+{
+	_i->diskseq_low = (__u32)(_diskseq & 0xFFFFFFFFU);
+	_i->diskseq_high = (__u32)(_diskseq >> 32);
+}
+
 /*
  * Used to specify tables.  These structures appear after the
  * dm_ioctl.
-- 
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


  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 [PATCH v2 0/4] Diskseq support in device-mapper Demi Marie Obenour
2023-06-24 23:09 ` [dm-devel] " Demi Marie Obenour
2023-06-24 23:09 ` [PATCH v2 1/4] dm ioctl: Allow userspace to opt-in to strict parameter checks Demi Marie Obenour
2023-06-24 23:09   ` [dm-devel] " Demi Marie Obenour
2023-06-24 23:09 ` Demi Marie Obenour [this message]
2023-06-24 23:09   ` [dm-devel] [PATCH v2 2/4] dm ioctl: Allow userspace to provide expected diskseq Demi Marie Obenour
2023-06-25 11:23   ` Markus Elfring
2023-06-25 17:39     ` Demi Marie Obenour
2023-06-25 17:39       ` [dm-devel] " Demi Marie Obenour
2023-06-26 12:59       ` Dan Carpenter
2023-06-26 12:59         ` [dm-devel] " Dan Carpenter
2023-06-26 13:30         ` [dm-devel] [v2 " Markus Elfring
2023-06-26 14:51           ` Dan Carpenter
2023-06-26 14:51             ` [dm-devel] " Dan Carpenter
2023-06-26 15:19             ` 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             ` [dm-devel] " Dan Carpenter
2023-06-27  7:19             ` [dm-devel] [v2 " Markus Elfring
2023-06-24 23:09 ` [PATCH v2 3/4] dm ioctl: Allow userspace to suppress uevent generation Demi Marie Obenour
2023-06-24 23:09   ` [dm-devel] " Demi Marie Obenour
2023-06-25 13:25   ` 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 ` [PATCH v2 4/4] dm ioctl: inform caller about already-existing device Demi Marie Obenour
2023-06-24 23:09   ` [dm-devel] " 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-3-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.