scsi: sd: usb_storage: uas: Access media prior to querying device properties

It has been observed that some USB/UAS devices return generic properties
hardcoded in firmware for mode pages for a period of time after a device
has been discovered. The reported properties are either garbage or they do
not accurately reflect the characteristics of the physical storage device
attached in the case of a bridge.

Prior to commit 1e029397d12f ("scsi: sd: Reorganize DIF/DIX code to
avoid calling revalidate twice") we would call revalidate several
times during device discovery. As a result, incorrect values would
eventually get replaced with ones accurately describing the attached
storage. When we did away with the redundant revalidate pass, several
cases were reported where devices reported nonsensical values or would
end up in write-protected state.

An initial attempt at addressing this issue involved introducing a
delayed second revalidate invocation. However, this approach still
left some devices reporting incorrect characteristics.

Tasos Sahanidis debugged the problem further and identified that
introducing a READ operation prior to MODE SENSE fixed the problem and that
it wasn't a timing issue. Issuing a READ appears to cause the devices to
update their state to reflect the actual properties of the storage
media. Device properties like vendor, model, and storage capacity appear to
be correctly reported from the get-go. It is unclear why these devices
defer populating the remaining characteristics.

Match the behavior of a well known commercial operating system and
trigger a READ operation prior to querying device characteristics to
force the device to populate the mode pages.

The additional READ is triggered by a flag set in the USB storage and
UAS drivers. We avoid issuing the READ for other transport classes
since some storage devices identify Linux through our particular
discovery command sequence.

Link: https://lore.kernel.org/r/20240213143306.2194237-1-martin.petersen@oracle.com
Fixes: 1e029397d12f ("scsi: sd: Reorganize DIF/DIX code to avoid calling revalidate twice")
Cc: stable@vger.kernel.org
Reported-by: Tasos Sahanidis <tasos@tasossah.com>
Reviewed-by: Ewan D. Milne <emilne@redhat.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Tested-by: Tasos Sahanidis <tasos@tasossah.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 0833b3e6aa6e8f35b791d3f75fe208fb0f888914..bdd0acf7fa3cb130e64fac2aacf684aa5a91da8b 100644 (file)
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3407,6 +3407,24 @@ static bool sd_validate_opt_xfer_size(struct scsi_disk *sdkp,
        return true;
 }
 
+static void sd_read_block_zero(struct scsi_disk *sdkp)
+{
+       unsigned int buf_len = sdkp->device->sector_size;
+       char *buffer, cmd[10] = { };
+
+       buffer = kmalloc(buf_len, GFP_KERNEL);
+       if (!buffer)
+               return;
+
+       cmd[0] = READ_10;
+       put_unaligned_be32(0, &cmd[2]); /* Logical block address 0 */
+       put_unaligned_be16(1, &cmd[7]); /* Transfer 1 logical block */
+
+       scsi_execute_cmd(sdkp->device, cmd, REQ_OP_DRV_IN, buffer, buf_len,
+                        SD_TIMEOUT, sdkp->max_retries, NULL);
+       kfree(buffer);
+}
+
 /**
  *     sd_revalidate_disk - called the first time a new disk is seen,
  *     performs disk spin up, read_capacity, etc.
@@ -3446,7 +3464,13 @@ static int sd_revalidate_disk(struct gendisk *disk)
         */
        if (sdkp->media_present) {
                sd_read_capacity(sdkp, buffer);
-
+               /*
+                * Some USB/UAS devices return generic values for mode pages
+                * until the media has been accessed. Trigger a READ operation
+                * to force the device to populate mode pages.
+                */
+               if (sdp->read_before_ms)
+                       sd_read_block_zero(sdkp);
                /*
                 * set the default to rotational.  All non-rotational devices
                 * support the block characteristics VPD page, which will

diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index c54e9805da536a0ec139ad789017b79131c88561..12cf9940e5b6759167f9ae7450df8af92a85c63a 100644 (file)
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -179,6 +179,13 @@ static int slave_configure(struct scsi_device *sdev)
                 */
                sdev->use_192_bytes_for_3f = 1;
 
+               /*
+                * Some devices report generic values until the media has been
+                * accessed. Force a READ(10) prior to querying device
+                * characteristics.
+                */
+               sdev->read_before_ms = 1;
+
                /*
                 * Some devices don't like MODE SENSE with page=0x3f,
                 * which is the command used for checking if a device

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 9707f53cfda9c08507082ac33b69b5d146c6927f..71ace274761f182f0cbb942676e74d7e2c26d7a1 100644 (file)
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -878,6 +878,13 @@ static int uas_slave_configure(struct scsi_device *sdev)
        if (devinfo->flags & US_FL_CAPACITY_HEURISTICS)
                sdev->guess_capacity = 1;
 
+       /*
+        * Some devices report generic values until the media has been
+        * accessed. Force a READ(10) prior to querying device
+        * characteristics.
+        */
+       sdev->read_before_ms = 1;
+
        /*
         * Some devices don't like MODE SENSE with page=0x3f,
         * which is the command used for checking if a device

diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 5ec1e71a09de7698616dff799a935da15083deef..01c02cb76ea6b103582461183bfe1699ecd2d028 100644 (file)
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -208,6 +208,7 @@ struct scsi_device {
        unsigned use_10_for_rw:1; /* first try 10-byte read / write */
        unsigned use_10_for_ms:1; /* first try 10-byte mode sense/select */
        unsigned set_dbd_for_ms:1; /* Set "DBD" field in mode sense */
+       unsigned read_before_ms:1;      /* perform a READ before MODE SENSE */
        unsigned no_report_opcodes:1;   /* no REPORT SUPPORTED OPERATION CODES */
        unsigned no_write_same:1;       /* no WRITE SAME command */
        unsigned use_16_for_rw:1; /* Use read/write(16) over read/write(10) */