| Message ID | ADB8844D07D40320+20250224034832.40529-1-wangyuli@uniontech.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [RESEND,v3] scsi: Bypass certain SCSI commands on disks with "use_192_bytes_for_3f" attribute | expand |
On 2/23/25 7:48 PM, WangYuli wrote: > 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. lshw is a user space utility. use_192_bytes_for_3f is not exposed to user space as far as I know. So how can the above statement be correct? > @@ -1613,6 +1614,17 @@ static int scsi_dispatch_cmd(struct scsi_cmnd *cmd) > + /* > + * Before we queue this command, check attribute use_192_bytes_for_3f. > + * Because transmits data with a length of 0xff bytes via ioctl may > + * cause some hard drives to hang and become unusable. > + */ > + if (cmd->cmnd[0] == MODE_SENSE && sdev->use_192_bytes_for_3f && > + cmd->cmnd[2] == 0x3f && cmd->cmnd[4] != 192) { > + cmd->result = DID_ABORT << 16; > + goto done; > + } From include/scsi/scsi_device.h: unsigned use_192_bytes_for_3f:1; /* ask for 192 bytes from page 0x3f */ The above code uses use_192_bytes_for_3f for another purpose. Please respect the purpose of the use_192_bytes_for_3f bitfield. Thanks, Bart.
Hi Bart Van Assche, On 2025/2/25 02:11, Bart Van Assche wrote: > lshw is a user space utility. use_192_bytes_for_3f is not exposed to > user space as far as I know. So how can the above statement be correct? > Disks have the attribute use_192_bytes_for_3f, which means that disks only accept MODE SENSE transfer lengths of 192 bytes However, when lshw sends MODE SENSE command to disks, use_192_bytes_for_3f will not be considered, which will cause some disks with use_192_bytes_for_3f to be unusable To solve this problem, it is necessary to determine whether the device has the use_192_bytes_for_3f attribute at the scsi level. If the device has use_192_bytes_for_3f, when lshw or other applications send MODE SENSE command through ioctl, the command with the data length field of 0xff needs to be filtered out to avoid device abnormality. > > From include/scsi/scsi_device.h: > > unsigned use_192_bytes_for_3f:1; /* ask for 192 bytes from page 0x3f */ > > The above code uses use_192_bytes_for_3f for another purpose. Please > respect the purpose of the use_192_bytes_for_3f bitfield. > > Thanks, > > Bart. > > Thanks,
On 2/25/25 5:11 AM, WangYuli wrote: > Disks have the attribute use_192_bytes_for_3f, which means that disks > only accept MODE SENSE transfer lengths of 192 bytes> > However, when lshw sends MODE SENSE command to disks, > use_192_bytes_for_3f will not be considered, which will cause some > disks with use_192_bytes_for_3f to be unusable > > To solve this problem, it is necessary to determine whether the > device has the use_192_bytes_for_3f attribute at the scsi level. If > the device has use_192_bytes_for_3f, when lshw or other applications > send MODE SENSE command through ioctl, the command with the data > length field of 0xff needs to be filtered out to avoid device > abnormality. Has it been considered to truncate the MODE SENSE buffer to 192 bytes instead of rejecting the MODE SENSE command? Thanks, Bart.
Hi Bart, On 2025/2/26 04:54, Bart Van Assche wrote: > > Has it been considered to truncate the MODE SENSE buffer to 192 bytes > instead of rejecting the MODE SENSE command? > > Alan Stern has raised a related issue before. My take on this is outlined below. 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); Link: https://lore.kernel.org/all/137902FEE03CCB3B+6130227f-9ddc-4043-9945-da465c28d9d1@uniontech.com/
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index be0890e4e706..29371826fc85 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1565,6 +1565,7 @@ static void scsi_complete(struct request *rq) static int scsi_dispatch_cmd(struct scsi_cmnd *cmd) { struct Scsi_Host *host = cmd->device->host; + struct scsi_device *sdev = cmd->device; int rtn = 0; atomic_inc(&cmd->device->iorequest_cnt); @@ -1613,6 +1614,17 @@ static int scsi_dispatch_cmd(struct scsi_cmnd *cmd) goto done; } + /* + * Before we queue this command, check attribute use_192_bytes_for_3f. + * Because transmits data with a length of 0xff bytes via ioctl may + * cause some hard drives to hang and become unusable. + */ + if (cmd->cmnd[0] == MODE_SENSE && sdev->use_192_bytes_for_3f && + cmd->cmnd[2] == 0x3f && cmd->cmnd[4] != 192) { + cmd->result = DID_ABORT << 16; + goto done; + } + if (unlikely(host->shost_state == SHOST_DEL)) { cmd->result = (DID_NO_CONNECT << 16); goto done;