This patch (as810b) copies a minimum of 36 bytes of INQUIRY data,
even if the device claims that not all of them are valid. Often badly
behaved devices put plausible data in the Vendor, Product, and Revision
strings but set the Additional Length byte to a small value. Using
potentially valid data is certainly better than allocating a short
buffer and then reading beyond the end of it, which is what we do now.
Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
---
On Fri, 27 Oct 2006, Patrick Mansfield wrote:
> The comment is confusing, as it implies the device will modify data past
> the indicated length, but well behaved devices should not do that, and
> with your patch should point to zero filled data.
> But definitely better to use possibly valid data for broken devices or
> NUL for well behaved devices rather than garbage values.
This version of the patch has an improved comment. And it's superior in
another respect too: It doesn't alter the value of sdev->inquiry_len.
Alan Stern
Index: usb-2.6/drivers/scsi/scsi_scan.c
===================================================================
--- usb-2.6.orig/drivers/scsi/scsi_scan.c
+++ usb-2.6/drivers/scsi/scsi_scan.c
@@ -621,6 +621,8 @@ static int scsi_probe_lun(struct scsi_de
static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
int *bflags)
{
+ int copy_len;
+
/*
* XXX do not save the inquiry, since it can change underneath us,
* save just vendor/model/rev.
@@ -631,12 +633,23 @@ static int scsi_add_lun(struct scsi_devi
* scanning run at their own risk, or supply a user level program
* that can correctly scan.
*/
- sdev->inquiry = kmalloc(sdev->inquiry_len, GFP_ATOMIC);
+
+ /*
+ * Copy at least 36 bytes of INQUIRY data, so that we don't
+ * dereference unallocated memory when accessing the Vendor,
+ * Product, and Revision strings. Badly behaved devices may set
+ * the INQUIRY Additional Length byte to a small value, indicating
+ * these strings are invalid, but often they contain plausible data
+ * nonetheless. It doesn't matter if the device sent < 36 bytes
+ * total, since scsi_probe_lun() initializes inq_result with 0s.
+ */
+ copy_len = max((int) sdev->inquiry_len, 36);
+ sdev->inquiry = kmalloc(copy_len, GFP_ATOMIC);
if (sdev->inquiry == NULL) {
return SCSI_SCAN_NO_RESPONSE;
}
- memcpy(sdev->inquiry, inq_result, sdev->inquiry_len);
+ memcpy(sdev->inquiry, inq_result, copy_len);
sdev->vendor = (char *) (sdev->inquiry + 8);
sdev->model = (char *) (sdev->inquiry + 16);
sdev->rev = (char *) (sdev->inquiry + 32);
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
