Re: [PATCH 12/21] libmultipath: free_multipath: fix FREE_PATHS case

[Benjamin Marzinski <bmarzins@redhat.com>]

On Wed, Dec 17, 2025 at 10:21:04PM +0100, Martin Wilck wrote:
> free_multipath was passing FREE_PATHS to both free_pgvec() and
> free_pathvec(), which was wrong because a path could be referenced
> in both structures. This can't happen any more now, as free_pgvec()
> has lost the ability to free paths.
> 
> In general, free_multipath can't make any assumptions about whether
> a given path is referenced in mpp->paths, mpp->pg, or both. Therefore
> look first for paths that are referenced in mpp->pg but not in mpp->paths,
> clean them, and finally loop over mpp->paths.

Like I said in 07/21, I don't think we should free paths here. The paths
linked from the multipath device are also in the pathvec. We should
always be freeing them from there, so we don't have a pathvec with freed
paths.

-Ben
 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/structs.c | 44 +++++++++++++++++++++++++++++-------------
>  1 file changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/libmultipath/structs.c b/libmultipath/structs.c
> index 1ee29f6..29b62e4 100644
> --- a/libmultipath/structs.c
> +++ b/libmultipath/structs.c
> @@ -298,8 +298,12 @@ void free_multipath_attributes(struct multipath *mpp)
>  }
>  
>  void
> -free_multipath (struct multipath * mpp, enum free_path_mode free_paths)
> +free_multipath (struct multipath *mpp, enum free_path_mode free_paths)
>  {
> +	struct pathgroup *pgp;
> +	struct path *pp;
> +	int i, j;
> +
>  	if (!mpp)
>  		return;
>  
> @@ -310,22 +314,36 @@ free_multipath (struct multipath * mpp, enum free_path_mode free_paths)
>  		mpp->alias = NULL;
>  	}
>  
> -	if (free_paths == KEEP_PATHS && mpp->pg) {
> -		struct pathgroup *pgp;
> -		struct path *pp;
> -		int i, j;
> -
> -		/*
> -		 * Make sure paths carry no reference to this mpp any more
> -		 */
> -		vector_foreach_slot(mpp->pg, pgp, i) {
> -			vector_foreach_slot(pgp->paths, pp, j)
> -				if (pp->mpp == mpp)
> +	/*
> +	 * If free_paths == FREE_PATHS, free all paths.
> +	 * Otherwise, make sure no references to this mpp remain.
> +	 *
> +	 * First loop over mpp->pg and clean the paths that might
> +	 * be missing in mpp->paths. Then clean the rest
> +	 * by looping over mpp->paths.
> +	 */
> +	vector_foreach_slot(mpp->pg, pgp, i) {
> +		vector_foreach_slot(pgp->paths, pp, j) {
> +			if (find_slot(mpp->paths, pp) == -1) {
> +				if (free_paths == FREE_PATHS)
> +					free_path(pp);
> +				else
>  					pp->mpp = NULL;
> +			}
>  		}
>  	}
>  
> -	free_pathvec(mpp->paths, free_paths);
> +	vector_foreach_slot(mpp->paths, pp, i) {
> +		if (free_paths == FREE_PATHS)
> +			free_path(pp);
> +		else
> +			pp->mpp = NULL;
> +	}
> +
> +	/*
> +	 * We've freed paths above already, use KEEP_PATHS here
> +	 */
> +	free_pathvec(mpp->paths, KEEP_PATHS);
>  	free_pgvec(mpp->pg);
>  	if (mpp->hwe) {
>  		vector_free(mpp->hwe);
> -- 
> 2.52.0