lkml.org 
[lkml]   [2018]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.4 050/114] usb-storage: fix bogus hardware error messages for ATA pass-thru devices
    Date
    4.4-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    [ Upstream commit a4fd4a724d6c30ad671046d83be2e9be2f11d275 ]

    Ever since commit a621bac3044e ("scsi_lib: correctly retry failed zero
    length REQ_TYPE_FS commands"), people have been getting bogus error
    messages for USB disk drives using ATA pass-thru. For example:

    [ 1344.880193] sd 6:0:0:0: [sdb] Attached SCSI disk
    [ 1345.069152] sd 6:0:0:0: [sdb] tag#0 FAILED Result: hostbyte=DID_ERROR driverbyte=DRIVER_SENSE
    [ 1345.069159] sd 6:0:0:0: [sdb] tag#0 Sense Key : Hardware Error [current] [descriptor]
    [ 1345.069162] sd 6:0:0:0: [sdb] tag#0 Add. Sense: No additional sense information
    [ 1345.069168] sd 6:0:0:0: [sdb] tag#0 CDB: ATA command pass through(16) 85 06 20 00 00 00 00 00 00 00 00 00 00 00 e5 00
    [ 1345.172252] sd 6:0:0:0: [sdb] tag#0 FAILED Result: hostbyte=DID_ERROR driverbyte=DRIVER_SENSE
    [ 1345.172258] sd 6:0:0:0: [sdb] tag#0 Sense Key : Hardware Error [current] [descriptor]
    [ 1345.172261] sd 6:0:0:0: [sdb] tag#0 Add. Sense: No additional sense information
    [ 1345.172266] sd 6:0:0:0: [sdb] tag#0 CDB: ATA command pass through(12)/Blank a1 06 20 da 00 00 4f c2 00 b0 00 00

    These messages can be quite annoying, because programs like udisks2
    provoke them every 10 minutes or so. Other programs can also have
    this effect, such as those in smartmontools.

    I don't fully understand how that commit induced the SCSI core to log
    these error messages, but the underlying cause for them is code added
    to usb-storage by commit f1a0743bc0e7 ("USB: storage: When a device
    returns no sense data, call it a Hardware Error"). At the time it was
    necessary to do this, in order to prevent an infinite retry loop with
    some not-so-great mass storage devices.

    However, the ATA pass-thru protocol uses SCSI sense data to return
    command status values, and some devices always report Check Condition
    status for ATA pass-thru commands to ensure that the host retrieves
    the sense data, even if the command succeeded. This violates the USB
    mass-storage protocol (Check Condition status is supposed to mean the
    command failed), but we can't help that.

    This patch attempts to mitigate the problem of these bogus error
    reports by changing usb-storage. The HARDWARE ERROR sense key will be
    inserted only for commands that aren't ATA pass-thru.

    Thanks to Ewan Milne for pointing out that this mechanism was present
    in usb-storage. 8 years after writing it, I had completely forgotten
    its existence.

    Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
    Tested-by: Kris Lindgren <kris.lindgren@gmail.com>
    Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1351305
    CC: Ewan D. Milne <emilne@redhat.com>
    CC: <stable@vger.kernel.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/usb/storage/transport.c | 14 +++++++++++++-
    1 file changed, 13 insertions(+), 1 deletion(-)

    diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
    index 02f86dd1a340..90a7bffe3484 100644
    --- a/drivers/usb/storage/transport.c
    +++ b/drivers/usb/storage/transport.c
    @@ -808,12 +808,24 @@ Retry_Sense:
    if (result == USB_STOR_TRANSPORT_GOOD) {
    srb->result = SAM_STAT_GOOD;
    srb->sense_buffer[0] = 0x0;
    + }
    +
    + /*
    + * ATA-passthru commands use sense data to report
    + * the command completion status, and often devices
    + * return Check Condition status when nothing is
    + * wrong.
    + */
    + else if (srb->cmnd[0] == ATA_16 ||
    + srb->cmnd[0] == ATA_12) {
    + /* leave the data alone */
    + }

    /* If there was a problem, report an unspecified
    * hardware error to prevent the higher layers from
    * entering an infinite retry loop.
    */
    - } else {
    + else {
    srb->result = DID_ERROR << 16;
    if ((sshdr.response_code & 0x72) == 0x72)
    srb->sense_buffer[1] = HARDWARE_ERROR;
    --
    2.17.1


    \
     
     \ /
      Last update: 2018-11-08 23:34    [W:4.173 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site