Re: [PATCH] libmultipath: limit paths that can get wwid from environment

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2023-02-09 at 11:28 -0600, Benjamin Marzinski wrote:
> Currently, whenever getting the uid_attribute from the udev database
> fails, multipath will try to get it from the environment variables.
> This
> normally isn't a problem, since either multipath -u is getting called
> from a uevent and the environment will have the correct value in that
> variable, or something else is being run and that variable won't be
> set.
> However, when find_multipaths is configured to "smart", this causes
> problems. For maybe devices, multipath needs to get the WWIDs of all
> the
> other block devices, to see if they match the maybe device wwid.  If
> one
> of those devices doesn't have uid_attribute set in its udev database,
> multipath will fall back to checking the environment for it, and it
> will
> find that variable set to the WWID of the maybe device that this
> uevent
> is for.  This means that all devices with no WWID will end up
> appearing
> to have the same WWID as the maybe device, causing multipath to
> incorrectly claim it.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>

I have to say I don't quite understand why we read from the environment
at all if the libudev call fails. This was coded before I joined the
project, so perhaps you can clarify it. Why do we expect the
environment to provide the correct value if libudev cannot?

If we need to keep this fallback, I wonder if we need another field in
"struct path" for it. For example, we could read MAJOR and MINOR from
the environment and use uid_attribute only if the result matches the
path's devt.

Regards
Martin






> ---
>  libmultipath/discovery.c | 2 +-
>  libmultipath/structs.h   | 1 +
>  multipath/main.c         | 2 ++
>  3 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 67ac0e6d..3a5ba173 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -2093,7 +2093,7 @@ get_udev_uid(struct path * pp, const char
> *uid_attribute, struct udev_device *ud
>         const char *value;
>  
>         value = udev_device_get_property_value(udev, uid_attribute);
> -       if (!value || strlen(value) == 0)
> +       if ((!value || strlen(value) == 0) && pp->can_use_env_uid)
>                 value = getenv(uid_attribute);
>         if (value && strlen(value)) {
>                 len = strlcpy(pp->wwid, value, WWID_SIZE);
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index e2294323..a1208751 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -368,6 +368,7 @@ struct path {
>         unsigned int dev_loss;
>         int eh_deadline;
>         bool is_checked;
> +       bool can_use_env_uid;
>         /* configlet pointers */
>         vector hwe;
>         struct gen_path generic_path;
> diff --git a/multipath/main.c b/multipath/main.c
> index b9f360b4..90f940f1 100644
> --- a/multipath/main.c
> +++ b/multipath/main.c
> @@ -607,6 +607,8 @@ check_path_valid(const char *name, struct config
> *conf, bool is_uevent)
>         pp = alloc_path();
>         if (!pp)
>                 return RTVL_FAIL;
> +       if (is_uevent)
> +               pp->can_use_env_uid = true;
>  
>         r = is_path_valid(name, conf, pp, is_uevent);
>         if (r <= PATH_IS_ERROR || r >= PATH_MAX_VALID_RESULT)

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





[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux