On Mon, 2017-04-24 at 17:39 -0500, Benjamin Marzinski wrote:
> The do_foreach_partmaps code could incorrectly list a device as a
> partition of another device, because of two issues.  First, the check
> to
> compare the dm UUID of two devices would allow two partition devices
> to
> match, or a partition device to match with itself, instead of only
> having a partition device match with the multipath device that it
> belongs to.  Second, the code to check if a multipath device's
> major:minor appeared in a partition device's table only used strstr
> to check of the existance of the major:minor string. This meant that
> any device with a minor number that started with the same digits
> would
> match. for instance, checking for "253:10" would also match
> "253:102".

Good catch! I missed that in my kpartx series although I was trying to
be paranoid ;-)

> -/*
> - * returns:
> - *    0 : if both uuids end with same suffix which starts with
> UUID_PREFIX
> - *    1 : otherwise
> - */
> -int
> -dm_compare_uuid(const char* mapname1, const char* mapname2)
> +static int
> +is_mpath_part(const char *part_name, const char *map_name)
>  {
> -     char *p1, *p2;
> -     char uuid1[WWID_SIZE], uuid2[WWID_SIZE];
> +     char *p;
> +     char part_uuid[WWID_SIZE], map_uuid[WWID_SIZE];
>  
> -     if (dm_get_prefixed_uuid(mapname1, uuid1))
> -             return 1;
> +     if (dm_get_prefixed_uuid(part_name, part_uuid))
> +             return 0;
>  
> -     if (dm_get_prefixed_uuid(mapname2, uuid2))
> -             return 1;
> +     if (dm_get_prefixed_uuid(map_name, map_uuid))
> +             return 0;
>  
> -     p1 = strstr(uuid1, UUID_PREFIX);
> -     p2 = strstr(uuid2, UUID_PREFIX);
> -     if (p1 && p2 && !strcmp(p1, p2))
> +     if (strncmp(part_uuid, "part", 4) != 0)
>               return 0;
>  
> -     return 1;
> +     p = strstr(part_uuid, UUID_PREFIX);
> +     if (p && !strcmp(p, map_uuid))
> +             return 1;
> +
> +     return 0;
>  }

While we've got this far, we might actually implement proper parsing of
 the UUID instead of "strstr" which would also match something like
"partially_recovered-mpath...".

ACK nonetheless, further checks can be added on top of this.

Martin

-- 
Dr. Martin Wilck <[email protected]>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to