All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Marzinski <bmarzins@redhat.com>
To: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: device-mapper development <dm-devel@lists.linux.dev>,
	Martin Wilck <Martin.Wilck@suse.com>
Subject: Re: [PATCH] multipathd: Don't look at pointer to removed path
Date: Fri, 30 Oct 2026 13:46:31 -0400	[thread overview]
Message-ID: <auTX97jTl602SYIu@redhat.com> (raw)
In-Reply-To: <20261029235014.818202-1-bmarzins@redhat.com>

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
> 


      reply	other threads:[~2026-10-30 17:46 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29 23:50 [PATCH] multipathd: Don't look at pointer to removed path Benjamin Marzinski
2026-10-30 17:46 ` Benjamin Marzinski [this message]

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=auTX97jTl602SYIu@redhat.com \
    --to=bmarzins@redhat.com \
    --cc=Martin.Wilck@suse.com \
    --cc=christophe.varoqui@opensvc.com \
    --cc=dm-devel@lists.linux.dev \
    /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.