[PATCH] multipathd: Don't look at pointer to removed path

[PATCH] multipathd: Don't look at pointer to removed path

[Benjamin Marzinski]

In uev_update_path(), if the path wwid changes, we remove it and use
needs_reinit to re-add the path. Once we remove the path, pp is pointing to
freed memory, but we still read the value it pp to see if it's NULL before we
check needs_reinit. This doesn't cause any bugs, since we don't dereference it,
but it would still be cleaner if we stopped accessing it completely after we
freed it. This is easy to do, since we only ever set needs_reinit if pp is
non-NULL. So just check needs_reinit before we check if pp is NULL.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 9bce4d6b..792bee36 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1769,6 +1769,9 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
 	}
 out:
 	lock_cleanup_pop(vecs->lock);
+	/* pp->initialized must not be INIT_PARTIAL if needs_reinit is set */
+	if (needs_reinit)
+		return uev_add_path(uev, vecs, 1);
 	if (!pp) {
 		/* If the path is blacklisted, print a debug/non-default verbosity message. */
 		if (uev->udev) {
@@ -1787,9 +1790,6 @@ out:
 
 		condlog(0, "%s: spurious uevent, path not found", uev->kernel);
 	}
-	/* pp->initialized must not be INIT_PARTIAL if needs_reinit is set */
-	if (needs_reinit)
-		retval = uev_add_path(uev, vecs, 1);
 	return retval;
 }
 
-- 
2.56.0

Re: [PATCH] multipathd: Don't look at pointer to removed path

[Benjamin Marzinski]

On Thu, Oct 29, 2026 at 07:50:14PM -0400, Benjamin Marzinski wrote:
> In uev_update_path(), if the path wwid changes, we remove it and use
> needs_reinit to re-add the path. Once we remove the path, pp is pointing to
> freed memory, but we still read the value it pp to see if it's NULL before we
> check needs_reinit. This doesn't cause any bugs, since we don't dereference it,
> but it would still be cleaner if we stopped accessing it completely after we
> freed it. This is easy to do, since we only ever set needs_reinit if pp is
> non-NULL. So just check needs_reinit before we check if pp is NULL.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Since this is just a cosmetic change, I'll wait till Brian's
WWID verifying patch lands, and rebase on top of that.

-Ben

> ---
>  multipathd/main.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 9bce4d6b..792bee36 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1769,6 +1769,9 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
>  	}
>  out:
>  	lock_cleanup_pop(vecs->lock);
> +	/* pp->initialized must not be INIT_PARTIAL if needs_reinit is set */
> +	if (needs_reinit)
> +		return uev_add_path(uev, vecs, 1);
>  	if (!pp) {
>  		/* If the path is blacklisted, print a debug/non-default verbosity message. */
>  		if (uev->udev) {
> @@ -1787,9 +1790,6 @@ out:
>  
>  		condlog(0, "%s: spurious uevent, path not found", uev->kernel);
>  	}
> -	/* pp->initialized must not be INIT_PARTIAL if needs_reinit is set */
> -	if (needs_reinit)
> -		retval = uev_add_path(uev, vecs, 1);
>  	return retval;
>  }
>  
> -- 
> 2.56.0
>