On Thu, Dec 17, 2020 at 11:04:44AM +0100, [email protected] wrote: > From: Martin Wilck <[email protected]> > > There were two leaks in check_path_valid(): if path status was > successfully determined before calling store_pathvec(), free_path() > wasn't called. Also, if an error exit occured, neither cleanup > function was called. > > This patch fixes both, at the cost of using "static" for the pp and > pathvec variables. >
I just noticed that your commit message doesn't totally match what the patch does anymore. But the code looks fine. Feel free to change the message when pushing to upstream-queue. Reviewed-by: Benjamin Marzinski <[email protected]> > Signed-off-by: Martin Wilck <[email protected]> > --- > multipath/main.c | 30 ++++++++++++++++++++---------- > 1 file changed, 20 insertions(+), 10 deletions(-) > > diff --git a/multipath/main.c b/multipath/main.c > index 1949a1c..043d8fa 100644 > --- a/multipath/main.c > +++ b/multipath/main.c > @@ -93,7 +93,7 @@ void rcu_register_thread_memb(void) {} > void rcu_unregister_thread_memb(void) {} > > static int > -filter_pathvec (vector pathvec, char * refwwid) > +filter_pathvec (vector pathvec, const char *refwwid) > { > int i; > struct path * pp; > @@ -594,8 +594,9 @@ static int > check_path_valid(const char *name, struct config *conf, bool is_uevent) > { > int fd, r = PATH_IS_ERROR; > - struct path *pp = NULL; > + struct path *pp; > vector pathvec = NULL; > + const char *wwid; > > pp = alloc_path(); > if (!pp) > @@ -664,14 +665,19 @@ check_path_valid(const char *name, struct config *conf, > bool is_uevent) > > if (store_path(pathvec, pp) != 0) { > free_path(pp); > + pp = NULL; > goto fail; > + } else { > + /* make sure path isn't freed twice */ > + wwid = pp->wwid; > + pp = NULL; > } > > /* For find_multipaths = SMART, if there is more than one path > * matching the refwwid, then the path is valid */ > if (path_discovery(pathvec, DI_SYSFS | DI_WWID) < 0) > goto fail; > - filter_pathvec(pathvec, pp->wwid); > + filter_pathvec(pathvec, wwid); > if (VECTOR_SIZE(pathvec) > 1) > r = PATH_IS_VALID; > else > @@ -679,21 +685,25 @@ check_path_valid(const char *name, struct config *conf, > bool is_uevent) > > out: > r = print_cmd_valid(r, pathvec, conf); > - free_pathvec(pathvec, FREE_PATHS); > /* > * multipath -u must exit with status 0, otherwise udev won't > * import its output. > */ > if (!is_uevent && r == PATH_IS_NOT_VALID) > - return RTVL_FAIL; > - return RTVL_OK; > + r = RTVL_FAIL; > + else > + r = RTVL_OK; > + goto cleanup; > > fail: > - if (pathvec) > - free_pathvec(pathvec, FREE_PATHS); > - else > + r = RTVL_FAIL; > + > +cleanup: > + if (pp != NULL) > free_path(pp); > - return RTVL_FAIL; > + if (pathvec != NULL) > + free_pathvec(pathvec, FREE_PATHS); > + return r; > } > > static int > -- > 2.29.0 -- dm-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/dm-devel
