| Message ID | a822c1be-b984-b47b-7418-48d5cf069801@profitbricks.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
Dear maintainer, is this patch looks fine to you? Regards, Michael Wang On 10/12/2016 10:54 AM, Michael Wang wrote: > > Inside parse_cmd() the pthread_cleanup_pop() rely on '!r' as the > indicator of locked or not, while this will be overwritten if the > handler return failed, and the unlock will be missing. > > This will lead into the situation that all the following operation > will trying to hold a lock which will never be released. > > This patch using a separate flag to record the status of locking to > make sure the unlock and lock are in pairs. > > Signed-off-by: Michael Wang <yun.wang@profitbricks.com> > --- > multipathd/cli.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/multipathd/cli.c b/multipathd/cli.c > index e8a9384..50161be 100644 > --- a/multipathd/cli.c > +++ b/multipathd/cli.c > @@ -481,6 +481,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout ) > tmo.tv_sec = 0; > } > if (h->locked) { > + int locked = 0; > struct vectors * vecs = (struct vectors *)data; > > pthread_cleanup_push(cleanup_lock, &vecs->lock); > @@ -491,10 +492,11 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout ) > r = 0; > } > if (r == 0) { > + locked = 1; > pthread_testcancel(); > r = h->fn(cmdvec, reply, len, data); > } > - pthread_cleanup_pop(!r); > + pthread_cleanup_pop(locked); > } else > r = h->fn(cmdvec, reply, len, data); > free_keys(cmdvec); > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Merged. Thanks. On Mon, Oct 17, 2016 at 10:02 AM, Michael Wang <yun.wang@profitbricks.com> wrote: > Dear maintainer, is this patch looks fine to you? > > Regards, > Michael Wang > > On 10/12/2016 10:54 AM, Michael Wang wrote: > > > > Inside parse_cmd() the pthread_cleanup_pop() rely on '!r' as the > > indicator of locked or not, while this will be overwritten if the > > handler return failed, and the unlock will be missing. > > > > This will lead into the situation that all the following operation > > will trying to hold a lock which will never be released. > > > > This patch using a separate flag to record the status of locking to > > make sure the unlock and lock are in pairs. > > > > Signed-off-by: Michael Wang <yun.wang@profitbricks.com> > > --- > > multipathd/cli.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/multipathd/cli.c b/multipathd/cli.c > > index e8a9384..50161be 100644 > > --- a/multipathd/cli.c > > +++ b/multipathd/cli.c > > @@ -481,6 +481,7 @@ parse_cmd (char * cmd, char ** reply, int * len, > void * data, int timeout ) > > tmo.tv_sec = 0; > > } > > if (h->locked) { > > + int locked = 0; > > struct vectors * vecs = (struct vectors *)data; > > > > pthread_cleanup_push(cleanup_lock, &vecs->lock); > > @@ -491,10 +492,11 @@ parse_cmd (char * cmd, char ** reply, int * len, > void * data, int timeout ) > > r = 0; > > } > > if (r == 0) { > > + locked = 1; > > pthread_testcancel(); > > r = h->fn(cmdvec, reply, len, data); > > } > > - pthread_cleanup_pop(!r); > > + pthread_cleanup_pop(locked); > > } else > > r = h->fn(cmdvec, reply, len, data); > > free_keys(cmdvec); > > > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/multipathd/cli.c b/multipathd/cli.c index e8a9384..50161be 100644 --- a/multipathd/cli.c +++ b/multipathd/cli.c @@ -481,6 +481,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout ) tmo.tv_sec = 0; } if (h->locked) { + int locked = 0; struct vectors * vecs = (struct vectors *)data; pthread_cleanup_push(cleanup_lock, &vecs->lock); @@ -491,10 +492,11 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout ) r = 0; } if (r == 0) { + locked = 1; pthread_testcancel(); r = h->fn(cmdvec, reply, len, data); } - pthread_cleanup_pop(!r); + pthread_cleanup_pop(locked); } else r = h->fn(cmdvec, reply, len, data); free_keys(cmdvec);
Inside parse_cmd() the pthread_cleanup_pop() rely on '!r' as the indicator of locked or not, while this will be overwritten if the handler return failed, and the unlock will be missing. This will lead into the situation that all the following operation will trying to hold a lock which will never be released. This patch using a separate flag to record the status of locking to make sure the unlock and lock are in pairs. Signed-off-by: Michael Wang <yun.wang@profitbricks.com> --- multipathd/cli.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)