[PATCH 0/4] multipath: fixes for invalid path device handling

[PATCH 0/4] multipath: fixes for invalid path device handling

[Benjamin Marzinski]

I recently got a bug from a user with a bad property
blacklist_exceptions line, where multipath tried to use a zram device,
which caused multipathd to crash. This patch set fixes that crash, and
then adds some additional safeguards to prevent things like this
happening in the future.

The first patch fixes the crash. This crash could be triggered by any
path device that isn't part of a multipath device, and has it's checker
fail to check the path correctly.

The second patch blacklists zram devices specifically. The third patch
fixes up some code weirdness that doesn't really cause any problems,
aside from the messages it prints.

The fourth patch changes how pathinfo works so that it now drops devices
of an unknown type if they can't get a wwid.  The biggest reason to do
this is so that multipath won't hold an open file descriptor for these
devices, which are almost definitely not multipathable (and occasionally
send them TUR ioctls, which are bound to fail, since they aren't scsi
devices).

As an aside, is there a good reason why multipath uses a devnode
blacklist to individually blacklist device types, instead of
blacklisting all denodes, and adding exceptions for the device types
that we do support?

Benjamin Marzinski (4):
  multipathd: avoid null pointer dereference in LOG_MSG
  multipath: blacklist zram devices
  multipathd: fix pp->initialized state ping-ponging
  multipathd: don't resend change events for unknown devices

 libmultipath/blacklist.c   |  2 +-
 libmultipath/checkers.c    |  2 +-
 libmultipath/discovery.c   | 10 +++++++---
 libmultipath/structs.h     |  1 +
 multipath/multipath.conf.5 |  2 +-
 multipathd/main.c          | 10 +++++++---
 6 files changed, 18 insertions(+), 9 deletions(-)

-- 
2.17.2

[PATCH 1/4] multipathd: avoid null pointer dereference in LOG_MSG

[Benjamin Marzinski]

LOG_MSG() will dereference pp->mpp. Commit cb5ec664 added a call to
LOG_MSG() before the check for (!pp->mpp) in check_path.  This can cause
multipathd to crash.  LOG_MSG() should only be called if pp->mpp is set
and a checker is selected.

