Re: [dm-devel] [PATCH 4/5] multipath: free vectors in configure

From: Benjamin Marzinski <bmarzins@redhat.com>
To: Martin Wilck <martin.wilck@suse.com>
Cc: "dm-devel@redhat.com" <dm-devel@redhat.com>
Subject: Re: [dm-devel] [PATCH 4/5] multipath: free vectors in configure
Date: Wed, 12 May 2021 14:53:44 -0500	[thread overview]
Message-ID: <20210512195344.GF25887@octiron.msp.redhat.com> (raw)
In-Reply-To: <f186788dcb161fd44859a0c8908c2eb83d3bee0d.camel@suse.com>

On Wed, May 12, 2021 at 12:36:42PM +0000, Martin Wilck wrote:
> On Tue, 2021-05-11 at 18:22 -0500, Benjamin Marzinski wrote:
> > configure() can retry multiple times, each time reallocing a maps and
> > paths vector, and leaking the previous ones. Fix this by always
> > freeing
> > the vectors before configure() exits. Found by coverity.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  multipath/main.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/multipath/main.c b/multipath/main.c
> > index ef89c7cf..25c5dbfd 100644
> > --- a/multipath/main.c
> > +++ b/multipath/main.c
> > @@ -466,7 +466,6 @@ configure (struct config *conf, enum mpath_cmds
> > cmd,
> >          */
> >         curmp = vector_alloc();
> >         pathvec = vector_alloc();
> > -       atexit(cleanup_vecs);
> >  
> >         if (!curmp || !pathvec) {
> >                 condlog(0, "can not allocate memory");
> > @@ -578,6 +577,11 @@ out:
> >         if (refwwid)
> >                 FREE(refwwid);
> >  
> > +       free_multipathvec(curmp, KEEP_PATHS);
> > +       vecs.mpvec = NULL;
> > +       free_pathvec(pathvec, FREE_PATHS);
> > +       vecs.pathvec = NULL;
> > +
> >         return r;
> >  }
> >  
> > @@ -1053,6 +1057,7 @@ main (int argc, char *argv[])
> >                 r = dm_flush_maps(1, retries) ? RTVL_FAIL : RTVL_OK;
> >                 goto out;
> >         }
> > +       atexit(cleanup_vecs);
> >         while ((r = configure(conf, cmd, dev_type, dev)) ==
> > RTVL_RETRY)
> >                 condlog(3, "restart multipath configuration
> > process");
> >  
> 
> 
> Nit: I'd rather move this atexit() call towards the beginning of
> main(), after the other atexit() calls.

Sure.

-Ben

> 
> Apart from that, ACK.
> 
> Martin

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel