All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Bunker <brian@purestorage.com>
To: dm-devel@lists.linux.dev
Cc: Benjamin Marzinski <bmarzins@redhat.com>,
	Martin Wilck <mwilck@suse.com>,
	Christophe Varoqui <christophe.varoqui@opensvc.com>,
	Brian Bunker <brian@purestorage.com>
Subject: [PATCH v1 1/1] multipathd: verify udev WWIDs with the device before moving paths
Date: Thu, 29 Oct 2026 10:57:27 -0700	[thread overview]
Message-ID: <20261029175727.25457-2-brian@purestorage.com> (raw)
In-Reply-To: <20261029175727.25457-1-brian@purestorage.com>

multipathd takes a path's WWID from udev. When a change uevent reports
a different WWID, uev_update_path() removes the path from its map and
re-adds it through uev_add_path(), which places it in whatever map owns
the new WWID. Nothing checks the new value against the device itself;
recheck_wwid only covers paths coming back up in the checker and
"multipathd add path". If udev reports the wrong WWID, or the uevent is
stale by the time it is processed, the path goes into the map of a
different LUN and I/O for one LUN is silently sent to another.

Use SG_IO to read VPD page 0x83 from the device in two places, for
paths where can_recheck_wwid() is true:

- In uev_update_path(), if the device still reports the cached WWID,
  ignore the udev value and leave the path where it is.

- In ev_add_path(), refuse to add a path whose WWID the device doesn't
  confirm. The path is orphaned, its WWID is cleared and it is marked
  INIT_MISSING_UDEV, so the checker retriggers udev (bounded by
  retrigger_tries) and the path is re-added from fresh data.

ev_add_path() is the common point for uevent, CLI and checker adds, so
the second check holds even when the udev data is stale by the time it
is used. If the SG_IO read fails, behavior is unchanged.

Split the SG_IO read out of check_path_wwid_change() into
get_sgio_wwid() so all three checks share it.

Signed-off-by: Brian Bunker <brian@purestorage.com>
---
 multipathd/main.c | 57 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 5 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 9bce4d6b..656efb6e 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1086,14 +1086,16 @@ handle_path_wwid_change(struct path *pp, struct vectors *vecs)
 	return removed;
 }
 
-bool
-check_path_wwid_change(struct path *pp)
+/*
+ * Read the WWID directly from the device with SG_IO, bypassing udev
+ * and sysfs. Returns true and fills in wwid on success.
+ */
+static bool get_sgio_wwid(struct path *pp, char *wwid)
 {
-	char wwid[WWID_SIZE];
-	int len = 0;
+	int len;
 	size_t i;
 
-	if (!strlen(pp->wwid))
+	if (pp->fd < 0)
 		return false;
 
 	/* Get the real fresh device wwid by sgio. sysfs still has old
@@ -1111,6 +1113,15 @@ check_path_wwid_change(struct path *pp)
 		/* no-op */
 	wwid[i] = '\0';
 	condlog(4, "%s: Got wwid %s by sgio", pp->dev, wwid);
+	return true;
+}
+
+bool check_path_wwid_change(struct path *pp)
+{
+	char wwid[WWID_SIZE];
+
+	if (!strlen(pp->wwid) || !get_sgio_wwid(pp, wwid))
+		return false;
 
 	if (strncmp(wwid, pp->wwid, WWID_SIZE)) {
 		condlog(0, "%s: wwid '%s' doesn't match wwid '%s' from device",
@@ -1121,6 +1132,24 @@ check_path_wwid_change(struct path *pp)
 	return false;
 }
 
+/*
+ * udev reported a new WWID for a path. Returns true if the device
+ * itself still reports old_wwid, in which case the udev value must
+ * not be trusted.
+ */
+static bool device_kept_wwid(struct path *pp, const char *old_wwid)
+{
+	char wwid[WWID_SIZE];
+
+	if (!can_recheck_wwid(pp) || !get_sgio_wwid(pp, wwid) ||
+	    strncmp(wwid, old_wwid, WWID_SIZE))
+		return false;
+
+	condlog(0, "%s: ignoring wwid '%s' from udev, device still reports '%s'",
+		pp->dev, pp->wwid, wwid);
+	return true;
+}
+
 /*
  * uev_add_path can call uev_update_path, and uev_update_path can call
  * uev_add_path
@@ -1315,6 +1344,20 @@ ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map)
 		condlog(0, "%s: failed to get path uid", pp->dev);
 		goto fail; /* leave path added to pathvec */
 	}
+	/*
+	 * The WWID normally comes from udev, possibly from an event that
+	 * was queued long before we got here. Adding a path to the map
+	 * of a different LUN corrupts data, so confirm the WWID with the
+	 * device first. On a mismatch, leave the path out and let the
+	 * checker retrigger udev to get a fresh WWID.
+	 */
+	if (can_recheck_wwid(pp) && check_path_wwid_change(pp)) {
+		condlog(0, "%s: not adding path, wwid '%s' not confirmed by device",
+			pp->dev, pp->wwid);
+		memset(pp->wwid, 0, WWID_SIZE);
+		pp->initialized = INIT_MISSING_UDEV;
+		goto fail;
+	}
 	mpp = find_mp_by_wwid(vecs->mpvec, pp->wwid);
 	if (mpp && pp->size && mpp->size != pp->size) {
 		condlog(0, "%s: failed to add new path %s, device size mismatch", mpp->alias, pp->dev);
@@ -1702,6 +1745,10 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
 			strcpy(pp->wwid, wwid);
 		else if (strlen(wwid) &&
 			 strncmp(wwid, pp->wwid, WWID_SIZE) != 0) {
+			if (device_kept_wwid(pp, wwid)) {
+				strcpy(pp->wwid, wwid);
+				goto out;
+			}
 			condlog(0, "%s: path wwid changed from '%s' to '%s'",
 				uev->kernel, wwid, pp->wwid);
 			ev_remove_path(pp, vecs, 1);
-- 
2.56.0


  reply	other threads:[~2026-10-29 17:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29 17:57 [PATCH v1 0/1] multipathd: don't trust udev WWIDs when moving paths between maps Brian Bunker
2026-10-29 17:57 ` Brian Bunker [this message]
2026-10-29 21:27   ` [PATCH v1 1/1] multipathd: verify udev WWIDs with the device before moving paths Benjamin Marzinski
2026-10-29 23:39     ` Brian Bunker

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=20261029175727.25457-2-brian@purestorage.com \
    --to=brian@purestorage.com \
    --cc=bmarzins@redhat.com \
    --cc=christophe.varoqui@opensvc.com \
    --cc=dm-devel@lists.linux.dev \
    --cc=mwilck@suse.com \
    /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.