diff mbox

[2/3] access vecs memory outside of locking range in check_path()

Message ID 1476759069-9832-2-git-send-email-tang.junhui@zte.com.cn (mailing list archive)
State Not Applicable, archived
Delegated to: Mike Snitzer
Headers show

Commit Message

tang.junhui@zte.com.cn Oct. 18, 2016, 2:51 a.m. UTC 
From: "tang.junhui" <tang.junhui@zte.com.cn>

there are vecs->mpvec memory accesses outside of locking range in
check_path(), the judgments is not necessary since the they has
existed in vector_foreach_slot(), so delete them.

Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
---
 multipathd/main.c | 45 ++++++++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

Comments

Benjamin Marzinski Oct. 18, 2016, 4:22 p.m. UTC | #1 
On Tue, Oct 18, 2016 at 10:51:08AM +0800, tang.junhui@zte.com.cn wrote:
> From: "tang.junhui" <tang.junhui@zte.com.cn>
> 
> there are vecs->mpvec memory accesses outside of locking range in
> check_path(), the judgments is not necessary since the they has
> existed in vector_foreach_slot(), so delete them.

ACK

-Ben


> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> ---
>  multipathd/main.c | 45 ++++++++++++++++++++++-----------------------
>  1 file changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index b6eb696..e369a79 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1804,30 +1804,29 @@ checkerloop (void *ap)
>  			condlog(4, "timeout waiting for DAEMON_IDLE");
>  			continue;
>  		}
> -		if (vecs->pathvec) {
> -			pthread_cleanup_push(cleanup_lock, &vecs->lock);
> -			lock(&vecs->lock);
> -			pthread_testcancel();
> -			vector_foreach_slot (vecs->pathvec, pp, i) {
> -				rc = check_path(vecs, pp, ticks);
> -				if (rc < 0) {
> -					vector_del_slot(vecs->pathvec, i);
> -					free_path(pp);
> -					i--;
> -				} else
> -					num_paths += rc;
> -			}
> -			lock_cleanup_pop(vecs->lock);
> -		}
> -		if (vecs->mpvec) {
> -			pthread_cleanup_push(cleanup_lock, &vecs->lock);
> -			lock(&vecs->lock);
> -			pthread_testcancel();
> -			defered_failback_tick(vecs->mpvec);
> -			retry_count_tick(vecs->mpvec);
> -			missing_uev_wait_tick(vecs);
> -			lock_cleanup_pop(vecs->lock);
> +
> +		pthread_cleanup_push(cleanup_lock, &vecs->lock);
> +		lock(&vecs->lock);
> +		pthread_testcancel();
> +		vector_foreach_slot (vecs->pathvec, pp, i) {
> +			rc = check_path(vecs, pp, ticks);
> +			if (rc < 0) {
> +				vector_del_slot(vecs->pathvec, i);
> +				free_path(pp);
> +				i--;
> +			} else
> +				num_paths += rc;
>  		}
> +		lock_cleanup_pop(vecs->lock);
> +
> +		pthread_cleanup_push(cleanup_lock, &vecs->lock);
> +		lock(&vecs->lock);
> +		pthread_testcancel();
> +		defered_failback_tick(vecs->mpvec);
> +		retry_count_tick(vecs->mpvec);
> +		missing_uev_wait_tick(vecs);
> +		lock_cleanup_pop(vecs->lock);
> +
>  		if (count)
>  			count--;
>  		else {
> -- 
> 2.8.1.windows.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Christophe Varoqui Oct. 19, 2016, 6 a.m. UTC | #2 
Merged.

On Tue, Oct 18, 2016 at 6:22 PM, Benjamin Marzinski <bmarzins@redhat.com>
wrote:

> On Tue, Oct 18, 2016 at 10:51:08AM +0800, tang.junhui@zte.com.cn wrote:
> > From: "tang.junhui" <tang.junhui@zte.com.cn>
> >
> > there are vecs->mpvec memory accesses outside of locking range in
> > check_path(), the judgments is not necessary since the they has
> > existed in vector_foreach_slot(), so delete them.
>
> ACK
>
> -Ben
>
>
> > Signed-off-by: tang.junhui <tang.junhui@zte.com.cn>
> > ---
> >  multipathd/main.c | 45 ++++++++++++++++++++++-----------------------
> >  1 file changed, 22 insertions(+), 23 deletions(-)
> >
> > diff --git a/multipathd/main.c b/multipathd/main.c
> > index b6eb696..e369a79 100644
> > --- a/multipathd/main.c
> > +++ b/multipathd/main.c
> > @@ -1804,30 +1804,29 @@ checkerloop (void *ap)
> >                       condlog(4, "timeout waiting for DAEMON_IDLE");
> >                       continue;
> >               }
> > -             if (vecs->pathvec) {
> > -                     pthread_cleanup_push(cleanup_lock, &vecs->lock);
> > -                     lock(&vecs->lock);
> > -                     pthread_testcancel();
> > -                     vector_foreach_slot (vecs->pathvec, pp, i) {
> > -                             rc = check_path(vecs, pp, ticks);
> > -                             if (rc < 0) {
> > -                                     vector_del_slot(vecs->pathvec, i);
> > -                                     free_path(pp);
> > -                                     i--;
> > -                             } else
> > -                                     num_paths += rc;
> > -                     }
> > -                     lock_cleanup_pop(vecs->lock);
> > -             }
> > -             if (vecs->mpvec) {
> > -                     pthread_cleanup_push(cleanup_lock, &vecs->lock);
> > -                     lock(&vecs->lock);
> > -                     pthread_testcancel();
> > -                     defered_failback_tick(vecs->mpvec);
> > -                     retry_count_tick(vecs->mpvec);
> > -                     missing_uev_wait_tick(vecs);
> > -                     lock_cleanup_pop(vecs->lock);
> > +
> > +             pthread_cleanup_push(cleanup_lock, &vecs->lock);
> > +             lock(&vecs->lock);
> > +             pthread_testcancel();
> > +             vector_foreach_slot (vecs->pathvec, pp, i) {
> > +                     rc = check_path(vecs, pp, ticks);
> > +                     if (rc < 0) {
> > +                             vector_del_slot(vecs->pathvec, i);
> > +                             free_path(pp);
> > +                             i--;
> > +                     } else
> > +                             num_paths += rc;
> >               }
> > +             lock_cleanup_pop(vecs->lock);
> > +
> > +             pthread_cleanup_push(cleanup_lock, &vecs->lock);
> > +             lock(&vecs->lock);
> > +             pthread_testcancel();
> > +             defered_failback_tick(vecs->mpvec);
> > +             retry_count_tick(vecs->mpvec);
> > +             missing_uev_wait_tick(vecs);
> > +             lock_cleanup_pop(vecs->lock);
> > +
> >               if (count)
> >                       count--;
> >               else {
> > --
> > 2.8.1.windows.1
>
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
diff mbox

Patch

diff --git a/multipathd/main.c b/multipathd/main.c
index b6eb696..e369a79 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1804,30 +1804,29 @@  checkerloop (void *ap)
 			condlog(4, "timeout waiting for DAEMON_IDLE");
 			continue;
 		}
-		if (vecs->pathvec) {
-			pthread_cleanup_push(cleanup_lock, &vecs->lock);
-			lock(&vecs->lock);
-			pthread_testcancel();
-			vector_foreach_slot (vecs->pathvec, pp, i) {
-				rc = check_path(vecs, pp, ticks);
-				if (rc < 0) {
-					vector_del_slot(vecs->pathvec, i);
-					free_path(pp);
-					i--;
-				} else
-					num_paths += rc;
-			}
-			lock_cleanup_pop(vecs->lock);
-		}
-		if (vecs->mpvec) {
-			pthread_cleanup_push(cleanup_lock, &vecs->lock);
-			lock(&vecs->lock);
-			pthread_testcancel();
-			defered_failback_tick(vecs->mpvec);
-			retry_count_tick(vecs->mpvec);
-			missing_uev_wait_tick(vecs);
-			lock_cleanup_pop(vecs->lock);
+
+		pthread_cleanup_push(cleanup_lock, &vecs->lock);
+		lock(&vecs->lock);
+		pthread_testcancel();
+		vector_foreach_slot (vecs->pathvec, pp, i) {
+			rc = check_path(vecs, pp, ticks);
+			if (rc < 0) {
+				vector_del_slot(vecs->pathvec, i);
+				free_path(pp);
+				i--;
+			} else
+				num_paths += rc;
 		}
+		lock_cleanup_pop(vecs->lock);
+
+		pthread_cleanup_push(cleanup_lock, &vecs->lock);
+		lock(&vecs->lock);
+		pthread_testcancel();
+		defered_failback_tick(vecs->mpvec);
+		retry_count_tick(vecs->mpvec);
+		missing_uev_wait_tick(vecs);
+		lock_cleanup_pop(vecs->lock);
+
 		if (count)
 			count--;
 		else {