ACK, with one small nit below. -Ben
On Fri, Jul 14, 2017 at 01:32:09PM +0200, Martin Wilck wrote: > Fix for NVME wwids looking like this: > nvme.0000-3163653363666438366239656630386200-4c696e75780000000000000000000000000000000000000000000000000000000000000000000000-00000002 > which are encountered in some combinations of Linux NVME host and target, > where the 2nd field represents the serial number (SN) and the 3rd the > model number (MN). > > The device mapper allows map names only up to 128 characters. > Strip the "00" sequences at the end of the serial and model field, > they are hex-encoded 0-bytes which are forbidden by the NVME spec > anyway. > > Signed-off-by: Martin Wilck <[email protected]> > --- > libmultipath/discovery.c | 79 > ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 79 insertions(+) > > diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c > index 9951af84..f46b9b17 100644 > --- a/libmultipath/discovery.c > +++ b/libmultipath/discovery.c > @@ -1598,6 +1598,82 @@ get_prio (struct path * pp) > return 0; > } > > +/* > + * Mangle string of length *len starting at start > + * by removing character sequence "00" (hex for a 0 byte), > + * starting at end, backwards. > + * Changes the value of *len if characters were removed. > + * Returns a pointer to the position where "end" was moved to. > + */ > +static char > +*skip_zeroes_backward(char* start, int *len, char *end) > +{ > + char *p = end; > + > + while (p >= start + 2 && *(p - 1) == '0' && *(p - 2) == '0') > + p -= 2; > + > + if (p == end) > + return p; > + > + memmove(p, end, start + *len + 1 - end); > + *len -= end - p; > + > + return p; > +} > + > +/* > + * Fix for NVME wwids looking like this: > + * > nvme.0000-3163653363666438366239656630386200-4c696e75780000000000000000000000000000000000000000000000000000000000000000000000-00000002 > + * which are encountered in some combinations of Linux NVME host and target. > + * The '00' are hex-encoded 0-bytes which are forbidden in the serial (SN) > + * and model (MN) fields. Discard them. > + * If a WWID of the above type is found, sets pp->wwid and returns a value > > 0. > + * Otherwise, returns 0. > + */ > +static int > +fix_broken_nvme_wwid(struct path *pp, const char *value, int size) > +{ > + static const char _nvme[] = "nvme."; > + int len, i; > + char mangled[256]; > + char *p; > + > + len = strlen(value); > + if (len >= sizeof(mangled) || len + 1 < size) > + return 0; Don't we check (len + 1 < size) before calling this? It seems odd to return that we can't do anything when in reality, we simply don't need to do anything. -Ben > + > + /* Check that value starts with "nvme.%04x-" */ > + if (memcmp(value, _nvme, sizeof(_nvme) - 1) || value[9] != '-') > + return 0; > + for (i = 5; i < 9; i++) > + if (!isxdigit(value[i])) > + return 0; > + > + memcpy(mangled, value, len + 1); > + > + /* search end of "model" part and strip trailing '00' */ > + p = memrchr(mangled, '-', len); > + if (p == NULL) > + return 0; > + > + p = skip_zeroes_backward(mangled, &len, p); > + > + /* search end of "serial" part */ > + p = memrchr(mangled, '-', p - mangled); > + if (p == NULL || memrchr(mangled, '-', p - mangled) != mangled + 9) > + /* We expect exactly 3 '-' in the value */ > + return 0; > + > + p = skip_zeroes_backward(mangled, &len, p); > + if (len >= size) > + return 0; > + > + memcpy(pp->wwid, mangled, len + 1); > + condlog(2, "%s: over-long WWID shortened to %s", pp->dev, pp->wwid); > + return len; > +} > + > static int > get_udev_uid(struct path * pp, char *uid_attribute, struct udev_device *udev) > { > @@ -1609,6 +1685,9 @@ get_udev_uid(struct path * pp, char *uid_attribute, > struct udev_device *udev) > value = getenv(uid_attribute); > if (value && strlen(value)) { > if (strlen(value) + 1 > WWID_SIZE) { > + len = fix_broken_nvme_wwid(pp, value, WWID_SIZE); > + if (len > 0) > + return len; > condlog(0, "%s: wwid overflow", pp->dev); > len = WWID_SIZE; > } else { > -- > 2.13.2 -- dm-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/dm-devel
