linux-kernel - RE: [PATCH] usb-storage: scsiglue: Changing the command result

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 15 Oct 2013 21:59:39 +0530
From:	Vishal Annapurve <vannapurve@...dia.com>
To:	Alan Stern <stern@...land.harvard.edu>,
	Ming Lei <tom.leiming@...il.com>
CC:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-usb <linux-usb@...r.kernel.org>
Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result

Hi Alan,

This issue seems more related to the devices using SCSI protocol and the
changes otherwise will be at more places giving the same end result.

I think as the comment says over the command_abort function,
intentional result change should only happen in case of timeout.

Regards,
Vishal

-----Original Message-----
From: Alan Stern [mailto:stern@...land.harvard.edu] 
Sent: Tuesday, October 15, 2013 5:55 PM
To: Ming Lei
Cc: Vishal Annapurve; Linux Kernel Mailing List; linux-usb
Subject: Re: [PATCH] usb-storage: scsiglue: Changing the command result

On Tue, 15 Oct 2013, Ming Lei wrote:

> On Thu, Sep 26, 2013 at 7:35 PM, Vishal Annapurve <vannapurve@...dia.com> wrote:
> > Hi,
> >
> > There was a recent commit in mainline for the scsi devices which do 
> > not respond properly to medium access command:
> >
> > commit    18a4d0a22ed6c54b67af7718c305cd010f09ddf8
> >
> > [SCSI] Handle disk devices which can not process medium access 
> > commands We have experienced several devices which fail in a fashion 
> > we do not currently handle gracefully in SCSI. After a failure these 
> > devices will respond to the SCSI primary command set (INQUIRY, TEST 
> > UNIT READY, etc.) but any command accessing the storage medium will time out.
> >
> > I came across a USB drive which showed similar problem and what I 
> > see is usb storage is still not able to cope with such devices properly.
> >
> > The control flow downwards is like:
> > scsi_times_out --> Setting cmd->result as DID_TIME_OUT 
> > scsi_error_handler scsi_unjam_host scsi_eh_abort_cmds
> > command_abort    (sets US_FLIDX_TIMED_OUT for us->dflags
> >                                   calls stop_transport,
> >                                   and waits for)    usb_stor_control_thread
> > (which is waiting for
> >                                                                    
> > transport call to return inside
> >
> > usb_stor_invoke_transport)
> >                                                                    
> > both usb_stor_control_thread and
> >
> > usb_stor_invoke_transport
> >                                                     check for 
> > us->dflags timed_out bit and
> >                                                      set the result 
> > as DID_ABORT
> >                                                      and signal 
> > completion for command_abort
> >                                                      to complete 
> > ......
> > sd_eh_action
> > checks for cmd->result and finds out that it's DID_ABORT rather than 
> > DID_TIME_OUT.
> >
> > This patch updates the command result to be TIME_OUT explicitly 
> > before returning from command_abort in scsiglue.c.
> >
> > I would like to know if this patch can work out for such USB Storage 
> > devices? What would be the better way to do the same?
> 
> Looks your diagnose is correct, and patch should be doable, the only 
> side effect is that previous returned DID_ABORT in srb->result is 
> replaced with DID_TIME_OUT now.
> 
> Another way is to implement .eh_timed_out callback to return different 
> error for the two failure, but looks not a big deal.

Instead of overriding the result code, a better way to do this would simply be to change the places where srb->result is currently set to DID_ABORT << 16.  Just make it DID_TIME_OUT << 16 instead.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/