On Wed, Dec 16, 2020 at 07:17:01PM +0100, mwilck@xxxxxxxx wrote: > From: Martin Wilck <mwilck@xxxxxxxx> > > 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. > > Signed-off-by: Martin Wilck <mwilck@xxxxxxxx> > --- > multipath/main.c | 29 +++++++++++++++++++---------- > 1 file changed, 19 insertions(+), 10 deletions(-) > > diff --git a/multipath/main.c b/multipath/main.c > index 1949a1c..056e29a 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) > @@ -665,13 +666,17 @@ check_path_valid(const char *name, struct config *conf, bool is_uevent) > if (store_path(pathvec, pp) != 0) { This will double-free the path, once here and again in cleanup. > free_path(pp); > 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 +684,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) > + r = RTVL_FAIL; > + > +cleanup: > + if (pp != NULL) shouldn't this be free_path(pp) -Ben > + free(pp); > + if (pathvec != NULL) > free_pathvec(pathvec, FREE_PATHS); > - else > - free_path(pp); > - return RTVL_FAIL; > + return r; > } > > static int > -- > 2.29.0 -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel