Add SCSI patches to avoid blacklist false positives (rhbz 1299810)

This commit is contained in:
Josh Boyer 2016-01-19 08:38:39 -05:00
parent 4b48e2b551
commit b23324436e
3 changed files with 330 additions and 0 deletions

View File

@ -0,0 +1,140 @@
From 4abc12dd59bed74aa1730c2b3129d1750604d530 Mon Sep 17 00:00:00 2001
From: Alan Stern <stern@rowland.harvard.edu>
Date: Mon, 3 Aug 2015 11:57:29 -0400
Subject: [PATCH 2/2] 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.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Giulio Bernardi <ugilio@gmail.com>
Tested-by: Giulio Bernardi <ugilio@gmail.com>
Signed-off-by: James Bottomley <JBottomley@Odin.com>
---
drivers/scsi/scsi_devinfo.c | 69 +++++++++++++++++++++++----------------------
1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index 2f49a224462d..2c1160c7ec92 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -407,51 +407,52 @@ static struct scsi_dev_info_list *scsi_dev_info_list_find(const char *vendor,
struct scsi_dev_info_list *devinfo;
struct scsi_dev_info_list_table *devinfo_table =
scsi_devinfo_lookup_by_key(key);
+ size_t vmax, mmax;
+ const char *vskip, *mskip;
if (IS_ERR(devinfo_table))
return (struct scsi_dev_info_list *) devinfo_table;
+ /* Prepare for "compatible" matches */
+
+ /*
+ * XXX why skip leading spaces? If an odd INQUIRY
+ * value, that should have been part of the
+ * scsi_static_device_list[] entry, such as " FOO"
+ * rather than "FOO". Since this code is already
+ * here, and we don't know what device it is
+ * trying to work with, leave it as-is.
+ */
+ vmax = 8; /* max length of vendor */
+ vskip = vendor;
+ while (vmax > 0 && *vskip == ' ') {
+ vmax--;
+ vskip++;
+ }
+ /* Also skip trailing spaces */
+ while (vmax > 0 && vskip[vmax - 1] == ' ')
+ --vmax;
+
+ mmax = 16; /* max length of model */
+ mskip = model;
+ while (mmax > 0 && *mskip == ' ') {
+ mmax--;
+ mskip++;
+ }
+ while (mmax > 0 && mskip[mmax - 1] == ' ')
+ --mmax;
+
list_for_each_entry(devinfo, &devinfo_table->scsi_dev_info_list,
dev_info_list) {
if (devinfo->compatible) {
/*
* Behave like the older version of get_device_flags.
*/
- size_t max;
- /*
- * XXX why skip leading spaces? If an odd INQUIRY
- * value, that should have been part of the
- * scsi_static_device_list[] entry, such as " FOO"
- * rather than "FOO". Since this code is already
- * here, and we don't know what device it is
- * trying to work with, leave it as-is.
- */
- max = 8; /* max length of vendor */
- while ((max > 0) && *vendor == ' ') {
- max--;
- vendor++;
- }
- /*
- * XXX removing the following strlen() would be
- * good, using it means that for a an entry not in
- * the list, we scan every byte of every vendor
- * listed in scsi_static_device_list[], and never match
- * a single one (and still have to compare at
- * least the first byte of each vendor).
- */
- if (memcmp(devinfo->vendor, vendor,
- min(max, strlen(devinfo->vendor))))
+ if (memcmp(devinfo->vendor, vskip, vmax) ||
+ devinfo->vendor[vmax])
continue;
- /*
- * Skip spaces again.
- */
- max = 16; /* max length of model */
- while ((max > 0) && *model == ' ') {
- max--;
- model++;
- }
- if (memcmp(devinfo->model, model,
- min(max, strlen(devinfo->model))))
+ if (memcmp(devinfo->model, mskip, mmax) ||
+ devinfo->model[mmax])
continue;
return devinfo;
} else {
--
2.5.0

View File

@ -0,0 +1,183 @@
From 26d61e8347b27a981d647d3ea4ec8c7f462c1fcf Mon Sep 17 00:00:00 2001
From: Alan Stern <stern@rowland.harvard.edu>
Date: Mon, 3 Aug 2015 11:57:21 -0400
Subject: [PATCH 1/2] SCSI: refactor device-matching code in scsi_devinfo.c
In drivers/scsi/scsi_devinfo.c, the scsi_dev_info_list_del_keyed() and
scsi_get_device_flags_keyed() routines contain a large amount of
duplicate code for finding vendor/product matches in a
scsi_dev_info_list. This patch factors out the duplicate code and
puts it in a separate function, scsi_dev_info_list_find().
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Suggested-by: Giulio Bernardi <ugilio@gmail.com>
Signed-off-by: James Bottomley <JBottomley@Odin.com>
---
drivers/scsi/scsi_devinfo.c | 112 ++++++++++++++++----------------------------
1 file changed, 41 insertions(+), 71 deletions(-)
diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index 9f77d23239a2..2f49a224462d 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -390,25 +390,26 @@ int scsi_dev_info_list_add_keyed(int compatible, char *vendor, char *model,
EXPORT_SYMBOL(scsi_dev_info_list_add_keyed);
/**
- * scsi_dev_info_list_del_keyed - remove one dev_info list entry.
+ * scsi_dev_info_list_find - find a matching dev_info list entry.
* @vendor: vendor string
* @model: model (product) string
* @key: specify list to use
*
* Description:
- * Remove and destroy one dev_info entry for @vendor, @model
+ * Finds the first dev_info entry matching @vendor, @model
* in list specified by @key.
*
- * Returns: 0 OK, -error on failure.
+ * Returns: pointer to matching entry, or ERR_PTR on failure.
**/
-int scsi_dev_info_list_del_keyed(char *vendor, char *model, int key)
+static struct scsi_dev_info_list *scsi_dev_info_list_find(const char *vendor,
+ const char *model, int key)
{
- struct scsi_dev_info_list *devinfo, *found = NULL;
+ struct scsi_dev_info_list *devinfo;
struct scsi_dev_info_list_table *devinfo_table =
scsi_devinfo_lookup_by_key(key);
if (IS_ERR(devinfo_table))
- return PTR_ERR(devinfo_table);
+ return (struct scsi_dev_info_list *) devinfo_table;
list_for_each_entry(devinfo, &devinfo_table->scsi_dev_info_list,
dev_info_list) {
@@ -452,25 +453,42 @@ int scsi_dev_info_list_del_keyed(char *vendor, char *model, int key)
if (memcmp(devinfo->model, model,
min(max, strlen(devinfo->model))))
continue;
- found = devinfo;
+ return devinfo;
} else {
if (!memcmp(devinfo->vendor, vendor,
sizeof(devinfo->vendor)) &&
!memcmp(devinfo->model, model,
sizeof(devinfo->model)))
- found = devinfo;
+ return devinfo;
}
- if (found)
- break;
}
- if (found) {
- list_del(&found->dev_info_list);
- kfree(found);
- return 0;
- }
+ return ERR_PTR(-ENOENT);
+}
+
+/**
+ * scsi_dev_info_list_del_keyed - remove one dev_info list entry.
+ * @vendor: vendor string
+ * @model: model (product) string
+ * @key: specify list to use
+ *
+ * Description:
+ * Remove and destroy one dev_info entry for @vendor, @model
+ * in list specified by @key.
+ *
+ * Returns: 0 OK, -error on failure.
+ **/
+int scsi_dev_info_list_del_keyed(char *vendor, char *model, int key)
+{
+ struct scsi_dev_info_list *found;
- return -ENOENT;
+ found = scsi_dev_info_list_find(vendor, model, key);
+ if (IS_ERR(found))
+ return PTR_ERR(found);
+
+ list_del(&found->dev_info_list);
+ kfree(found);
+ return 0;
}
EXPORT_SYMBOL(scsi_dev_info_list_del_keyed);
@@ -565,64 +583,16 @@ int scsi_get_device_flags_keyed(struct scsi_device *sdev,
int key)
{
struct scsi_dev_info_list *devinfo;
- struct scsi_dev_info_list_table *devinfo_table;
+ int err;
- devinfo_table = scsi_devinfo_lookup_by_key(key);
+ devinfo = scsi_dev_info_list_find(vendor, model, key);
+ if (!IS_ERR(devinfo))
+ return devinfo->flags;
- if (IS_ERR(devinfo_table))
- return PTR_ERR(devinfo_table);
+ err = PTR_ERR(devinfo);
+ if (err != -ENOENT)
+ return err;
- list_for_each_entry(devinfo, &devinfo_table->scsi_dev_info_list,
- dev_info_list) {
- if (devinfo->compatible) {
- /*
- * Behave like the older version of get_device_flags.
- */
- size_t max;
- /*
- * XXX why skip leading spaces? If an odd INQUIRY
- * value, that should have been part of the
- * scsi_static_device_list[] entry, such as " FOO"
- * rather than "FOO". Since this code is already
- * here, and we don't know what device it is
- * trying to work with, leave it as-is.
- */
- max = 8; /* max length of vendor */
- while ((max > 0) && *vendor == ' ') {
- max--;
- vendor++;
- }
- /*
- * XXX removing the following strlen() would be
- * good, using it means that for a an entry not in
- * the list, we scan every byte of every vendor
- * listed in scsi_static_device_list[], and never match
- * a single one (and still have to compare at
- * least the first byte of each vendor).
- */
- if (memcmp(devinfo->vendor, vendor,
- min(max, strlen(devinfo->vendor))))
- continue;
- /*
- * Skip spaces again.
- */
- max = 16; /* max length of model */
- while ((max > 0) && *model == ' ') {
- max--;
- model++;
- }
- if (memcmp(devinfo->model, model,
- min(max, strlen(devinfo->model))))
- continue;
- return devinfo->flags;
- } else {
- if (!memcmp(devinfo->vendor, vendor,
- sizeof(devinfo->vendor)) &&
- !memcmp(devinfo->model, model,
- sizeof(devinfo->model)))
- return devinfo->flags;
- }
- }
/* nothing found, return nothing */
if (key != SCSI_DEVINFO_GLOBAL)
return 0;
--
2.5.0

View File

@ -682,6 +682,10 @@ Patch627: ideapad-laptop-Add-Lenovo-Yoga-700-to-no_hw_rfkill-d.patch
Patch628: i915-stable-backports.patch
#rhbz 1299810
Patch629: SCSI-refactor-device-matching-code-in-scsi_devinfo.c.patch
Patch630: SCSI-fix-bug-in-scsi_dev_info_list-matching.patch
# END OF PATCH DEFINITIONS
%endif
@ -2125,6 +2129,9 @@ fi
#
#
%changelog
* Tue Jan 19 2016 Josh Boyer <jwboyer@fedoraproject.org>
- Add SCSI patches to avoid blacklist false positives (rhbz 1299810)
* Mon Jan 18 2016 Josh Boyer <jwboyer@fedoraproject.org> - 4.3.3-302
- Backport stable fixed marked in upstream 4.4
- Fix rfkill issues on Yoga 700 (rhbz 1295272)