| Message ID | 1620775324-23984-5-git-send-email-bmarzins@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
| Series | Memory issues found by coverity | expand |
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. Apart from that, ACK. Martin -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
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
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");
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(-)