Also, checker_message() should fail to a generic message if c->cls isn't
set (which means that a checker hasn't been selected).

Fixes: cb5ec664 (multipathd: check_path: improve logging for "unusable
                 path" case)
Cc: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/checkers.c | 2 +-
 multipathd/main.c       | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
index 848c4c3..ca95cae 100644
--- a/libmultipath/checkers.c
+++ b/libmultipath/checkers.c
@@ -295,7 +295,7 @@ const char *checker_message(const struct checker *c)
 {
 	int id;
 
-	if (!c || c->msgid < 0 ||
+	if (!c || !c->cls || c->msgid < 0 ||
 	    (c->msgid >= CHECKER_GENERIC_MSGTABLE_SIZE &&
 	     c->msgid < CHECKER_FIRST_MSGID))
 		goto bad_id;
diff --git a/multipathd/main.c b/multipathd/main.c
index 0e3ac2c..1caa40f 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2017,8 +2017,10 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
 	}
 
 	if (newstate == PATH_WILD || newstate == PATH_UNCHECKED) {
-		condlog(2, "%s: unusable path - checker failed", pp->dev);
-		LOG_MSG(2, verbosity, pp);
+		condlog(2, "%s: unusable path (%s) - checker failed", pp->dev,
+			checker_state_name(newstate));
+		if (pp->mpp && checker_selected(&pp->checker))
+			LOG_MSG(2, verbosity, pp);
 		conf = get_multipath_config();
 		pthread_cleanup_push(put_multipath_config, conf);
 		pathinfo(pp, conf, 0);
-- 
2.17.2

Re: [PATCH 1/4] multipathd: avoid null pointer dereference in LOG_MSG

[Martin Wilck]

On Thu, 2019-02-07 at 17:52 -0600, Benjamin Marzinski wrote:
> LOG_MSG() will dereference pp->mpp. Commit cb5ec664 added a call to
> LOG_MSG() before the check for (!pp->mpp) in check_path.  This can
> cause
> multipathd to crash.  LOG_MSG() should only be called if pp->mpp is
> set
> and a checker is selected.
> 
> Also, checker_message() should fail to a generic message if c->cls
> isn't
> set (which means that a checker hasn't been selected).
> 
> Fixes: cb5ec664 (multipathd: check_path: improve logging for
> "unusable
>                  path" case)
> Cc: Martin Wilck <mwilck@suse.com>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Thanks a lot, but could we do the below instead? IMO it's better to
avoid similar errors in the future.

Martin


From e5bef42f8f9d2aef9406873f515a45914c1aa251 Mon Sep 17 00:00:00 2001
From: Benjamin Marzinski <bmarzins@redhat.com>
Date: Thu, 7 Feb 2019 17:52:59 -0600
Subject: [PATCH] multipathd: avoid null pointer dereference in LOG_MSG

LOG_MSG() will dereference pp->mpp. Commit cb5ec664 added a call to
LOG_MSG() before the check for (!pp->mpp) in check_path.  This can cause
multipathd to crash.  LOG_MSG() should check the fields before dereferencing
them. Make checker_selected() an inline function to allow the compiler
to optimize away the usually redundant test "if (&checker->pp != NULL)".

Also, checker_message() should fail to a generic message if c->cls isn't
set (which means that a checker hasn't been selected).

Fixes: cb5ec664 (multipathd: check_path: improve logging for "unusable
                 path" case)
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/checkers.c | 9 +--------
 libmultipath/checkers.h | 6 +++++-
 multipathd/main.c       | 6 ++++--
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
index 848c4c34..f4fdcae9 100644
--- a/libmultipath/checkers.c
+++ b/libmultipath/checkers.c
@@ -261,13 +261,6 @@ int checker_check (struct checker * c, int path_state)
 	return r;
 }
 
-int checker_selected(const struct checker *c)
-{
-	if (!c)
-		return 0;
-	return c->cls != NULL;
-}
-
 const char *checker_name(const struct checker *c)
 {
 	if (!c || !c->cls)
@@ -295,7 +288,7 @@ const char *checker_message(const struct checker *c)
 {
 	int id;
 
-	if (!c || c->msgid < 0 ||
+	if (!c || !c->cls || c->msgid < 0 ||
 	    (c->msgid >= CHECKER_GENERIC_MSGTABLE_SIZE &&
 	     c->msgid < CHECKER_FIRST_MSGID))
 		goto bad_id;
diff --git a/libmultipath/checkers.h b/libmultipath/checkers.h
index b2e8f9aa..dab197f9 100644
--- a/libmultipath/checkers.h
+++ b/libmultipath/checkers.h
@@ -129,6 +129,11 @@ struct checker {
 						you want to stuff data in. */
 };
 
+static inline int checker_selected(const struct checker *c)
+{
+	return c != NULL && c->cls != NULL;
+}
+
 const char *checker_state_name(int);
 int init_checkers(const char *);
 void cleanup_checkers (void);
@@ -142,7 +147,6 @@ void checker_set_fd (struct checker *, int);
 void checker_enable (struct checker *);
 void checker_disable (struct checker *);
 int checker_check (struct checker *, int);
-int checker_selected(const struct checker *);
 int checker_is_sync(const struct checker *);
 const char *checker_name (const struct checker *);
 /*
diff --git a/multipathd/main.c b/multipathd/main.c
index d1a4f629..d1dd286c 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -92,7 +92,8 @@ static int use_watchdog;
 
 #define LOG_MSG(lvl, verb, pp)					\
 do {								\
-	if (lvl <= verb) {					\
+	if (pp->mpp && checker_selected(&pp->checker) &&	\
+	    lvl <= verb) {					\
 		if (pp->offline)				\
 			condlog(lvl, "%s: %s - path offline",	\
 				pp->mpp->alias, pp->dev);	\
@@ -2017,7 +2018,8 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
 	}
 
 	if (newstate == PATH_WILD || newstate == PATH_UNCHECKED) {
-		condlog(2, "%s: unusable path - checker failed", pp->dev);
+		condlog(2, "%s: unusable path (%s) - checker failed",
+			pp->dev, checker_state_name(newstate));
 		LOG_MSG(2, verbosity, pp);
 		conf = get_multipath_config();
 		pthread_cleanup_push(put_multipath_config, conf);
-- 
2.20.1

Re: [PATCH 1/4] multipathd: avoid null pointer dereference in LOG_MSG\

[Benjamin Marzinski]

On Fri, Feb 08, 2019 at 10:05:51AM +0100, Martin Wilck wrote:
> On Thu, 2019-02-07 at 17:52 -0600, Benjamin Marzinski wrote:
> > LOG_MSG() will dereference pp->mpp. Commit cb5ec664 added a call to
> > LOG_MSG() before the check for (!pp->mpp) in check_path.  This can
> > cause
> > multipathd to crash.  LOG_MSG() should only be called if pp->mpp is
> > set
> > and a checker is selected.
> > 
> > Also, checker_message() should fail to a generic message if c->cls
> > isn't
> > set (which means that a checker hasn't been selected).
> > 
> > Fixes: cb5ec664 (multipathd: check_path: improve logging for
> > "unusable
> >                  path" case)
> > Cc: Martin Wilck <mwilck@suse.com>
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> 
> Thanks a lot, but could we do the below instead? IMO it's better to
> avoid similar errors in the future.
> 
> Martin

Sure. I can resend the series with this patch instead.

-Ben

> 
> 
> >From e5bef42f8f9d2aef9406873f515a45914c1aa251 Mon Sep 17 00:00:00 2001
> From: Benjamin Marzinski <bmarzins@redhat.com>
> Date: Thu, 7 Feb 2019 17:52:59 -0600
> Subject: [PATCH] multipathd: avoid null pointer dereference in LOG_MSG
> 
> LOG_MSG() will dereference pp->mpp. Commit cb5ec664 added a call to
> LOG_MSG() before the check for (!pp->mpp) in check_path.  This can cause
> multipathd to crash.  LOG_MSG() should check the fields before dereferencing
> them. Make checker_selected() an inline function to allow the compiler
> to optimize away the usually redundant test "if (&checker->pp != NULL)".
> 
> Also, checker_message() should fail to a generic message if c->cls isn't
> set (which means that a checker hasn't been selected).
> 
> Fixes: cb5ec664 (multipathd: check_path: improve logging for "unusable
>                  path" case)
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/checkers.c | 9 +--------
>  libmultipath/checkers.h | 6 +++++-
>  multipathd/main.c       | 6 ++++--
>  3 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
> index 848c4c34..f4fdcae9 100644
> --- a/libmultipath/checkers.c
> +++ b/libmultipath/checkers.c
> @@ -261,13 +261,6 @@ int checker_check (struct checker * c, int path_state)
>  	return r;
>  }
>  
> -int checker_selected(const struct checker *c)
> -{
> -	if (!c)
> -		return 0;
> -	return c->cls != NULL;
> -}
> -
>  const char *checker_name(const struct checker *c)
>  {
>  	if (!c || !c->cls)
> @@ -295,7 +288,7 @@ const char *checker_message(const struct checker *c)
>  {
>  	int id;
>  
> -	if (!c || c->msgid < 0 ||
> +	if (!c || !c->cls || c->msgid < 0 ||
>  	    (c->msgid >= CHECKER_GENERIC_MSGTABLE_SIZE &&
>  	     c->msgid < CHECKER_FIRST_MSGID))
>  		goto bad_id;
> diff --git a/libmultipath/checkers.h b/libmultipath/checkers.h
> index b2e8f9aa..dab197f9 100644
> --- a/libmultipath/checkers.h
> +++ b/libmultipath/checkers.h
> @@ -129,6 +129,11 @@ struct checker {
>  						you want to stuff data in. */
>  };
>  
> +static inline int checker_selected(const struct checker *c)
> +{
> +	return c != NULL && c->cls != NULL;
> +}
> +
>  const char *checker_state_name(int);
>  int init_checkers(const char *);
>  void cleanup_checkers (void);
> @@ -142,7 +147,6 @@ void checker_set_fd (struct checker *, int);
>  void checker_enable (struct checker *);
>  void checker_disable (struct checker *);
>  int checker_check (struct checker *, int);
> -int checker_selected(const struct checker *);
>  int checker_is_sync(const struct checker *);
>  const char *checker_name (const struct checker *);
>  /*
> diff --git a/multipathd/main.c b/multipathd/main.c
> index d1a4f629..d1dd286c 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -92,7 +92,8 @@ static int use_watchdog;
>  
>  #define LOG_MSG(lvl, verb, pp)					\
>  do {								\
> -	if (lvl <= verb) {					\
> +	if (pp->mpp && checker_selected(&pp->checker) &&	\
> +	    lvl <= verb) {					\
>  		if (pp->offline)				\
>  			condlog(lvl, "%s: %s - path offline",	\
>  				pp->mpp->alias, pp->dev);	\
> @@ -2017,7 +2018,8 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
>  	}
>  
>  	if (newstate == PATH_WILD || newstate == PATH_UNCHECKED) {
> -		condlog(2, "%s: unusable path - checker failed", pp->dev);
> +		condlog(2, "%s: unusable path (%s) - checker failed",
> +			pp->dev, checker_state_name(newstate));
>  		LOG_MSG(2, verbosity, pp);
>  		conf = get_multipath_config();
>  		pthread_cleanup_push(put_multipath_config, conf);
> -- 
> 2.20.1
> 

[PATCH 2/4] multipath: blacklist zram devices

[Benjamin Marzinski]

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/blacklist.c   | 2 +-
 multipath/multipath.conf.5 | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
index 709895e..e0d0279 100644
--- a/libmultipath/blacklist.c
+++ b/libmultipath/blacklist.c
@@ -192,7 +192,7 @@ setup_default_blist (struct config * conf)
 	char * str;
 	int i;
 
-	str = STRDUP("^(ram|raw|loop|fd|md|dm-|sr|scd|st|dcssblk)[0-9]");
+	str = STRDUP("^(ram|zram|raw|loop|fd|md|dm-|sr|scd|st|dcssblk)[0-9]");
 	if (!str)
 		return 1;
 	if (store_ble(conf->blist_devnode, str, ORIGIN_DEFAULT))
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 88b8edd..0fe8461 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -1218,7 +1218,7 @@ Regular expression matching the device nodes to be excluded/included.
 .RS
 .PP
 The default \fIblacklist\fR consists of the regular expressions
-"^(ram|raw|loop|fd|md|dm-|sr|scd|st|dcssblk)[0-9]" and
+"^(ram|zram|raw|loop|fd|md|dm-|sr|scd|st|dcssblk)[0-9]" and
 "^(td|hd|vd)[a-z]". This causes virtual devices, non-disk devices, and some other
 device types to be excluded from multipath handling by default.
 .RE
-- 
2.17.2

Re: [PATCH 2/4] multipath: blacklist zram devices

[Martin Wilck]

On Thu, 2019-02-07 at 17:53 -0600, Benjamin Marzinski wrote:
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/blacklist.c   | 2 +-
>  multipath/multipath.conf.5 | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Ack, but - how come that this device hadn't been auto-blacklisted
by the "udev property missing" test?

Martin

> 
> diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
> index 709895e..e0d0279 100644
> --- a/libmultipath/blacklist.c
> +++ b/libmultipath/blacklist.c
> @@ -192,7 +192,7 @@ setup_default_blist (struct config * conf)
>  	char * str;
>  	int i;
>  
> -	str = STRDUP("^(ram|raw|loop|fd|md|dm-|sr|scd|st|dcssblk)[0-
> 9]");
> +	str = STRDUP("^(ram|zram|raw|loop|fd|md|dm-
> |sr|scd|st|dcssblk)[0-9]");
>  	if (!str)
>  		return 1;
>  	if (store_ble(conf->blist_devnode, str, ORIGIN_DEFAULT))
> diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
> index 88b8edd..0fe8461 100644
> --- a/multipath/multipath.conf.5
> +++ b/multipath/multipath.conf.5
> @@ -1218,7 +1218,7 @@ Regular expression matching the device nodes to
> be excluded/included.
>  .RS
>  .PP
>  The default \fIblacklist\fR consists of the regular expressions
> -"^(ram|raw|loop|fd|md|dm-|sr|scd|st|dcssblk)[0-9]" and
> +"^(ram|zram|raw|loop|fd|md|dm-|sr|scd|st|dcssblk)[0-9]" and
>  "^(td|hd|vd)[a-z]". This causes virtual devices, non-disk devices,
> and some other
>  device types to be excluded from multipath handling by default.
>  .RE

[PATCH 3/4] multipathd: fix pp->initialized state ping-ponging

[Benjamin Marzinski]

When a multipath device fails to get a wwid in pathinfo, it moves to the
INIT_MISSING_UDEV state. After a device in this state sends
retrigger_tries change uevents in check_path(), it moves to the
INIT_FAILED state.  However, when check_path() is run on a device in
INIT_FAILED, it can call pathinfo, which will set the path back
into INIT_MISSING_UDEV if it cannot get a wwid.  The next call to
check_path() will put the path back into INIT_FAILED.  The device will
continue to ping-pong between these states.

To solve this a new pp->initialized state has been added INIT_NEW.  New
path devices start in this state, instead of INIT_FAILED. INIT_NEW and
INIT_FAILED are treated exactly the same, with one exception. A device
in INIT_FAILED cannot transition back to INIT_MISSING_UDEV.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/discovery.c | 8 +++++---
 libmultipath/structs.h   | 1 +
 multipathd/main.c        | 4 +++-
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 1748eeb..6aef188 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1966,8 +1966,10 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
 	if ((mask & DI_WWID) && !strlen(pp->wwid)) {
 		get_uid(pp, path_state, pp->udev);
 		if (!strlen(pp->wwid)) {
-			pp->initialized = INIT_MISSING_UDEV;
-			pp->tick = conf->retrigger_delay;
+			if (pp->initialized != INIT_FAILED) {
+				pp->initialized = INIT_MISSING_UDEV;
+				pp->tick = conf->retrigger_delay;
+			}
 			return PATHINFO_OK;
 		}
 		else
@@ -2000,7 +2002,7 @@ blank:
 	 * Recoverable error, for example faulty or offline path
 	 */
 	pp->chkrstate = pp->state = PATH_DOWN;
-	if (pp->initialized == INIT_FAILED)
+	if (pp->initialized == INIT_NEW || pp->initialized == INIT_FAILED)
 		memset(pp->wwid, 0, WWID_SIZE);
 
 	return PATHINFO_OK;
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 375c728..b794b0d 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -202,6 +202,7 @@ enum ghost_delay_states {
 };
 
 enum initialized_states {
+	INIT_NEW,
 	INIT_FAILED,
 	INIT_MISSING_UDEV,
 	INIT_REQUESTED_UDEV,
diff --git a/multipathd/main.c b/multipathd/main.c
index 1caa40f..4d0fa8c 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2028,7 +2028,9 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
 		return 1;
 	}
 	if (!pp->mpp) {
-		if (!strlen(pp->wwid) && pp->initialized == INIT_FAILED &&
+		if (!strlen(pp->wwid) &&
+		    (pp->initialized == INIT_FAILED ||
+		     pp->initialized == INIT_NEW) &&
 		    (newstate == PATH_UP || newstate == PATH_GHOST)) {
 			condlog(2, "%s: add missing path", pp->dev);
 			conf = get_multipath_config();
-- 
2.17.2

Re: [PATCH 3/4] multipathd: fix pp->initialized state ping-ponging

[Martin Wilck]

On Thu, 2019-02-07 at 17:53 -0600, Benjamin Marzinski wrote:
> When a multipath device fails to get a wwid in pathinfo, it moves to
> the
> INIT_MISSING_UDEV state. After a device in this state sends
> retrigger_tries change uevents in check_path(), it moves to the
> INIT_FAILED state.  However, when check_path() is run on a device in
> INIT_FAILED, it can call pathinfo, which will set the path back
> into INIT_MISSING_UDEV if it cannot get a wwid.  The next call to
> check_path() will put the path back into INIT_FAILED.  The device
> will
> continue to ping-pong between these states.
> 
> To solve this a new pp->initialized state has been added
> INIT_NEW.  New
> path devices start in this state, instead of INIT_FAILED. INIT_NEW
> and
> INIT_FAILED are treated exactly the same, with one exception. A
> device
> in INIT_FAILED cannot transition back to INIT_MISSING_UDEV.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>

> ---
>  libmultipath/discovery.c | 8 +++++---
>  libmultipath/structs.h   | 1 +
>  multipathd/main.c        | 4 +++-
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 1748eeb..6aef188 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -1966,8 +1966,10 @@ int pathinfo(struct path *pp, struct config
> *conf, int mask)
>  	if ((mask & DI_WWID) && !strlen(pp->wwid)) {
>  		get_uid(pp, path_state, pp->udev);
>  		if (!strlen(pp->wwid)) {
> -			pp->initialized = INIT_MISSING_UDEV;
> -			pp->tick = conf->retrigger_delay;
> +			if (pp->initialized != INIT_FAILED) {
> +				pp->initialized = INIT_MISSING_UDEV;
> +				pp->tick = conf->retrigger_delay;
> +			}
>  			return PATHINFO_OK;
>  		}
>  		else
> @@ -2000,7 +2002,7 @@ blank:
>  	 * Recoverable error, for example faulty or offline path
>  	 */
>  	pp->chkrstate = pp->state = PATH_DOWN;
> -	if (pp->initialized == INIT_FAILED)
> +	if (pp->initialized == INIT_NEW || pp->initialized ==
> INIT_FAILED)
>  		memset(pp->wwid, 0, WWID_SIZE);
>  
>  	return PATHINFO_OK;
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index 375c728..b794b0d 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -202,6 +202,7 @@ enum ghost_delay_states {
>  };
>  
>  enum initialized_states {
> +	INIT_NEW,
>  	INIT_FAILED,
>  	INIT_MISSING_UDEV,
>  	INIT_REQUESTED_UDEV,
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 1caa40f..4d0fa8c 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2028,7 +2028,9 @@ check_path (struct vectors * vecs, struct path
> * pp, int ticks)
>  		return 1;
>  	}
>  	if (!pp->mpp) {
> -		if (!strlen(pp->wwid) && pp->initialized == INIT_FAILED
> &&
> +		if (!strlen(pp->wwid) &&
> +		    (pp->initialized == INIT_FAILED ||
> +		     pp->initialized == INIT_NEW) &&
>  		    (newstate == PATH_UP || newstate == PATH_GHOST)) {
>  			condlog(2, "%s: add missing path", pp->dev);
>  			conf = get_multipath_config();

[PATCH 4/4] multipathd: don't resend change events for unknown devices

[Benjamin Marzinski]

If multipath fails to get the wwid for a device, and the device is
of an unknown type (pp->bus == SYSFS_BUS_UNDEF), don't send change
events. Instead, assume that the device was not meant to be used
and skip it.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/discovery.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 6aef188..10bd8cd 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1966,6 +1966,8 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
 	if ((mask & DI_WWID) && !strlen(pp->wwid)) {
 		get_uid(pp, path_state, pp->udev);
 		if (!strlen(pp->wwid)) {
+			if (pp->bus == SYSFS_BUS_UNDEF)
+				return PATHINFO_SKIPPED;
 			if (pp->initialized != INIT_FAILED) {
 				pp->initialized = INIT_MISSING_UDEV;
 				pp->tick = conf->retrigger_delay;
-- 
2.17.2

Re: [PATCH 4/4] multipathd: don't resend change events for unknown devices

[Martin Wilck]

On Thu, 2019-02-07 at 17:53 -0600, Benjamin Marzinski wrote:
> If multipath fails to get the wwid for a device, and the device is
> of an unknown type (pp->bus == SYSFS_BUS_UNDEF), don't send change
> events. Instead, assume that the device was not meant to be used
> and skip it.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>

> ---
>  libmultipath/discovery.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 6aef188..10bd8cd 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -1966,6 +1966,8 @@ int pathinfo(struct path *pp, struct config
> *conf, int mask)
>  	if ((mask & DI_WWID) && !strlen(pp->wwid)) {
>  		get_uid(pp, path_state, pp->udev);
>  		if (!strlen(pp->wwid)) {
> +			if (pp->bus == SYSFS_BUS_UNDEF)
> +				return PATHINFO_SKIPPED;
>  			if (pp->initialized != INIT_FAILED) {
>  				pp->initialized = INIT_MISSING_UDEV;
>  				pp->tick = conf->retrigger_delay;