On Wed, May 11, 2016 at 12:35:41PM +0200, Hannes Reinecke wrote:
> Ben Marzinski pointed out that filter_devnode() is used
> without any lock or configuration settings in uev_trigger(),
> and hence might be invalid when processing events during
> reconfiguration.
> So move it into the individual functions and handle it
> with the vector lock held.

ACK

-Ben

>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  multipathd/main.c | 45 +++++++++++++++++++++++++++------------------
>  1 file changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 58e8854..2c7486d 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -580,6 +580,11 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)
>       pthread_cleanup_push(cleanup_lock, &vecs->lock);
>       lock(vecs->lock);
>       pthread_testcancel();
> +     if (filter_devnode(conf->blist_devnode, conf->elist_devnode,
> +                        uev->kernel) > 0) {
> +             ret = 0;
> +             goto out_unlock;
> +     }
>       pp = find_path_by_dev(vecs->pathvec, uev->kernel);
>       if (pp) {
>               int r;
> @@ -637,6 +642,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)
>               free_path(pp);
>               ret = 1;
>       }
> +out_unlock:
>       lock_cleanup_pop(vecs->lock);
>       return ret;
>  }
> @@ -780,22 +786,23 @@ fail:
>  static int
>  uev_remove_path (struct uevent *uev, struct vectors * vecs)
>  {
> -     struct path *pp;
> -     int ret;
> +     struct path *pp = NULL;
> +     int ret = 0;
>
>       condlog(2, "%s: remove path (uevent)", uev->kernel);
>       pthread_cleanup_push(cleanup_lock, &vecs->lock);
>       lock(vecs->lock);
>       pthread_testcancel();
> -     pp = find_path_by_dev(vecs->pathvec, uev->kernel);
> -     if (pp)
> -             ret = ev_remove_path(pp, vecs);
> -     lock_cleanup_pop(vecs->lock);
> -     if (!pp) {
> -             /* Not an error; path might have been purged earlier */
> -             condlog(0, "%s: path already removed", uev->kernel);
> -             return 0;
> +     if (filter_devnode(conf->blist_devnode, conf->elist_devnode,
> +                        uev->kernel) == 0) {
> +             pp = find_path_by_dev(vecs->pathvec, uev->kernel);
> +             if (pp)
> +                     ret = ev_remove_path(pp, vecs);
> +             else
> +                     /* Not an error; path might have been purged earlier */
> +                     condlog(0, "%s: path already removed", uev->kernel);
>       }
> +     lock_cleanup_pop(vecs->lock);
>       return ret;
>  }
>
> @@ -905,7 +912,7 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
>       ro = uevent_get_disk_ro(uev);
>
>       if (ro >= 0) {
> -             struct path * pp;
> +             struct path * pp = NULL;
>               struct multipath *mpp = NULL;
>
>               condlog(2, "%s: update path write_protect to '%d' (uevent)",
> @@ -918,6 +925,10 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
>                * need to be at the same indentation level, hence
>                * this slightly convoluted codepath.
>                */
> +             if (filter_devnode(conf->blist_devnode, conf->elist_devnode,
> +                                uev->kernel) > 0) {
> +                     goto out_unlock;
> +             }
>               pp = find_path_by_dev(vecs->pathvec, uev->kernel);
>               if (pp) {
>                       if (pp->initialized == INIT_REQUESTED_UDEV) {
> @@ -937,11 +948,13 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
>                                       uev->kernel, mpp->alias, retval);
>                       }
>               }
> +     out_unlock:
>               lock_cleanup_pop(vecs->lock);
>               if (!pp) {
> -                     condlog(0, "%s: spurious uevent, path not found",
> -                             uev->kernel);
> -                     return 1;
> +                     if (retval)
> +                             condlog(0, "%s: spurious uevent, path not found",
> +                                     uev->kernel);
> +                     return retval;
>               }
>               if (retval == 2)
>                       return uev_add_path(uev, vecs);
> @@ -1059,10 +1072,6 @@ uev_trigger (struct uevent * uev, void * trigger_data)
>       /*
>        * path add/remove event
>        */
> -     if (filter_devnode(conf->blist_devnode, conf->elist_devnode,
> -                        uev->kernel) > 0)
> -             goto out;
> -
>       if (!strncmp(uev->action, "add", 3)) {
>               r = uev_add_path(uev, vecs);
>               goto out;
> --
> 2.6.6