<div dir="ltr">Merged</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 11, 2016 at 7:18 PM, Benjamin Marzinski <span dir="ltr"><<a href="mailto:bmarzins@redhat.com" target="_blank">bmarzins@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, May 11, 2016 at 12:35:41PM +0200, Hannes Reinecke wrote:<br>
> Ben Marzinski pointed out that filter_devnode() is used<br>
> without any lock or configuration settings in uev_trigger(),<br>
> and hence might be invalid when processing events during<br>
> reconfiguration.<br>
> So move it into the individual functions and handle it<br>
> with the vector lock held.<br>
<br>
</span>ACK<br>
<br>
-Ben<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> Signed-off-by: Hannes Reinecke <<a href="mailto:hare@suse.com">hare@suse.com</a>><br>
> ---<br>
>  multipathd/main.c | 45 +++++++++++++++++++++++++++------------------<br>
>  1 file changed, 27 insertions(+), 18 deletions(-)<br>
><br>
> diff --git a/multipathd/main.c b/multipathd/main.c<br>
> index 58e8854..2c7486d 100644<br>
> --- a/multipathd/main.c<br>
> +++ b/multipathd/main.c<br>
> @@ -580,6 +580,11 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)<br>
>       pthread_cleanup_push(cleanup_lock, &vecs->lock);<br>
>       lock(vecs->lock);<br>
>       pthread_testcancel();<br>
> +     if (filter_devnode(conf->blist_devnode, conf->elist_devnode,<br>
> +                        uev->kernel) > 0) {<br>
> +             ret = 0;<br>
> +             goto out_unlock;<br>
> +     }<br>
>       pp = find_path_by_dev(vecs->pathvec, uev->kernel);<br>
>       if (pp) {<br>
>               int r;<br>
> @@ -637,6 +642,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)<br>
>               free_path(pp);<br>
>               ret = 1;<br>
>       }<br>
> +out_unlock:<br>
>       lock_cleanup_pop(vecs->lock);<br>
>       return ret;<br>
>  }<br>
> @@ -780,22 +786,23 @@ fail:<br>
>  static int<br>
>  uev_remove_path (struct uevent *uev, struct vectors * vecs)<br>
>  {<br>
> -     struct path *pp;<br>
> -     int ret;<br>
> +     struct path *pp = NULL;<br>
> +     int ret = 0;<br>
><br>
>       condlog(2, "%s: remove path (uevent)", uev->kernel);<br>
>       pthread_cleanup_push(cleanup_lock, &vecs->lock);<br>
>       lock(vecs->lock);<br>
>       pthread_testcancel();<br>
> -     pp = find_path_by_dev(vecs->pathvec, uev->kernel);<br>
> -     if (pp)<br>
> -             ret = ev_remove_path(pp, vecs);<br>
> -     lock_cleanup_pop(vecs->lock);<br>
> -     if (!pp) {<br>
> -             /* Not an error; path might have been purged earlier */<br>
> -             condlog(0, "%s: path already removed", uev->kernel);<br>
> -             return 0;<br>
> +     if (filter_devnode(conf->blist_devnode, conf->elist_devnode,<br>
> +                        uev->kernel) == 0) {<br>
> +             pp = find_path_by_dev(vecs->pathvec, uev->kernel);<br>
> +             if (pp)<br>
> +                     ret = ev_remove_path(pp, vecs);<br>
> +             else<br>
> +                     /* Not an error; path might have been purged earlier */<br>
> +                     condlog(0, "%s: path already removed", uev->kernel);<br>
>       }<br>
> +     lock_cleanup_pop(vecs->lock);<br>
>       return ret;<br>
>  }<br>
><br>
> @@ -905,7 +912,7 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)<br>
>       ro = uevent_get_disk_ro(uev);<br>
><br>
>       if (ro >= 0) {<br>
> -             struct path * pp;<br>
> +             struct path * pp = NULL;<br>
>               struct multipath *mpp = NULL;<br>
><br>
>               condlog(2, "%s: update path write_protect to '%d' (uevent)",<br>
> @@ -918,6 +925,10 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)<br>
>                * need to be at the same indentation level, hence<br>
>                * this slightly convoluted codepath.<br>
>                */<br>
> +             if (filter_devnode(conf->blist_devnode, conf->elist_devnode,<br>
> +                                uev->kernel) > 0) {<br>
> +                     goto out_unlock;<br>
> +             }<br>
>               pp = find_path_by_dev(vecs->pathvec, uev->kernel);<br>
>               if (pp) {<br>
>                       if (pp->initialized == INIT_REQUESTED_UDEV) {<br>
> @@ -937,11 +948,13 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)<br>
>                                       uev->kernel, mpp->alias, retval);<br>
>                       }<br>
>               }<br>
> +     out_unlock:<br>
>               lock_cleanup_pop(vecs->lock);<br>
>               if (!pp) {<br>
> -                     condlog(0, "%s: spurious uevent, path not found",<br>
> -                             uev->kernel);<br>
> -                     return 1;<br>
> +                     if (retval)<br>
> +                             condlog(0, "%s: spurious uevent, path not found",<br>
> +                                     uev->kernel);<br>
> +                     return retval;<br>
>               }<br>
>               if (retval == 2)<br>
>                       return uev_add_path(uev, vecs);<br>
> @@ -1059,10 +1072,6 @@ uev_trigger (struct uevent * uev, void * trigger_data)<br>
>       /*<br>
>        * path add/remove event<br>
>        */<br>
> -     if (filter_devnode(conf->blist_devnode, conf->elist_devnode,<br>
> -                        uev->kernel) > 0)<br>
> -             goto out;<br>
> -<br>
>       if (!strncmp(uev->action, "add", 3)) {<br>
>               r = uev_add_path(uev, vecs);<br>
>               goto out;<br>
> --<br>
> 2.6.6<br>
</div></div></blockquote></div><br></div>