scsi: don't store LUN bits in CDB[1] for USB mass-storage devices

The SCSI specification requires that the second Command Data Byte should contain the LUN value in its high-order bits if the recipient device reports SCSI level 2 or below. Nevertheless, some USB mass-storage devices use those bits for other purposes in vendor-specific commands. Currently Linux has no way to send such commands, because the SCSI stack always overwrites the LUN bits. Testing shows that Windows 7 and XP do not store the LUN bits in the CDB when sending commands to a USB device. This doesn't matter if the device uses the Bulk-Only or UAS transports (which virtually all modern USB mass-storage devices do), as these have a separate mechanism for sending the LUN value. Therefore this patch introduces a flag in the Scsi_Host structure to inform the SCSI midlayer that a transport does not require the LUN bits to be stored in the CDB, and it makes usb-storage set this flag for all devices using the Bulk-Only transport. (UAS is handled by a separate driver, but it doesn't really matter because no SCSI-2 or lower device is at all likely to use UAS.) The patch also cleans up the code responsible for storing the LUN value by adding a bitflag to the scsi_device structure. The test for whether to stick the LUN value in the CDB can be made when the device is probed, and stored for future use rather than being made over and over in the fast path. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Reported-by: Tiziano Bacocco <tiziano.bacocco@gmail.com> Acked-by: Martin K. Petersen <martin.petersen@oracle.com> Acked-by: James Bottomley <James.Bottomley@HansenPartnership.com> Signed-off-by: Christoph Hellwig <hch@lst.de>

scsi: don't store LUN bits in CDB[1] for USB mass-storage devices
The SCSI specification requires that the second Command Data Byte should contain the LUN value in its high-order bits if the recipient device reports SCSI level 2 or below. Nevertheless, some USB mass-storage devices use those bits for other purposes in vendor-specific commands. Currently Linux has no way to send such commands, because the SCSI stack always overwrites the LUN bits. Testing shows that Windows 7 and XP do not store the LUN bits in the CDB when sending commands to a USB device. This doesn't matter if the device uses the Bulk-Only or UAS transports (which virtually all modern USB mass-storage devices do), as these have a separate mechanism for sending the LUN value. Therefore this patch introduces a flag in the Scsi_Host structure to inform the SCSI midlayer that a transport does not require the LUN bits to be stored in the CDB, and it makes usb-storage set this flag for all devices using the Bulk-Only transport. (UAS is handled by a separate driver, but it doesn't really matter because no SCSI-2 or lower device is at all likely to use UAS.) The patch also cleans up the code responsible for storing the LUN value by adding a bitflag to the scsi_device structure. The test for whether to stick the LUN value in the CDB can be made when the device is probed, and stored for future use rather than being made over and over in the fast path. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Reported-by: Tiziano Bacocco <tiziano.bacocco@gmail.com> Acked-by: Martin K. Petersen <martin.petersen@oracle.com> Acked-by: James Bottomley <James.Bottomley@HansenPartnership.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
Alan Stern · Christoph Hellwig
1 parent 64bdcbc449
Showing 6 changed files with 36 additions and 6 deletions Side-by-side Diff
drivers/scsi/scsi.c
drivers/scsi/scsi_scan.c
drivers/scsi/scsi_sysfs.c
drivers/usb/storage/usb.c
include/scsi/scsi_device.h
include/scsi/scsi_host.h
@@ -670,14 +670,10 @@
 		return SCSI_MLQUEUE_DEVICE_BUSY;
 	}
  
-	/*
-	 * If SCSI-2 or lower, store the LUN value in cmnd.
-	 */
-	if (cmd->device->scsi_level <= SCSI_2 &&
-	    cmd->device->scsi_level != SCSI_UNKNOWN) {
+	/* Store the LUN value in cmnd, if needed. */
+	if (cmd->device->lun_in_cdb)
 		cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) |
 			       (cmd->device->lun << 5 & 0xe0);
-	}
  
 	scsi_log_send(cmd);
  
@@ -736,6 +736,16 @@
 		sdev->scsi_level++;
 	sdev->sdev_target->scsi_level = sdev->scsi_level;
  
+	/*
+	 * If SCSI-2 or lower, and if the transport requires it,
+	 * store the LUN value in CDB[1].
+	 */
+	sdev->lun_in_cdb = 0;
+	if (sdev->scsi_level <= SCSI_2 &&
+	    sdev->scsi_level != SCSI_UNKNOWN &&
+	    !sdev->host->no_scsi2_lun_in_cdb)
+		sdev->lun_in_cdb = 1;
+
 	return 0;
 }
  
@@ -1263,7 +1263,19 @@
 	sdev->sdev_dev.class = &sdev_class;
 	dev_set_name(&sdev->sdev_dev, "%d:%d:%d:%llu",
 		     sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
+	/*
+	 * Get a default scsi_level from the target (derived from sibling
+	 * devices).  This is the best we can do for guessing how to set
+	 * sdev->lun_in_cdb for the initial INQUIRY command.  For LUN 0 the
+	 * setting doesn't matter, because all the bits are zero anyway.
+	 * But it does matter for higher LUNs.
+	 */
 	sdev->scsi_level = starget->scsi_level;
+	if (sdev->scsi_level <= SCSI_2 &&
+			sdev->scsi_level != SCSI_UNKNOWN &&
+			!shost->no_scsi2_lun_in_cdb)
+		sdev->lun_in_cdb = 1;
+
 	transport_setup_device(&sdev->sdev_gendev);
 	spin_lock_irqsave(shost->host_lock, flags);
 	list_add_tail(&sdev->same_target_siblings, &starget->devices);
@@ -983,6 +983,14 @@
 	if (!(us->fflags & US_FL_SCM_MULT_TARG))
 		us_to_host(us)->max_id = 1;
  
+	/*
+	 * Like Windows, we won't store the LUN bits in CDB[1] for SCSI-2
+	 * devices using the Bulk-Only transport (even though this violates
+	 * the SCSI spec).
+	 */
+	if (us->transport == usb_stor_Bulk_transport)
+		us_to_host(us)->no_scsi2_lun_in_cdb = 1;
+
 	/* Find the endpoints and calculate pipe values */
 	result = get_pipes(us);
 	if (result)
@@ -174,6 +174,7 @@
 	unsigned wce_default_on:1;	/* Cache is ON by default */
 	unsigned no_dif:1;	/* T10 PI (DIF) should be disabled */
 	unsigned broken_fua:1;		/* Don't set FUA bit */
+	unsigned lun_in_cdb:1;		/* Store LUN bits in CDB[1] */
  
 	atomic_t disk_events_disable_depth; /* disable depth for disk events */
  
@@ -693,6 +693,9 @@
 	 */
 	struct workqueue_struct *tmf_work_q;
  
+	/* The transport requires the LUN bits NOT to be stored in CDB[1] */
+	unsigned no_scsi2_lun_in_cdb:1;
+
 	/*
 	 * Value host_blocked counts down from
 	 */
...	...	@@ -670,14 +670,10 @@
670	670	return SCSI_MLQUEUE_DEVICE_BUSY;
671	671	}
672	672
673		- /*
674		- * If SCSI-2 or lower, store the LUN value in cmnd.
675		- */
676		- if (cmd->device->scsi_level <= SCSI_2 &&
677		- cmd->device->scsi_level != SCSI_UNKNOWN) {
	673	+ /* Store the LUN value in cmnd, if needed. */
	674	+ if (cmd->device->lun_in_cdb)
678	675	cmd->cmnd[1] = (cmd->cmnd[1] & 0x1f) \|
679	676	(cmd->device->lun << 5 & 0xe0);
680		- }
681	677
682	678	scsi_log_send(cmd);
683	679
...	...	@@ -736,6 +736,16 @@
736	736	sdev->scsi_level++;
737	737	sdev->sdev_target->scsi_level = sdev->scsi_level;
738	738
	739	+ /*
	740	+ * If SCSI-2 or lower, and if the transport requires it,
	741	+ * store the LUN value in CDB[1].
	742	+ */
	743	+ sdev->lun_in_cdb = 0;
	744	+ if (sdev->scsi_level <= SCSI_2 &&
	745	+ sdev->scsi_level != SCSI_UNKNOWN &&
	746	+ !sdev->host->no_scsi2_lun_in_cdb)
	747	+ sdev->lun_in_cdb = 1;
	748	+
739	749	return 0;
740	750	}
741	751
...	...	@@ -1263,7 +1263,19 @@
1263	1263	sdev->sdev_dev.class = &sdev_class;
1264	1264	dev_set_name(&sdev->sdev_dev, "%d:%d:%d:%llu",
1265	1265	sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
	1266	+ /*
	1267	+ * Get a default scsi_level from the target (derived from sibling
	1268	+ * devices). This is the best we can do for guessing how to set
	1269	+ * sdev->lun_in_cdb for the initial INQUIRY command. For LUN 0 the
	1270	+ * setting doesn't matter, because all the bits are zero anyway.
	1271	+ * But it does matter for higher LUNs.
	1272	+ */
1266	1273	sdev->scsi_level = starget->scsi_level;
	1274	+ if (sdev->scsi_level <= SCSI_2 &&
	1275	+ sdev->scsi_level != SCSI_UNKNOWN &&
	1276	+ !shost->no_scsi2_lun_in_cdb)
	1277	+ sdev->lun_in_cdb = 1;
	1278	+
1267	1279	transport_setup_device(&sdev->sdev_gendev);
1268	1280	spin_lock_irqsave(shost->host_lock, flags);
1269	1281	list_add_tail(&sdev->same_target_siblings, &starget->devices);
...	...	@@ -983,6 +983,14 @@
983	983	if (!(us->fflags & US_FL_SCM_MULT_TARG))
984	984	us_to_host(us)->max_id = 1;
985	985
	986	+ /*
	987	+ * Like Windows, we won't store the LUN bits in CDB[1] for SCSI-2
	988	+ * devices using the Bulk-Only transport (even though this violates
	989	+ * the SCSI spec).
	990	+ */
	991	+ if (us->transport == usb_stor_Bulk_transport)
	992	+ us_to_host(us)->no_scsi2_lun_in_cdb = 1;
	993	+
986	994	/* Find the endpoints and calculate pipe values */
987	995	result = get_pipes(us);
988	996	if (result)
...	...	@@ -174,6 +174,7 @@
174	174	unsigned wce_default_on:1; /* Cache is ON by default */
175	175	unsigned no_dif:1; /* T10 PI (DIF) should be disabled */
176	176	unsigned broken_fua:1; /* Don't set FUA bit */
	177	+ unsigned lun_in_cdb:1; /* Store LUN bits in CDB[1] */
177	178
178	179	atomic_t disk_events_disable_depth; /* disable depth for disk events */
179	180
...	...	@@ -693,6 +693,9 @@
693	693	*/
694	694	struct workqueue_struct *tmf_work_q;
695	695
	696	+ /* The transport requires the LUN bits NOT to be stored in CDB[1] */
	697	+ unsigned no_scsi2_lun_in_cdb:1;
	698	+
696	699	/*
697	700	* Value host_blocked counts down from
698	701	*/