On Fri, Dec 19, 2025 at 03:40:55PM +0100, Martin Wilck wrote:
> Paths belong to the pathvec and should be freed from it.
> Use cleanup_multipath() instead of cleanup_multipath_and_paths(),
> and free the paths via the pathvec instead.
> 

I don't know if this matters, but until you stop free_multipaths() from
clearing pp->mpp in patch 15/26, this will temporarily cause UAF errors,
since you free the paths from the pathvec first, and then attempt to
reset their pp->mpp value in free_multipaths() later.

You could avoid the temporary problem by moving the pathvec definition
before the mpp definition, since cleanup functions are run in reverse
order of their declaration.

But after applying patch 15/26, this is fine, so
Reviewed-by: Benjamin Marzinski <[email protected]>

> Suggested-by: Benjamin Marzinski <[email protected]>
> Signed-off-by: Martin Wilck <[email protected]>
> ---
>  multipath/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/multipath/main.c b/multipath/main.c
> index 4b8d7dd..d28628b 100644
> --- a/multipath/main.c
> +++ b/multipath/main.c
> @@ -214,12 +214,12 @@ static int check_usable_paths(struct config *conf,
>                             const char *devpath, enum devtypes dev_type)
>  {
>       struct udev_device __attribute__((cleanup(cleanup_udev_device))) *ud = 
> NULL;
> -     struct multipath __attribute__((cleanup(cleanup_multipath_and_paths))) 
> *mpp = NULL;
> +     struct multipath __attribute__((cleanup(cleanup_multipath))) *mpp = 
> NULL;
>       struct pathgroup *pg;
>       struct path *pp;
>       char __attribute__((cleanup(cleanup_charp))) *params = NULL;
>       char __attribute__((cleanup(cleanup_charp))) *status = NULL;
> -     vector __attribute((cleanup(cleanup_vector))) pathvec = NULL;
> +     vector __attribute((cleanup(cleanup_pathvec_and_free_paths))) pathvec = 
> NULL;
>       char uuid[DM_UUID_LEN];
>       dev_t devt;
>       int r = 1, i, j;
> -- 
> 2.52.0


Reply via email to