On Fri, 2017-08-04 at 11:40 +0200, Hannes Reinecke wrote:
> When checking the model and vendor string we need to use the
> minimum value of either string, otherwise we'll miss out on
> wildcard matches.
> Without this patch certain Hitachi arrays will not be presenting
> VPD pages correctly.
>
> Fixes: 5e7ff2c ("SCSI: fix new bug in scsi_dev_info_list string matching")
> Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
> ---
> drivers/scsi/scsi_devinfo.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
> index 28fea83..f8a302f 100644
> --- a/drivers/scsi/scsi_devinfo.c
> +++ b/drivers/scsi/scsi_devinfo.c
> @@ -456,11 +456,13 @@ static struct scsi_dev_info_list *scsi_dev_info_list_find(const char *vendor,
> /*
> * Behave like the older version of get_device_flags.
> */
> - if (memcmp(devinfo->vendor, vskip, vmax) ||
> + if (memcmp(devinfo->vendor, vskip,
> + min(vmax, strlen(devinfo->vendor))) ||
> (vmax < sizeof(devinfo->vendor) &&
> devinfo->vendor[vmax]))
> continue;
> - if (memcmp(devinfo->model, mskip, mmax) ||
> + if (memcmp(devinfo->model, mskip,
> + min(mmax, strlen(devinfo->model))) ||
> (mmax < sizeof(devinfo->model) &&
> devinfo->model[mmax]))
> continue;
Hello Hannes,
Will this reintroduce the bug mentioned in commit b704f70ce200? The description
of that commit is as follows:
=========================================================================
SCSI: fix bug in scsi_dev_info_list matching
The "compatible" matching algorithm used for looking up old-style
blacklist entries in a scsi_dev_info_list is buggy. The core of the
algorithm looks like this:
if (memcmp(devinfo->vendor, vendor,
min(max, strlen(devinfo->vendor))))
/* not a match */
where max is the length of the device's vendor string after leading
spaces have been removed but trailing spaces have not. Because of the
min() computation, either entry could be a proper substring of the
other and the code would still think that they match.
In the case originally reported, the device's vendor and product
strings were "Inateck " and " ". These matched against
the following entry in the global device list:
{"", "Scanner", "1.80", BLIST_NOLUN}
because "" is a substring of "Inateck " and "" (the result of removing
leading spaces from the device's product string) is a substring of
"Scanner". The mistaken match prevented the system from scanning and
finding the device's second Logical Unit.
This patch fixes the problem by making two changes. First, the code
for leading-space removal is hoisted out of the loop. (This means it
will sometimes run unnecessarily, but since a large percentage of all
lookups involve the "compatible" entries in global device list, this
should be an overall improvement.) Second and more importantly, the
patch removes trailing spaces and adds a check to verify that the two
resulting strings are exactly the same length. This prevents matches
where one entry is a proper substring of the other.
=========================================================================
Thanks,
Bart.
