| Message ID | AC1BB7F0327EFB9C+20250206054107.9085-1-wangyuli@uniontech.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | usb-storage: Bypass certain SCSI commands on disks with "use_192_bytes_for_3f" attribute | expand |
On Thu, Feb 06, 2025 at 01:41:07PM +0800, WangYuli wrote: > @@ -369,6 +370,13 @@ static int queuecommand_lck(struct scsi_cmnd *srb) > return SCSI_MLQUEUE_HOST_BUSY; > } > > + if (srb->cmnd[0] == MODE_SENSE && sdev->use_192_bytes_for_3f == 1 && > + srb->cmnd[2] == 0x3f && srb->cmnd[4] != 192) { > + srb->result = DID_ABORT << 16; > + done(srb); > + return 0; > + } > + Please always run scripts/checkpatch.pl on your changes before sending them out so you don't get a grumpy maintainer asking why you didn't run scripts/checkpatch.pl on your patch :( thanks, greg k-h
On Thu, Feb 06, 2025 at 01:41:07PM +0800, WangYuli wrote: > On some external USB hard drives, mounting can fail if "lshw" is > executed during the process. > > This occurs because data sent to the device's output endpoint in > certain abnormal scenarios does not receive a response, leading to > a mount timeout. > > [ Description of "use_192_bytes_for_3f" in the kernel code: ] > /* > * Many disks only accept MODE SENSE transfer lengths of > * 192 bytes (that's what Windows uses). > */ > sdev->use_192_bytes_for_3f = 1; > > The kernel's SCSI driver, when handling devices with this attribute, > sends commands with a length of 192 bytes like this: > if (sdp->use_192_bytes_for_3f) > res = sd_do_mode_sense(sdp, 0, 0x3F, buffer, 192, &data, NULL); > > However, "lshw" disregards the "use_192_bytes_for_3f" attribute and > transmits data with a length of 0xff bytes via ioctl, which can cause > some hard drives to hang and become unusable. > > To resolve this issue, prevent commands with a length of 0xff bytes > from being queued via ioctl when it detects the "use_192_bytes_for_3f" > attribute on the device. Is usb-storage really the right place to put this test? Wouldn't it be better to put it in the SCSI layer where the ioctl is converted to a SCSI command? That way it would affect all SCSI devices with the use_192_bytes_for_3f flag, not just USB devices. Also, instead of making the command fail completely, wouldn't it be better to change the transfer length to 192 if the original value was larger? Alan Stern
On 2025/2/6 22:58, Alan Stern wrote: > Is usb-storage really the right place to put this test? Wouldn't it > be better to put it in the SCSI layer where the ioctl is converted to > a SCSI command? That way it would affect all SCSI devices with the > use_192_bytes_for_3f flag, not just USB devices. Yes, yes... This problem may occur not only in USB devices. It is more appropriate to modify it at the SCSI layer. I'll send the patch v2 soon. > > Also, instead of making the command fail completely, wouldn't it be > better to change the transfer length to 192 if the original value was > larger? But I personally think that it is not appropriate to modify it directly to 192. After all, it is called by the user through ioctl, and the kernel itself will not construct such a data frame. As shown in the following code: sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer) { int res; struct scsi_device *sdp = sdkp->device; struct scsi_mode_data data; int old_wp = sdkp->write_prot; set_disk_ro(sdkp->disk, 0); if (sdp->skip_ms_page_3f) { sd_first_printk(KERN_NOTICE, sdkp, "Assuming Write Enabled\n"); return; } if (sdp->use_192_bytes_for_3f) { res = sd_do_mode_sense(sdp, 0, 0x3F, buffer, 192, &data, NULL); >
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c index d2f476e48d0c..366ab402217c 100644 --- a/drivers/usb/storage/scsiglue.c +++ b/drivers/usb/storage/scsiglue.c @@ -361,6 +361,7 @@ static int queuecommand_lck(struct scsi_cmnd *srb) { void (*done)(struct scsi_cmnd *) = scsi_done; struct us_data *us = host_to_us(srb->device->host); + struct scsi_device *sdev = srb->device; /* check for state-transition errors */ if (us->srb != NULL) { @@ -369,6 +370,13 @@ static int queuecommand_lck(struct scsi_cmnd *srb) return SCSI_MLQUEUE_HOST_BUSY; } + if (srb->cmnd[0] == MODE_SENSE && sdev->use_192_bytes_for_3f == 1 && + srb->cmnd[2] == 0x3f && srb->cmnd[4] != 192) { + srb->result = DID_ABORT << 16; + done(srb); + return 0; + } + /* fail the command if we are disconnecting */ if (test_bit(US_FLIDX_DISCONNECTING, &us->dflags)) { usb_stor_dbg(us, "Fail command during disconnect\n");