From b23324436e9d7a043a512a6ff5f0d7a85e868681 Mon Sep 17 00:00:00 2001 From: Josh Boyer Date: Jan 19 2016 13:38:39 +0000 Subject: Add SCSI patches to avoid blacklist false positives (rhbz 1299810) --- diff --git a/SCSI-fix-bug-in-scsi_dev_info_list-matching.patch b/SCSI-fix-bug-in-scsi_dev_info_list-matching.patch new file mode 100644 index 0000000..d79ccf9 --- /dev/null +++ b/SCSI-fix-bug-in-scsi_dev_info_list-matching.patch @@ -0,0 +1,140 @@ +From 4abc12dd59bed74aa1730c2b3129d1750604d530 Mon Sep 17 00:00:00 2001 +From: Alan Stern +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 +Reported-by: Giulio Bernardi +Tested-by: Giulio Bernardi +Signed-off-by: James Bottomley +--- + 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 + diff --git a/SCSI-refactor-device-matching-code-in-scsi_devinfo.c.patch b/SCSI-refactor-device-matching-code-in-scsi_devinfo.c.patch new file mode 100644 index 0000000..e87baad --- /dev/null +++ b/SCSI-refactor-device-matching-code-in-scsi_devinfo.c.patch @@ -0,0 +1,183 @@ +From 26d61e8347b27a981d647d3ea4ec8c7f462c1fcf Mon Sep 17 00:00:00 2001 +From: Alan Stern +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 +Suggested-by: Giulio Bernardi +Signed-off-by: James Bottomley +--- + 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 + diff --git a/kernel.spec b/kernel.spec index a375a85..22c5d8a 100644 --- a/kernel.spec +++ b/kernel.spec @@ -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 +- Add SCSI patches to avoid blacklist false positives (rhbz 1299810) + * Mon Jan 18 2016 Josh Boyer - 4.3.3-302 - Backport stable fixed marked in upstream 4.4 - Fix rfkill issues on Yoga 700 (rhbz 1295272)