[PATCH v3 1/1] multipathd: check re-added paths' WWIDs against sysfs

[Brian Bunker <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. If the udev value is wrong, 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.

Before such a re-add puts the path into a map, compare its WWID with
one built from the VPD page 0x83 data the kernel caches in sysfs. If
they differ, leave the path out of the map: orphan it, clear its WWID
and mark it INIT_MISSING_UDEV, so the checker retriggers udev and the
path is re-added from fresh data. This reads no data from the device,
so it can't hang. The check only runs for paths where
can_recheck_wwid() is true, where the udev WWID and the one built from
sysfs come from the same designator selection.

To tell re-adds from ordinary adds, turn the need_do_map argument of
uev_add_path() and ev_add_path() into enum path_add_mode, with a new
PATH_ADD_REINIT value that behaves like PATH_ADD_MAP and is passed only
for the re-add at the end of uev_update_path(). Ordinary adds are
unchanged.

Add get_sysfs_wwid() to libmultipath. It shares get_vpd_uid() and the
WWID clean-up in get_uid() with the existing sysfs fallback, so both
build the WWID the same way.

Signed-off-by: Brian Bunker <brian@purestorage.com>
---
 libmultipath/discovery.c          | 42 ++++++++++++++++++++++---------
 libmultipath/discovery.h          |  1 +
 libmultipath/libmultipath.version |  5 ++++
 multipathd/cli_handlers.c         |  2 +-
 multipathd/main.c                 | 39 ++++++++++++++++++++--------
 multipathd/main.h                 | 12 ++++++++-
 6 files changed, 76 insertions(+), 25 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 344da19c..62286928 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -2180,8 +2180,7 @@ get_udev_uid(struct path * pp, const char *uid_attribute, struct udev_device *ud
 	return len;
 }
 
-static int
-get_vpd_uid(struct path * pp)
+static int get_vpd_uid(const struct path *pp, char *wwid, int len)
 {
 	struct udev_device *parent = pp->udev;
 
@@ -2195,7 +2194,7 @@ get_vpd_uid(struct path * pp)
 	if (!parent)
 		return -EINVAL;
 
-	return get_vpd_sysfs(parent, 0x83, pp->wwid, WWID_SIZE);
+	return get_vpd_sysfs(parent, 0x83, wwid, len);
 }
 
 /* based on code from s390-tools/dasdinfo/dasdinfo.c */
@@ -2236,7 +2235,7 @@ static ssize_t uid_fallback(struct path *pp, int path_state,
 		len = dasd_get_uid(pp);
 		*origin = "sysfs";
 	} else if (pp->bus == SYSFS_BUS_SCSI) {
-		len = get_vpd_uid(pp);
+		len = get_vpd_uid(pp, pp->wwid, WWID_SIZE);
 		*origin = "sysfs";
 		if (len < 0 && path_state == PATH_UP) {
 			condlog(1, "%s: failed to get sysfs uid: %s",
@@ -2299,6 +2298,17 @@ bool can_recheck_wwid(const struct path *pp)
 	return (pp->bus == SYSFS_BUS_SCSI && has_uid_fallback(pp));
 }
 
+static void tidy_wwid(char *wwid)
+{
+	size_t i;
+
+	strchop(wwid);
+	for (i = 0; wwid[i] != '\0'; i++) {
+		if (wwid[i] == '/')
+			wwid[i] = '_';
+	}
+}
+
 int
 get_uid (struct path * pp, int path_state, struct udev_device *udev,
 	 int allow_fallback)
@@ -2307,7 +2317,6 @@ get_uid (struct path * pp, int path_state, struct udev_device *udev,
 	ssize_t len = 0;
 	struct config *conf;
 	int used_fallback = 0;
-	size_t i;
 
 	if (!pp->uid_attribute) {
 		conf = get_multipath_config();
@@ -2347,18 +2356,27 @@ get_uid (struct path * pp, int path_state, struct udev_device *udev,
 			pp->wwid);
 		memset(pp->wwid, 0x0, WWID_SIZE);
 		return 1;
-	} else {
-		strchop(pp->wwid);
-		for (i = 0; pp->wwid[i] != '\0'; i++) {
-			if (pp->wwid[i] == '/')
-				pp->wwid[i] = '_';
-		}
-	}
+	} else
+		tidy_wwid(pp->wwid);
 	condlog((used_fallback)? 1 : 3, "%s: uid = %s (%s)", pp->dev,
 		*pp->wwid == '\0' ? "<empty>" : pp->wwid, origin);
 	return 0;
 }
 
+/*
+ * Get the WWID of a SCSI path from the VPD page 0x83 data the kernel
+ * caches in sysfs, built the same way as get_uid() does when it falls
+ * back to sysfs. This does no I/O on the device. It only matches the
+ * udev WWID if can_recheck_wwid(pp) is true.
+ */
+bool get_sysfs_wwid(const struct path *pp, char *wwid)
+{
+	if (pp->bus != SYSFS_BUS_SCSI || get_vpd_uid(pp, wwid, WWID_SIZE) <= 0)
+		return false;
+	tidy_wwid(wwid);
+	return true;
+}
+
 int pathinfo(struct path *pp, struct config *conf, int mask)
 {
 	int path_state;
diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
index 2ef1d94b..e433a6dd 100644
--- a/libmultipath/discovery.h
+++ b/libmultipath/discovery.h
@@ -56,6 +56,7 @@ ssize_t sysfs_get_inquiry(struct udev_device *udev,
 int sysfs_get_asymmetric_access_state(struct path *pp,
 				      char *buff, int buflen);
 bool can_recheck_wwid(const struct path *pp);
+bool get_sysfs_wwid(const struct path *pp, char *wwid);
 int get_uid(struct path * pp, int path_state, struct udev_device *udev,
 	    int allow_fallback);
 bool is_vpd_page_supported(int fd, int pg);
diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version
index f631f453..4f8e1fd5 100644
--- a/libmultipath/libmultipath.version
+++ b/libmultipath/libmultipath.version
@@ -246,3 +246,8 @@ local:
 	*;
 };
 
+LIBMULTIPATH_34.1.0 {
+global:
+	get_sysfs_wwid;
+} LIBMULTIPATH_34.0.0;
+
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index f5a13c46..9b4a04c2 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -673,7 +673,7 @@ cli_add_path (void *v, struct strbuf *reply, void *data)
 			return 1;
 		}
 	}
-	return ev_add_path(pp, vecs, 1);
+	return ev_add_path(pp, vecs, PATH_ADD_MAP);
 blacklisted:
 	append_strbuf_str(reply, "blacklisted\n");
 	condlog(2, "%s: path blacklisted", param);
diff --git a/multipathd/main.c b/multipathd/main.c
index 9bce4d6b..45ba6aff 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1128,7 +1128,7 @@ check_path_wwid_change(struct path *pp)
 static int uev_update_path (struct uevent *uev, struct vectors * vecs);
 
 static int
-uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
+uev_add_path(struct uevent *uev, struct vectors *vecs, enum path_add_mode mode)
 {
 	struct path *pp;
 	int ret = 0, i;
@@ -1180,7 +1180,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
 				     DI_ALL | DI_BLACKLIST);
 			pthread_cleanup_pop(1);
 			if (r == PATHINFO_OK && !prev_mpp)
-				ret = ev_add_path(pp, vecs, need_do_map);
+				ret = ev_add_path(pp, vecs, mode);
 			else if (r == PATHINFO_OK &&
 				 !strncmp(pp->wwid, prev_mpp->wwid, WWID_SIZE)) {
 				/*
@@ -1255,7 +1255,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
 		conf = get_multipath_config();
 		pp->checkint = conf->checkint;
 		put_multipath_config(conf);
-		ret = ev_add_path(pp, vecs, need_do_map);
+		ret = ev_add_path(pp, vecs, mode);
 	} else {
 		condlog(0, "%s: failed to store path info, "
 			"dropping event",
@@ -1297,10 +1297,10 @@ sysfs_get_ro (struct path *pp)
  * 0: added
  * 1: error
  */
-int
-ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map)
+int ev_add_path(struct path *pp, struct vectors *vecs, enum path_add_mode mode)
 {
 	struct multipath * mpp;
+	char sysfs_wwid[WWID_SIZE];
 	char *params __attribute__((cleanup(cleanup_charp))) = NULL;
 	int retries = 3;
 	int start_waiter = 0;
@@ -1315,6 +1315,23 @@ 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 */
 	}
+	/*
+	 * This path is being re-added because its udev data was missing or
+	 * changed. The uevent may have been queued long before we got here,
+	 * and adding a path to the map of a different LUN corrupts data, so
+	 * check the WWID against the VPD data the kernel has cached in
+	 * sysfs. On a mismatch, leave the path out and let the checker
+	 * retrigger udev to get a fresh WWID.
+	 */
+	if (mode == PATH_ADD_REINIT && can_recheck_wwid(pp) &&
+	    get_sysfs_wwid(pp, sysfs_wwid) &&
+	    strncmp(sysfs_wwid, pp->wwid, WWID_SIZE)) {
+		condlog(0, "%s: not adding path, wwid '%s' from udev doesn't match '%s' from sysfs",
+			pp->dev, pp->wwid, sysfs_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);
@@ -1375,7 +1392,7 @@ rescan:
 	if (ro == 1)
 		mpp->force_readonly = 1;
 
-	if (!need_do_map)
+	if (mode == PATH_ADD_NO_MAP)
 		return 0;
 
 	if (!dm_map_present(mpp->alias)) {
@@ -1789,7 +1806,7 @@ out:
 	}
 	/* pp->initialized must not be INIT_PARTIAL if needs_reinit is set */
 	if (needs_reinit)
-		retval = uev_add_path(uev, vecs, 1);
+		retval = uev_add_path(uev, vecs, PATH_ADD_REINIT);
 	return retval;
 }
 
@@ -1905,13 +1922,13 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 	 */
 	list_for_each_entry_safe(merge_uev, tmp, &uev->merge_node, node) {
 		if (!strncmp(merge_uev->action, "add", 3))
-			r += uev_add_path(merge_uev, vecs, 0);
+			r += uev_add_path(merge_uev, vecs, PATH_ADD_NO_MAP);
 		if (!strncmp(merge_uev->action, "remove", 6))
 			r += uev_remove_path(merge_uev, vecs, 0);
 	}
 
 	if (!strncmp(uev->action, "add", 3))
-		r += uev_add_path(uev, vecs, 1);
+		r += uev_add_path(uev, vecs, PATH_ADD_MAP);
 	if (!strncmp(uev->action, "remove", 6))
 		r += uev_remove_path(uev, vecs, 1);
 	if (!strncmp(uev->action, "change", 6))
@@ -3015,7 +3032,7 @@ update_uninitialized_path(struct vectors * vecs, struct path * pp)
 		pthread_cleanup_pop(1);
 		/* INIT_OK implies ret == PATHINFO_OK */
 		if (pp->initialized == INIT_OK) {
-			ev_add_path(pp, vecs, 1);
+			ev_add_path(pp, vecs, PATH_ADD_MAP);
 			pp->tick = 1;
 		} else if (ret == PATHINFO_SKIPPED) {
 			int i;
@@ -3036,7 +3053,7 @@ update_uninitialized_path(struct vectors * vecs, struct path * pp)
 					CHECK_PATH_REMOVED :
 					CHECK_PATH_SKIPPED;
 		}
-		ev_add_path(pp, vecs, 1);
+		ev_add_path(pp, vecs, PATH_ADD_MAP);
 		pp->tick = 1;
 	}
 	return CHECK_PATH_CHECKED;
diff --git a/multipathd/main.h b/multipathd/main.h
index b66208ec..d3826a89 100644
--- a/multipathd/main.h
+++ b/multipathd/main.h
@@ -36,7 +36,17 @@ enum daemon_status wait_for_state_change_if(enum daemon_status oldstate,
 					    unsigned long ms);
 void schedule_reconfigure(enum force_reload_types requested_type);
 int need_to_delay_reconfig (struct vectors *);
-int ev_add_path (struct path *, struct vectors *, int);
+/*
+ * PATH_ADD_REINIT is PATH_ADD_MAP for a path that is being re-added
+ * because its udev data was missing or changed. Its WWID is checked
+ * against sysfs before the path is put into a map.
+ */
+enum path_add_mode {
+	PATH_ADD_NO_MAP,
+	PATH_ADD_MAP,
+	PATH_ADD_REINIT,
+};
+int ev_add_path(struct path *, struct vectors *, enum path_add_mode);
 int ev_remove_path (struct path *, struct vectors *, int);
 int ev_add_map (char *, const char *, struct vectors *);
 int flush_map(struct multipath *, struct vectors *);
-- 
2.56.0