Re: [dm-devel] [PATCH] multipath-tools: release lock on handler failure

[Michael Wang]

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 
> ---
>  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, >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

[dm-devel] [PATCH] multipath-tools: release lock on handler failure

[Michael Wang]

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 
---
 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, >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);
-- 
2.5.0

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel