On Fri, Sep 25, 2026 at 11:40:07PM +0200, [email protected] wrote:
> From: Martin Wilck <[email protected]>
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 344da19..66af333 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -37,6 +37,7 @@
> #include "print.h"
> #include "strbuf.h"
> #include "pgpolicies.h"
> +#include "wwids.h"
>
> #define VPD_BUFLEN 4096
>
> @@ -45,6 +46,97 @@ struct vpd_vendor_page vpd_vendor_pages[VPD_VP_ARRAY_SIZE]
> = {
> [VPD_VP_HP3PAR] = { 0xc0, "hp3par" },
> };
>
> +/* from libdm/libdm-common.c; license: LGPL-2.1-only */
> +static bool _is_whitelisted_char(char c)
> +{
> + /*
> + * Actually, DM supports any character in a device name.
> + * This whitelist is just for proper integration with udev.
> + */
> + if ((c >= '0' && c <= '9') || (c >= 'A' && c <= 'Z') ||
> + (c >= 'a' && c <= 'z') || strchr("#+-.:=@_", c) != NULL)
> + return true;
> +
> + return false;
> +}
> +
> +size_t maybe_unmangle_wwid(char new_wwid[WWID_SIZE], const char *wwid)
> +{
> + char buf[WWID_SIZE], *dst;
> + const char *src;
> + bool mangled = false, invalid = false;
> + size_t rc, len;
> +
> + if (!wwid) {
> + *new_wwid = '\0';
> + return 0;
> + }
> +
> + /* this might be > WWID_SIZE - 1 */
> + len = strlen(wwid);
> + /*
> + * A correctly mangled string doesn't contain invalid characters.
> + * In particular, if the original string contained `\xXY`, it will
> + * itself be mangled to '\x5cxXY`.
> + */
> + /* clang-format off */
> + for (src = wwid, dst = buf;
> + dst - buf < WWID_SIZE - 1 && *src;
> + src++, dst++) {
> + /*clang-format on */
> + if (_is_whitelisted_char(*src)) {
> + *dst = *src;
> + continue;
> + }
> + if (*src == '\\') {
> + char hex[3], c;
> +
> + /*
> + * udev mangling always outputs 2-digit hex numbers
> + * 1-digit would be unprintable, anyway
> + */
> + if (src + 4 > wwid + len || *(src + 1) != 'x' ||
> + sscanf(src + 2, "%2[A-Fa-f0-9]", hex) == 0 ||
> + strlen(hex) != 2) {
> + condlog(3, "%s: wwid \"%s\" contains invalid
> mangle sequence \"%s\"",
> + __func__, wwid, src);
> + invalid = true;
> + break;
> + }
> + mangled = true;
> + src += 1 + strlen(hex);
> + c = strtol(hex, NULL, 16);
> + /* Allow non-ascii unicode, but avoid non-printable
> characters */
> + *dst = (c & 0x80) || isprint(c) ? c : '_';
> + } else {
> + condlog(3, "%s: wwid \"%s\" contains illegal character
> '%02x'",
> + __func__, wwid, (unsigned char)*src);
> + invalid = true;
> + break;
> + }
> + }
> +
> + /*
> + * 1. If we found an invalid character, it can't be mangled by udev,
> + * use original WWID.
> + * 2. If we didn't found a `\xHH`, it isn't mangled, use original.
> + * 3. If the unmangled WWID is known, use it.
> + * 4. If the original WWID is known, use it.
> + * 5. If in doubt, use the unmangled WWID.
> + */
> + if (!invalid && mangled &&
This check should use "buf" not "new_wwid".
> + (!check_wwids_file(new_wwid, 0) || check_wwids_file(wwid, 0))) {
> + *dst = '\0';
> + rc = strlcpy(new_wwid, buf, WWID_SIZE);
> + condlog(3, "%s: unmangled WWID \"%s\" -> \"%s\"", __func__,
> + wwid, new_wwid);
> + } else {
> + rc = strlcpy(new_wwid, wwid, WWID_SIZE);
> + condlog(4, "%s: kept WWID \"%s\"", __func__, new_wwid);
> + }
Nitpick: I think these conditionals would be easier to check against
rules as:
if (invalid || !mangled || check_wwids_file(wwid, 0) == 0) {
/* Keep original wwid */
} else {
/* Use unmangled wwid */
}
> + return rc;
> +}
> +
> int
> alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
> const char *wwid, int flag, struct path **pp_ptr)
> @@ -65,8 +157,7 @@ alloc_path_with_pathinfo (struct config *conf, struct
> udev_device *udevice,
> if (!pp)
> return PATHINFO_FAILED;
>
> - if (wwid)
> - strlcpy(pp->wwid, wwid, sizeof(pp->wwid));
> + maybe_unmangle_wwid(pp->wwid, wwid);
>
> if (safe_sprintf(pp->dev, "%s", devname)) {
> condlog(0, "pp->dev too small");
> @@ -2163,8 +2254,8 @@ get_udev_uid(struct path * pp, const char
> *uid_attribute, struct udev_device *ud
> value = udev_device_get_property_value(udev, uid_attribute);
> 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);
> + if (value && *value) {
> + len = maybe_unmangle_wwid(pp->wwid, value);
> if (len >= WWID_SIZE) {
> len = fix_broken_nvme_wwid(pp, value, WWID_SIZE);
> if (len > 0)
> @@ -2224,7 +2315,7 @@ static ssize_t dasd_get_uid(struct path *pp)
> if (p)
> *p = '\0';
>
> - return strlcpy(pp->wwid, value, WWID_SIZE);
> + return maybe_unmangle_wwid(pp->wwid, value);
We're reading this from sysfs, so I don't think this one needs to be
unmangled.
> }
>
> static ssize_t uid_fallback(struct path *pp, int path_state,
> @@ -2254,6 +2345,8 @@ static ssize_t uid_fallback(struct path *pp, int
> path_state,
> sizeof(value));
> if (!sysfs_attr_value_ok(len, sizeof(value)))
> return -1;
> +
> + /* WWID read from sysfs, unmangling is not necessary */
I'm pretty sure that unmangling is unnecessary in all of the methods in
uid_fallback, so this comment is confusing.
-Ben
