| Message ID | 20180625151020.31927-1-mwilck@suse.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
Christophe, without this patch, current upstream multipath _crashes_ on path removal. Please apply it. If reviewers disagree with this fix, I can post updates later. Martin On Mon, 2018-06-25 at 17:10 +0200, Martin Wilck wrote: > In my previous patch f0462f0c8338, I overlooked that during path > removal, > the path that mpp->hwe references may be removed and and thus mpp- > >hwe > may become stale. Fix it. > > Fixes: f0462f0c8338 "libmultipath: use vector for for pp->hwe and mp- > >hwe" > Signed-off-by: Martin Wilck <mwilck@suse.com> > --- > multipathd/main.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/multipathd/main.c b/multipathd/main.c > index 72f06b56..cc493c18 100644 > --- a/multipathd/main.c > +++ b/multipathd/main.c > @@ -1076,6 +1076,14 @@ ev_remove_path (struct path *pp, struct > vectors * vecs, int need_do_map) > mpp->alias); > goto fail; > } > + > + /* > + * Make sure mpp->hwe doesn't point to freed memory > + * We call extract_hwe_from_path() below to restore > mpp->hwe > + */ > + if (mpp->hwe == pp->hwe) > + mpp->hwe = NULL; > + > if ((i = find_slot(mpp->paths, (void *)pp)) != -1) > vector_del_slot(mpp->paths, i); > > @@ -1109,6 +1117,9 @@ ev_remove_path (struct path *pp, struct vectors > * vecs, int need_do_map) > */ > } > > + if (mpp->hwe == NULL) > + extract_hwe_from_path(mpp); > + > if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { > condlog(0, "%s: failed to setup map for" > " removal of path %s", mpp->alias, > pp->dev);
Right, done. The leftover from your previous v3 patchset is merged too. Thanks, Christophe. On Mon, Jul 2, 2018 at 11:33 AM Martin Wilck <mwilck@suse.com> wrote: > Christophe, > > without this patch, current upstream multipath _crashes_ on path > removal. Please apply it. If reviewers disagree with this fix, I can > post updates later. > > Martin > > On Mon, 2018-06-25 at 17:10 +0200, Martin Wilck wrote: > > In my previous patch f0462f0c8338, I overlooked that during path > > removal, > > the path that mpp->hwe references may be removed and and thus mpp- > > >hwe > > may become stale. Fix it. > > > > Fixes: f0462f0c8338 "libmultipath: use vector for for pp->hwe and mp- > > >hwe" > > Signed-off-by: Martin Wilck <mwilck@suse.com> > > --- > > multipathd/main.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/multipathd/main.c b/multipathd/main.c > > index 72f06b56..cc493c18 100644 > > --- a/multipathd/main.c > > +++ b/multipathd/main.c > > @@ -1076,6 +1076,14 @@ ev_remove_path (struct path *pp, struct > > vectors * vecs, int need_do_map) > > mpp->alias); > > goto fail; > > } > > + > > + /* > > + * Make sure mpp->hwe doesn't point to freed memory > > + * We call extract_hwe_from_path() below to restore > > mpp->hwe > > + */ > > + if (mpp->hwe == pp->hwe) > > + mpp->hwe = NULL; > > + > > if ((i = find_slot(mpp->paths, (void *)pp)) != -1) > > vector_del_slot(mpp->paths, i); > > > > @@ -1109,6 +1117,9 @@ ev_remove_path (struct path *pp, struct vectors > > * vecs, int need_do_map) > > */ > > } > > > > + if (mpp->hwe == NULL) > > + extract_hwe_from_path(mpp); > > + > > if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { > > condlog(0, "%s: failed to setup map for" > > " removal of path %s", mpp->alias, > > pp->dev); > > -- > Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107 > SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg) > > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Hello Christophe, On Mon, 2018-07-02 at 12:05 +0200, Christophe Varoqui wrote: > Right, done. > The leftover from your previous v3 patchset is merged too. > Thanks, > Christophe. I think in that mail from July 2nd, you referred to the following patch set which had been acked by Ben on June 25th (https://www.redhat.com/archives/dm-devel/2018-June/msg00367.html): * [PATCH v3 00/10] libmpathpersist fixes and some more * [PATCH v3 01/10] libmpathpersist: remove duplicate test in * [PATCH v3 02/10] libmpathpersist: fix typo in * [PATCH v3 03/10] libmpathpersist: fix stack overflow in * [PATCH v3 04/10] libmultipath: add (get|put)_unaligned_be64 * [PATCH v3 05/10] multipath-tools/tests: add tests for * [PATCH v3 06/10] libmpathpersist: fix byte swapping for big endian * [PATCH v3 07/10] (lib)mpathpersist: use O_RDONLY file descriptors * [PATCH v3 08/10] libmultipath: fix gcc 8.1 "truncated output" * [PATCH v3 09/10] multipathd: fix buffer size in cli_getprkey() * [PATCH v3 10/10] libmultipath: avoid error messages from RDAC check I just double-checked, and it seems that out of these, only patch 9 and 10 got applied to your tree. Did you have an issue with the others, or was it just an oversight? Regards Martin PS: I'll send a review or Ben's late big series very soon. > > On Mon, Jul 2, 2018 at 11:33 AM Martin Wilck <mwilck@suse.com> wrote: > > Christophe, > > > > without this patch, current upstream multipath _crashes_ on path > > removal. Please apply it. If reviewers disagree with this fix, I > > can > > post updates later. > > > > Martin > > > > On Mon, 2018-06-25 at 17:10 +0200, Martin Wilck wrote: > > > In my previous patch f0462f0c8338, I overlooked that during path > > > removal, > > > the path that mpp->hwe references may be removed and and thus > > mpp- > > > >hwe > > > may become stale. Fix it. > > > > > > Fixes: f0462f0c8338 "libmultipath: use vector for for pp->hwe and > > mp- > > > >hwe" > > > Signed-off-by: Martin Wilck <mwilck@suse.com> > > > --- > > > multipathd/main.c | 11 +++++++++++ > > > 1 file changed, 11 insertions(+) > > > > > > diff --git a/multipathd/main.c b/multipathd/main.c > > > index 72f06b56..cc493c18 100644 > > > --- a/multipathd/main.c > > > +++ b/multipathd/main.c > > > @@ -1076,6 +1076,14 @@ ev_remove_path (struct path *pp, struct > > > vectors * vecs, int need_do_map) > > > mpp->alias); > > > goto fail; > > > } > > > + > > > + /* > > > + * Make sure mpp->hwe doesn't point to freed memory > > > + * We call extract_hwe_from_path() below to restore > > > mpp->hwe > > > + */ > > > + if (mpp->hwe == pp->hwe) > > > + mpp->hwe = NULL; > > > + > > > if ((i = find_slot(mpp->paths, (void *)pp)) != -1) > > > vector_del_slot(mpp->paths, i); > > > > > > @@ -1109,6 +1117,9 @@ ev_remove_path (struct path *pp, struct > > vectors > > > * vecs, int need_do_map) > > > */ > > > } > > > > > > + if (mpp->hwe == NULL) > > > + extract_hwe_from_path(mpp); > > > + > > > if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { > > > condlog(0, "%s: failed to setup map for" > > > " removal of path %s", mpp->alias, > > > pp->dev); > > > > -- > > dm-devel mailing list > > dm-devel@redhat.com > > https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/multipathd/main.c b/multipathd/main.c index 72f06b56..cc493c18 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -1076,6 +1076,14 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) mpp->alias); goto fail; } + + /* + * Make sure mpp->hwe doesn't point to freed memory + * We call extract_hwe_from_path() below to restore mpp->hwe + */ + if (mpp->hwe == pp->hwe) + mpp->hwe = NULL; + if ((i = find_slot(mpp->paths, (void *)pp)) != -1) vector_del_slot(mpp->paths, i); @@ -1109,6 +1117,9 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map) */ } + if (mpp->hwe == NULL) + extract_hwe_from_path(mpp); + if (setup_map(mpp, params, PARAMS_SIZE, vecs)) { condlog(0, "%s: failed to setup map for" " removal of path %s", mpp->alias, pp->dev);
In my previous patch f0462f0c8338, I overlooked that during path removal, the path that mpp->hwe references may be removed and and thus mpp->hwe may become stale. Fix it. Fixes: f0462f0c8338 "libmultipath: use vector for for pp->hwe and mp->hwe" Signed-off-by: Martin Wilck <mwilck@suse.com> --- multipathd/main.c | 11 +++++++++++ 1 file changed, 11 insertions(+)